Last Comment Bug 730742 - node.js mishandles Connection: header during websocket handshake
: node.js mishandles Connection: header during websocket handshake
Status: RESOLVED FIXED
:
Product: Tech Evangelism Graveyard
Classification: Graveyard
Component: English US (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal
: ---
Assigned To: english-us
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-26 17:26 PST by Knight00931
Modified: 2015-04-19 23:39 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Knight00931 2012-02-26 17:26:47 PST
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1
Build ID: 20120226031015

Steps to reproduce:

Just go to Twimbow (http://www.twimbow.com/dashboard.php), it's a twitter web client.
Error console says Firefox can't establish a connection to the server with ws://twimbow.com ...



Actual results:

Error console shows the following message many times.

Timestamp: 2012/2/27 上午 09:22:37
Error: Firefox can't establish a connection to the server at ws://twimbow.com:8125/rand/178840519c8ebf4aeb55910df24319c409db1133b3fb820bcc2e73f26345fbc5c5d81de8f0ea5eb162e8e563fb770da20e34d865545584c62f9309f6755abd5e11bf6aa15bdbe911080d257a8e1b88e21e8a47c681af86d6210264b3b8d171f926bd388f8dd51c3384c96aa4862808097666f7ab5fc3799f/410d551e8d2fba2423ed59110472b242/asd.
Source File: http://www.twimbow.com/js/TwitterUsersLoader.js
Line: 634


Expected results:

No error message like Firefox stable 10.0 serious.
p.s I've tried Firefox 10 and Nightly 20120226 build with fresh profile.
Thanks.
Comment 1 Jason Duell [:jduell] (needinfo me) 2012-02-28 19:15:13 PST
This is a server bug--but we might want to change our behavior to make these bugs less likely.  

Our opening handshake for the HTTP connect includes

  Connection: keep-alive, Upgrade

while Chrome uses 

  Connection: Upgrade

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).

So not our fault.  But that said, is there any benefit to sending keep-alive?  I'm not even clear what it means in an HTTP/1.1 context, where connections are persistent by default.   Patrick, did you include keep-alive intentionally?

Also, we could potentially trim down our websocket handshake requests: chrome is sending only Connection, Host, Origin, Sec-WebSocket-{Key|Version}, Upgrade, and if needed, Cookie and Sec-WebSocket-Protocol.  I.e. only the headers required by the WS spec.   We also send Accept, Accept-{Encoding|Language}, Pragma: No-Cache, and User-Agent (I'm a bit surprised Chrome isn't sending User-Agent: I guess they're optimistic that servers won't need to work around UA differences).  This means (factoring out the GET and Cookie header, which obviously vary by request) that chrome has a baseline of 171 bytes of headers vs 448 for us.   As long as cookies and the request URI aren't too long, it's still one packet, so this isn't a big deal.  But if we want to trim, now would be a good time, while WS are still rolling out.

Reassigning to evangelism--I've emailed twimbow with a link to this bug (hi twimbow!).
Comment 2 Jason Duell [:jduell] (needinfo me) 2012-02-28 19:16:03 PST
Hmm, I guess Afrikaans isn't the right language for our evangelism here :)
Comment 3 Knight00931 2012-02-29 03:17:30 PST
Thanks for the reply, so just wait for twimbow.com confirms this and then modify connection behavior in Nightly?
Comment 4 Patrick McManus [:mcmanus] 2012-02-29 06:35:43 PST
We're not going to change firefox connection behavior.

The whole point of basing the websockets handshake on http is to reuse that code and infrastructure, which is what is going on here... this is a bog standard http request, and if it fails to bootstrap websockets it might still result in a valid HTTP connection for other HTTP purposes.

This is a server side bug - it should be fixed on the server side.
Comment 5 Patrick McManus [:mcmanus] 2012-02-29 06:36:49 PST
jason - thanks for digging into it!
Comment 6 Jason Duell [:jduell] (needinfo me) 2012-02-29 11:50:23 PST
Twimbow got back to me, and this issue will be fixed on their end soon.
Comment 7 Jason Duell [:jduell] (needinfo me) 2012-02-29 12:13:35 PST
Hmm, this may be an issue generally with all node.js servers.  Reopening while I verify.
Comment 8 Jason Duell [:jduell] (needinfo me) 2012-02-29 15:53:42 PST
It is a node.js issue, and they're on it:

https://github.com/joyent/http-parser/issues/99
Comment 9 Knight00931 2012-03-16 22:52:46 PDT
Sorry to bother you again, I know that I shouldn't reply here.
But the last changes were 2 weeks ago, have them fixed it?Thanks.
https://github.com/joyent/http-parser/pull/100
Comment 10 Knight00931 2012-04-06 15:54:51 PDT
Seems there are no more news about thier http-parser.
sigh.

Note You need to log in before you can comment on or make changes to this bug.