Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocket: wrong subprotocol choose check (response Sec-WebSocket-Protocol header) #2844

Closed
farwayer opened this issue Feb 25, 2024 · 2 comments · Fixed by #2845
Closed

WebSocket: wrong subprotocol choose check (response Sec-WebSocket-Protocol header) #2844

farwayer opened this issue Feb 25, 2024 · 2 comments · Fixed by #2845
Labels
bug Something isn't working

Comments

@farwayer
Copy link

farwayer commented Feb 25, 2024

Bug Description

  1. client sends Sec-WebSocket-Protocol header with protocol list (separated by ',')
  2. server responds with one protocol from this list
  3. WebSocket client compares request header (protocol list) with response header (one protocol) value and fails with the error Protocol was not set in the opening handshake

if (secProtocol !== null && secProtocol !== request.headersList.get('Sec-WebSocket-Protocol')) {

Reproducible By

let ws = new WebSocket('ws://server...', ['msgpack', 'json'])
ws.addEventListener('error', e => console.log(e.error))
// server choses json and responds with `Sec-WebSocket-Protocol: json`
// client failed

Expected Behavior

Correct subprotocol check

Environment

  • Node.js v21.6.2
  • --experimental-websocket flag
@farwayer farwayer added the bug Something isn't working label Feb 25, 2024
@KhafraDev
Copy link
Member

KhafraDev commented Feb 25, 2024

I will have a fix for this soon.

Relevant part of the specification:

   The client can request that the server use a specific subprotocol by
   including the |Sec-WebSocket-Protocol| field in its handshake.  If it
   is specified, the server needs to include the same field and one of
   the selected subprotocol values in its response for the connection to
   be established.

https://datatracker.ietf.org/doc/html/rfc6455#section-1.9

@kettanaito
Copy link
Contributor

@KhafraDev, is there a chance to backport this to Node.js v18? This bug breaks WebSocket connections with multiple protocols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants