Crow icon indicating copy to clipboard operation
Crow copied to clipboard

Add the possibility to add response header on WebSocket handshake request

Open KaSSaaaa opened this issue 1 year ago • 5 comments

Hi,

I'm working on a small project with HTTP & WS and I chose Crow for that matter. The only thing is, I'm using a JS library for testing purposes but the WebSocket keeps closing because some headers are not available on the handshake response.

Is there any possibility to add headers to the handshake query's response ?

Otherwise, I've looked a bit into the source code and it shouldn't be too complicated to add something like an "onhandshake" method (like you have "onopen",...) which could allow the user to add the response headers on the handshake response. I would be glad to create a Pull Request with that feature if necessary.

Best regards, Pierre

KaSSaaaa avatar Jul 29 '23 15:07 KaSSaaaa

Out of curiosity... which headers are missing? Just adding headers might not be enough. Most headers negotiate features, which probably are not implemented yet.

MichaelSB avatar Jul 29 '23 17:07 MichaelSB

I'm missing the "Sec-WebSocket-Protocol" header. I've tried editing the code of the latest release version with the "onhandshake" callback and it works pretty well.

KaSSaaaa avatar Jul 29 '23 17:07 KaSSaaaa

What now follows is just my opinion on the matter... There's more to "Sec-WebSocket-Protocol" than just adding a header to make it work correctly. It may be the case in your case, but it is not generally speaking. Adding an onhandshake-hook to crow, that allows adding random headers to the handshake imo is not the way to go here. If you need "Sec-WebSocket-Protocol" support, one should add "Sec-WebSocket-Protocol" support and not something that works in singular cases and does nothing in others. That said... as far as I understand the spec by shortly looking at it, adding support for Sec-WebSocket-Protocol-support wouldn't be that hard to do. You get a list of protocols, have a list of protocols and if there's at least one match, continue normally with one of the matches in the response to the client, otherwise fail the connection.

MichaelSB avatar Jul 29 '23 18:07 MichaelSB

You're right ! I can add something like that just for "Sec-WebSocket-Protocol" then. Like a hook on the WebSocketRule that takes a list of strings, check that in the Connection and then add them to the response body. What do you think ?

KaSSaaaa avatar Jul 29 '23 18:07 KaSSaaaa

What you just described is likely what I would do. Sounds good to me :D

MichaelSB avatar Jul 29 '23 18:07 MichaelSB