lxzan / gws

simple, fast, reliable websocket server & client, supports running over tcp/kcp/unix domain socket. keywords: ws, proxy, chat, go, golang...
https://pkg.go.dev/github.com/lxzan/gws
Apache License 2.0
1.34k stars 84 forks source link

pass SubProtocols to Upgrade #70

Closed mrkmrtns closed 8 months ago

mrkmrtns commented 8 months ago

Problem

We have problem that SubProtocols are not known beforehand and are rather dynamic. Therefore we need capability to specify support subprotocols for every websocket connection.

I made it by adding next to Upgrade function UpgradeWithOptions.

https://github.com/lxzan/gws/compare/master...mrkmrtns:gws:upgradewithoptions You can check it out here, if it suits for You then I can prepare PR.

UpgradeOptions struct can be extended in future for any other overwrites/opts - currently added there only SubProtocols for our need.

Any questions/recommendations are welcome.

lxzan commented 8 months ago

What usage scenarios require a dynamic Sec-WebSocket-Protocol ?

mrkmrtns commented 8 months ago

Sec-WebSocket-Protocol is used for packing into there also session credentials - it allows us to check at time connection is created if You are allowed to establish it in first place. We do not want to allow establish connection at all if You are not allowed to.

Another option would be to use query parameters for that, but for obvious reasons it's not good idea to set it that way. Afaik using SubProtocols is the only way You can send it in header from browser.

So we can say there is practically infinite numbers of those, one per valid session.

lxzan commented 8 months ago

The original was to use sub-protocols for authentication, not a recommended practice, but it works.

This can be achieved without modifying the gws source code

let ws = new WebSocket($addr, ['gws', $token])
upgrader := gws.NewUpgrader(&Handler{}, &gws.ServerOption{
    SubProtocols: []string{"gws"},
})

The server can then get the token from the request header

mrkmrtns commented 8 months ago

When sending "multiple" then it indeed works, although with just token would be nicer imo. As I see You do not want to add overwrite option - then can workaround as You suggested.