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

[bug] Websocket subprotocol is not chosen on client preferance #822

Closed
KSDaemon opened this issue Oct 20, 2022 · 0 comments · Fixed by #823
Closed

[bug] Websocket subprotocol is not chosen on client preferance #822

KSDaemon opened this issue Oct 20, 2022 · 0 comments · Fixed by #823
Labels

Comments

@KSDaemon
Copy link
Contributor

Describe the bug

From the Websocket RFC6455:

For client side:

|Sec-WebSocket-Protocol| header field, with a list of values indicating which protocols the client would like to speak, ordered by preference.

And for server side:

Either a single value representing the subprotocol the server is ready to use or null. The value chosen MUST be derived from the client's handshake, specifically by selecting one of the values from the |Sec-WebSocket-Protocol| field that the server is willing to use for this connection (if any).

So if the client provides a few options for subprotocol. The server should choose the first one it supports.

Right now, if client provides a few options, lib choose the first one it supports (and not the first one from the client).

e.g. So if the client sends Sec-WebSocket-Protocol: wamp.2.cbor, wamp.2,json and server supports wamp.2,json, wamp.2.cbor then wamp.2,json will be chosen but not wamp.2.cbor as it should be.

A clear and concise description of what the bug is.

Lib version: all :)

Code Snippets
The problem is in server.go: selectSubprotocol func:

		clientProtocols := Subprotocols(r)
		for _, serverProtocol := range u.Subprotocols {
			for _, clientProtocol := range clientProtocols {
				if clientProtocol == serverProtocol {
					return clientProtocol
				}
			}
		}

should be changed to:

        clientProtocols := Subprotocols(r)
        for _, clientProtocol := range clientProtocols {
            for _, serverProtocol := range u.Subprotocols {
                if clientProtocol == serverProtocol {
                    return clientProtocol
                }
            }
        }
@KSDaemon KSDaemon added the bug label Oct 20, 2022
@coreydaley coreydaley moved this to 🏗 In progress in Gorilla Web Toolkit Jul 23, 2023
@coreydaley coreydaley moved this from 🏗 In progress to 👀 In review in Gorilla Web Toolkit Aug 17, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Gorilla Web Toolkit Nov 7, 2023
coreydaley added a commit that referenced this issue Nov 7, 2023
Fixes #822

**Summary of Changes**

1. Changed the order of subprotocol selection to prefer client one

---------

Co-authored-by: Corey Daley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants