Handle Sec-WebSocket-Protocol header

[benkolera]

It looks like the handshake code doesn't do anything with the Sec-WebSockets-Protocol header. When I try to connect to my websocket proof of concept server (https://gist.github.com/benkolera/9427813) with www.novnc.com I get an exception in the novnc javascript code:

WebSocket connection to 'ws://localhost:5003/websockify' failed: Error during   WebSocket handshake: Sent non-empty 'Sec-WebSocket-Protocol' header but no response was received 

Now looking into the headers I can see that novnc is trying to connect and establish with us to send things as binary or base64 (both of which we can do).

GET /websockify HTTP/1.1
Upgrade: websocket
Connection: Upgrade
Host: localhost:5003
Origin: http://kanaka.github.io
Sec-WebSocket-Protocol: binary, base64
Pragma: no-cache
Cache-Control: no-cache
Sec-WebSocket-Key: U9FEmmtMOzsE9Zhlzsyyfw==
Sec-WebSocket-Version: 13
Sec-WebSocket-Extensions: permessage-deflate; client_max_window_bits, x-webkit-deflate-frame
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.117 Safari/537.36
Cookie: __ngDebug=false

But when we respond with the websocket upgrade there is no mention of the protocol that we agree to use:

HTTP/1.1 101 WebSocket Protocol Handshake
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Accept: V17xzl+yibPrLHx/qPywxD/AuZY=

And I believe that it is required by the spec to do such a thing (https://tools.ietf.org/html/rfc6455#section-1.3)

Is this something that has been considered before and not done for a reason or has in not been done because of the lack of need or time? If it is the latter, I can do the work to get together a pull request that would make this happen. Without thinking about it too much or having too much experience with the code other than the code dive I just did, I had it in my head that we'd have to change pending request to have a list of protocols being requested and have the acceptRequest method choose a protocol (if there is a choice to be made) somehow.

[jaspervdj]

Right, I thought this was optional but it is actually required by the spec.

I haven't done it yet because the need hasn't come up, and I'm a little bit short on time at this point due to work. It would be nice if you could put together a pull request! I don't mind giving a few implementation pointers.

PendingConnection has a RequestHead that can be obtained using pendingRequest. I would add the protocol list to this RequestHead type and parse it as an empty list if it not present.

Then, I would add an acceptRequestWith function which takes a (new) AcceptRequest parameter as well as the PendingConnection. This new datatype would only have one parameter for now, which is the selected protocol as a Maybe. It is then up to the user to do the right thing -- I don't think that's much of a problem.

Do you agree with this? Do you need more information on anything? I'd be glad to help where necessary.

[wraithm]

Hi Jasper, I'm also experiencing this issue. It looks like @wuzzeb created a pull request (#76) that hasn't been merged yet. I'm going to try to merge your more recent code with @wuzzeb's and see if it fixes the issue I'm having.

[creichert]

I am experiencing timeouts after 5 minutes and I'm wondering if this might be the root cause.

I patched yesod-websockets with the forkPingThread suggestions but some people still see the following in their logs intermittently:

17/Dec/2014:20:52:30 +0000 [Error#yesod] Exception from Warp: MalformedRequest (RequestHead {requestPath = "/", requestHeaders = [("X-Real-IP","213.180.206.198"),("X-Forwarded-Proto","http"),("Upgrade","WebSocket"),("Connection","Upgrade"),("Host","xn------6cdmlcgteeqj6an5bzbyeta.xn--p1ai"),("Origin","http://xn------6cdmlcgteeqj6an5bzbyeta.xn--p1ai"),("Cookie","_SESSION=++ToXox/8yfcv8Eh7+ndEmAROaykPHAuRaX98pCxOC0XegcUkIzCOEAyQJNgpzii8TCYtPKionScpVsF5Y0LMdUPGiro8IyIVybUVXYYR+p0UZfp467lFaWVB1rJHVN7zpOQwRxOTVY="),("Sec-WebSocket-Key1","142 9 8 08604 X"),("Sec-WebSocket-Key2","N1 4 0_ 0y93`@ 592uG Fn")], requestSecure = False}) "Header missing: Sec-WebSocket-Key" @(yesod-1.4.1:Yesod.Default.Main ./Yesod/Default/Main.hs:73:15)

Seems to be at least related to the Sec-WebSocket-Protocol header.

[wraithm]

Ignore my previous comment. I didn't realize that @wuzzeb's commits on #76 were already merged. I guess #76 didn't fix #61 for me. I'm going to try to figure out what's going on.

[wraithm]

Okay, the fix for this issue was fairly simple. I noticed that the "Sec-WebSocket-Protocol" header value from my browser was "Sec-WebSocket-Protocol".

So, instead of

conn <- acceptRequest pending

I used,

conn <- acceptRequestWith pending (AcceptRequest (Just "Sec-WebSocket-Protocol"))

That fixed my issue entirely. Everything worked flawlessly after that.

[creichert]

@wraith great, I'm testing that patch now. Was your problem related to the one I posted?

[wraithm]

I don't think so. I'm using websockets-snap, and I wasn't having problems with timeouts. I was having problems with the handshake, as stated in the first comment.