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

Choose websocket subprotocol preffered by client #282

Open
KSDaemon opened this issue Oct 19, 2022 · 7 comments
Open

Choose websocket subprotocol preffered by client #282

KSDaemon opened this issue Oct 19, 2022 · 7 comments
Assignees
Labels

Comments

@KSDaemon
Copy link
Collaborator

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, Nexus choose the first one it supports (and not the first one from the client). And first in list is json.

So if the client sends Sec-WebSocket-Protocol: wamp.2.cbor, wamp.2,json → Nexus will choose json and not the cbor

@KSDaemon KSDaemon self-assigned this Oct 19, 2022
@KSDaemon KSDaemon added the bug label Oct 19, 2022
@KSDaemon
Copy link
Collaborator Author

KSDaemon commented Oct 20, 2022

@KSDaemon
Copy link
Collaborator Author

KSDaemon commented Nov 14, 2022

So even the original author agrees that that is bug and fix is ok. But as there is no maintainer in websocket project (4 years looking for new maintainer) — fix can stuck for ages.....

@gammazero What do you think? Maybe we can switch to fork with bugfix temporarily? As websocket library development is mostly frozen,  I don't expect any new features to come soon (and even bugfixes are very slow to get into the codebase). So we won't lose much with bugfix-fork dependency.

@KSDaemon
Copy link
Collaborator Author

So maybe we can switch someday to something else, like https://github.com/lesismal/nbio. (We use it internally, pretty good XP, speed)

@KSDaemon
Copy link
Collaborator Author

KSDaemon commented Dec 1, 2022

@gammazero what do you think? Please have a look when you have time. This one is pretty small but important question

@om26er
Copy link
Contributor

om26er commented Jan 12, 2023

gorilla project has been archived since last year. So now we are on our own for this one https://github.com/gorilla/websocket

@gammazero
Copy link
Owner

I am fine with switching to another websocket implementation if we need to, but it looks like gorilla/websocket is now actively maintained again so would prefer to stay with it.

@KSDaemon
Copy link
Collaborator Author

KSDaemon commented Nov 7, 2023

Wow! It took just about 1 year and few weeks till my PR with the Web Socket subprotocol fix landed in the upstream library! Now we can finally update the dependency!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants