parse Cookies Bug about websocket-node HOT 12 CLOSED

Interesting. Can you submit a pull request?

from websocket-node.

Sorry dude, I'm not crazy good with github, or git for that matter. Ill spend some time trying to figure it out over the next week but no promises

from websocket-node.

No problem. It's pretty simple:

1. Fork the repository into your own account
2. Check out your fork, make the necessary changes and push to master.
3. Submit a pull request from the master branch of your fork.

That's it!

Sent from my iPhone

On Aug 26, 2012, at 4:27 PM, roderickObrist [email protected] wrote:

Sorry dude, I'm not crazy good with github, or git for that matter. Ill spend some time trying to figure it out over the next week but no promises

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

from websocket-node.

The fix looks reasonable, it is better written to expect malformed cookie headers. Can you paste the value of the cookie header that's being sent? Perhaps the parsing RegEx is not correct and it's breaking on an otherwise-valid cookie?

from websocket-node.

Well the RegEx.match method will return null when there is no match. So I reckon you need this protection anyway. To be honest I don't have the original cookie anymore, but I don't think it had an "=" in it. Im pretty sure if you just set any random string as the cookie... e.g

document.cookie = Math.random().toString(36);
new WebSocket();

Since there is no = the match will return null.

from websocket-node.

Right, but I don't believe that's actually a valid value for the Cookie header.

Sent from my iPhone

On Sep 2, 2012, at 12:21 AM, roderickObrist [email protected] wrote:

Well the RegEx.match method will return null when there is no match. So I reckon you need this protection anyway. To be honest I don't have the original cookie anymore, but I don't think it had an "=" in it. Im pretty sure if you just set any random string as the cookie... e.g

document.cookie = Math.random().toString(36);
new WebSocket();
Since there is no = the match will return null.

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

from websocket-node.

Do you know what value was being sent for the Cookie header that caused the error in the first place?

from websocket-node.

So I don't know what you mean by 'valid' cookie, I understand the implication of name value pair. I'm not certain if there is a governing standard which is widely accepted.

But, but if I type the previous code in firebug or chrome dev tools, the network tab shows:

Firebug:

GET / HTTP/1.1
Host:---------------------------------
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip, deflate
Connection: keep-alive, Upgrade
Sec-WebSocket-Version: 13
Origin:------------------------------------
Sec-WebSocket-Key: 5lK3Xeto7l6fyOZzTnQPFA==
Cookie: 0f5clh1mrleg
Pragma: no-cache
Cache-Control: no-cache
Upgrade: websocket

Chrome:

Connection:Upgrade
Cookie:2caxr9m6uu40a4i
Host:--------------------------------
Origin:--------------------------------
Sec-WebSocket-Extensions:x-webkit-deflate-frame
Sec-WebSocket-Key:EEjf2aZakFO/m+UVC4bvCA==
Sec-WebSocket-Version:13
Upgrade:websocket
(Key3):00:00:00:00:00:00:00:00

There are all the request headers, valid or not, they are sent. and they break the websocket server... Well they did last time I checked.

Websocket server expects
name=value
Chrome/Firefox sends
value

You can recreate this by opening a blank page from a websocket enabled server. F12 console.

document.cookie = Math.random().toString(36).slice(2); 

window.connectionThatWillThrowWebsocketServer = new Websocket('ws://' + window.location.hostname + '/');

from websocket-node.

Thank you for the additional information. I was under the impression that all cookies had to be in the form of key=value so I'm a bit confused as to why a browser would send a Cookie header that doesn't match that format.

I've applied the patch that you've suggested to the master branch, it'll be included in the next release.

from websocket-node.

I found the relevant RFC. It's RFC 2109. http://www.ietf.org/rfc/rfc2109.txt

According to section the grammer in section 4.1, "Syntax," a value without an "=" is valid, but it represents a key with no value, rather than an unformatted value or a value with no key. Interesting.

from websocket-node.

Ok, just committed a totally new parseCookies function to reflect the grammar in the RFC.

from websocket-node.

Well done dude,

Sorry I didn't get the pull request thing done in time. I saw the patch.
Good stuff, your solution will mean that cookies without an = still get parsed, which is better than my "Ignore it if its useless to me" approach.

Peace out

from websocket-node.