netty / netty

Netty project - an event-driven asynchronous network application framework
http://netty.io
Apache License 2.0
33.34k stars 15.9k forks source link

Should not allow multiple Subprotocols for WebSocket Client Handshake #7166

Closed irunika closed 7 years ago

irunika commented 7 years ago

According to the specification of WebSocket sub-protocols in RFC 6455 section 1.9 even though the server could have multiple subprotocols which can be allowed, a client can only ask for a specific subprotocol in the handshake. But in Netty even the client comes with multiple subprotocols, the server accepts it. IMHO this is not the way which the handshake should be done with subprotocols.

normanmaurer commented 7 years ago

@irunika would you be interested in providing a PR for a fix with a unit test ?

irunika commented 7 years ago

@normanmaurer K sure. I will send a PR.

normanmaurer commented 7 years ago

@irunika thanks!

irunika commented 7 years ago

@normanmaurer I looked into your code. I think I was wrong about the behavior. I checked with Wireshark and found out that netty is responding with the first found compatible sub protocol. Not all. So that is fine.
But even though the supported sub protocols are null in the server and if client comes asking some sub protocol handshake is happening without sec-websocket-protocol header in the response which means that it does not support sub protocols. Is this a good behavior? Because even the client is asking for sub protocols netty handshakes and say it does not support them by not sending the sec-websocket-protocol header.

irunika commented 7 years ago

Closing PR since the given issue is not valid anymore. Will open issue for null issue.