calling verifyClient callback multiple times results in "RSV1 must be clear" errors

[delasky]

• [ X] I've searched for any related issues and avoided creating a duplicate issue.

Description

Calling the verifyClient callback twice in verifyClient function triggers an "Invalid WebSocket frame: RSV1 must be clear"

It's not clear to me why this leads to that specific error. Calling the callback multiple times should probably be expressly forbidden with a related error message.

The error occurs when this.completeUpgrade is called twice.

I also think this might be the root cause behind #1238.

Reproducible in:

version:
Node.js version(s): 7.4.0
OS version(s): Mac OS X 10.10.5

Steps to reproduce:

1. run the following code

let WebSocket = require('ws')

let verify = (info, cb) => {
    cb(true)
    cb(true)
}

const wss = new WebSocket.Server({ port: 8080, verifyClient: verify});

wss.on('connection', function connection(ws) {
  ws.on('message', function incoming(message) {
      console.log('received: %s', message);
    });

  ws.send('something');
});

const ws = new WebSocket('ws://localhost:8080');

ws.on('open', function open() {

  ws.send("hello");
})

Expected result:

A different error message, or ignore subsequent calls of the verifyClient callback.

Actual result:

RSV1 must be clear

[lpinca]

The error happens because the first call completes the handshake, all other calls write invalid data (handshake headers) on the socket.

We can add a guard to ensure the callback is only called once but in general it's user responsibility.

#1238 is probably caused by nodejs/node#17789. A patch is already available: nodejs/node#17806.

[lpinca]

I think this is actually a good thing, but I'm willing to be convinced otherwise. If we prevent the callback from being called multiple times, users will never know that something is wrong on their end.

[delasky]

Thats fair. If i have time this week I might put in a PR to guard with a specific error, but at least now people who search in the issues can see this. Don't think its high priority

[lpinca]

Ok, thank you.