handleProtocols property is not executed if no protocols are present

[andsouto]

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

Description

I'm using the handleProtocols property when creating a server to force clients connecting to the server to send the name of the required protocol. But when the client doesn't send any protocol, the function is not executed so validation can't be done.

Reproducible in:

• version: 6.2.1
• Node.js version(s): v8.15.1

Steps to reproduce:

1. Create a WebSocket.Server with a function in the handleProtocols property of the Options object

2. Create a client connecting with the server that doesn't send the Sec-WebSocket-Protocol.

3. The connection is established without calling the handleProtocols function.

Expected result:

I expect it to be executed always to be able to reject the request if the required protocol is not sent by the client.

Actual result:

Function is not executed and the socket is established.

Attachments:

function handleProtocols(protocols: string[]) {
	return protocols.includes(PROTOCOL) ? PROTOCOL : false;
}

httpServer = http.createServer();
const wss = new WebSocket.Server({
	handleProtocols,
	server,
});
httpServer.listen(PORT);

const ws = new WebSocket(`ws://localhost:${PORT}`);

[lpinca]

This is by design. There is no subprotocol to negotiate.
Use https://github.com/websockets/ws/blob/master/doc/ws.md#servershouldhandlerequest if you want to abort the handshake when no subprotocol is sent by the client.

[andsouto]

Thanks for your answer. I think I got it. I managed to achieve what I need using verifyClient:

const verifyClient: VerifyClientCallbackAsync = (info, cb) => {
	const protocol = info.req.headers['sec-websocket-protocol'];
	if (protocol && protocol.includes(PROTOCOL)) {
		cb(true);
	} else {
		cb(false, 403);
	}
};

I suppose the same could be achieved replacing shouldHandle as you suggested but this way seemed easier to me.

Thank you so much and sorry for the noise.

[lpinca]

Yes, that works too. See also #377 (comment).
Closing, discussion can continue if needed.