node.js websockets handshake broken with Firefox? #2849

Closed
jduell opened this Issue Feb 29, 2012 · 7 comments

Comments

Projects
None yet
4 participants

jduell commented Feb 29, 2012

Hi there--I'm a developer on the Firefox network stack. We've just seen a bug with a site that couldn't handle Firefox's (legal, but different than Chrome) websockets connection handshake:

https://bugzilla.mozilla.org/show_bug.cgi?id=730742

The site owner has told me they're using node.js, so I'm wondering if this is a general node.js issue. I'd test it myself but I figure someone reading this will know in less time than it'd take me to set up node.js

thanks

Jason Duell
Mozilla

einaros commented Feb 29, 2012

Could you ask him which websocket library he's using? I maintain one of the most up to date libraries (https://github.com/einaros/ws), and I've had no reports of this behavior.

jduell commented Feb 29, 2012

I've emailed him.

I've read the Bugzilla issue, I don't think it's a bug in his websocket library, but in the way node recognizes Upgrade requests.

Code for doing this resides in http-parser. Here you can see that Upgrade header is only matched when header value begins with letter "u". Parser state is later set here, and that's what node detects. We should find out if what you guys are doing is legal (stuffing both keep-alive and upgrade into Connection header) and if it is, open an issue at http-parser.

Edit: oh, right, RFCs say it's legal.

einaros commented Feb 29, 2012

@mmalecki, the letter is lower cased as part of the token matching, so it'll still match.

Granted that you were worried about u vs U that is :)

@einaros No, my point is, their header starts from "k".

Our opening handshake for the HTTP connect includes

  Connection: keep-alive, Upgrade

while Chrome uses 

  Connection: Upgrade

Comment.

einaros commented Feb 29, 2012

I'll have to read the thread when I'm back at a computer. :)

Owner

bnoordhuis commented Feb 29, 2012

Yes, that's a bug in the parser. Continues in joyent/http-parser#99.

bnoordhuis closed this Feb 29, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment