WebSocket accept() fails when Connection header contains multiple tokens

[bakercp]

Currently the accept() method fails when receiving WebSocket connections from Firefox because Firefox sends the :

Connection: keep-alive, Upgrade

while webkit (and others) use

Connection: Upgrade

Thus, when using Firefox (which is using a poco compatible WEBSOCKET_VERSION === 13), the connection fails.

Currently my work-around is to do a more thorough header check and if the Connection headers contains the Upgrade token (in any position), I reset the header like this request.set("Connection","Upgrade") before constructing the websocket like this Websocket ws(request,response);. When I do this, I am able to communicate successfully with the same browser Firefox using Poco's WebSocket Server implementation.

Other WebSocket server implementations have run into this same problem (a quick google search will locate a few other github issues that were resolved). A justification for the Firefox multi-token Connection header was given here - a quick summary:

RFC 2068 section 14.10 is quite clear that it's permissible to send multiple comma-delineated tokens in the Connection header. And the websocket spec--RFC 6455--says that "The request MUST contain a |Connection| header field whose value MUST include the "Upgrade" token" (section 4.1): note the use of "include", not "be equal to". But twimbow's server is simply failing to reply at all to our HTTP handshake, and we eventually time out the connection. (If I change the Connection header to just "Upgrade", we connect fine)...

Anyway, my workaround is fine for the moment, but perhaps a Poco could drop the single-token Connection header requirement in a future version?

Thanks!

[bakercp]

By the way, the "offending" line of code is here:

https://github.com/pocoproject/poco/blob/develop/Net/src/WebSocket.cpp#L136

[aleks-f]

Fixed in develop, I'm not quite sure if the response should preserve the whole Connection value (right now it returns only "Upgrade")?

[bakercp]

That is a good question ... it's not entirely clear from my research what should happen in this case. According to the HTTP 1.1. spec, the keep-alive headers aren't required (http://tools.ietf.org/html/rfc2616#section-8.1.2) and are probably being used "just in case" by Firefox.

The other WebSocket server implementations I have seen just return "Upgrade" (i.e. they just set the response Connection: Upgrade, and don't make any effort to pass the full value from the request). I suppose, if Poco's Server side WebSocket is playing by the 1.1 rules, it could be justified in not returning anything other than the Upgrade.

Ultimately, in my mind, your fix closes this issue though, so thanks!

[obiltschnig]

btw, the code in the 1.4.6 branch already has a fix for this issue.

Günter Obiltschnig Applied Informatics Software Engineering GmbH A-9182 Maria Elend | Maria Elend 96/4 | www.appinf.com P: +43 4253 32596 M: +43 676 5166737 F: +43 4253 32096

Company Registration: FN 276491 f | Landesgericht Klagenfurt Managing Director: DI Günter Obiltschnig

On 26.02.2013, at 19:31, Christopher Baker wrote:

That is a good question ... it's not entirely clear from my research what should happen in this case. According to the HTTP 1.1. spec, the keep-alive headers aren't required (http://tools.ietf.org/html/rfc2616#section-8.1.2) and are probably being used "just in case" by Firefox.

The other WebSocket server implementations I have seen just return "Upgrade" (i.e. they just set the response Connection: Upgrade, and don't make any effort to pass the full value from the request). I suppose, if Poco's Server side WebSocket is playing by the 1.1 rules, it could be justified in not returning anything other than the Upgrade.

Ultimately, in my mind, your fix closes this issue though, so thanks!

— Reply to this email directly or view it on GitHub.

[bakercp]

@obiltschnig Ah, I'm using 1.4.3 (in openFrameworks) and only looked at the develop branch, assuming that it was the most up to date. My apologies for not looking more closely at the latest release. It looks like the .hasToken method may be a cleaner way to do it. Thanks, CB

[bakercp]

hi @obiltschnig and @aleks-f while I (hopefully) have your attention for a minute --

I have been running into some situations where I'm not able to catch the client disconnect messages (or client disconnect exceptions), leading the test code to forever loop on this line:

https://github.com/pocoproject/poco/blob/poco-1.4.6/Net/samples/WebSocketServer/src/WebSocketServer.cpp#L142

Would it make more sense to change the logical OR to an AND?

while (n > 0 && (flags & WebSocket::FRAME_OP_BITMASK) != WebSocket::FRAME_OP_CLOSE);

?

Thanks, CB