handle cookies with %uXXXX encoding as sent by IE #360

Closed
wants to merge 1 commit into
from

Projects

None yet

7 participants

@mreinsch
mreinsch commented Mar 9, 2012

We are seeing several 500 errors for some incoming requests from IE9 users. Issue there was a cookie set by mixpanel using a different encoding, i.e. %uXXXX to represent UTF-16 characters.

This is one of the exceptions for instance:

ArgumentError (cannot parse Cookie header: invalid %-encoding (%7B%22all%22%3A%20%7B%22%24initial_referrer%22%3A%20%22http%3A//www.akakura365.net/%u30AD%u30E3%u30CA%u30EB%u30B3%u30FC%u30C8petit/%22%2C%22%24initial_referring_domain%22%3A%
20%22www.akakura365.net%22%7D%2C%22events%22%3A%20%7B%7D%2C%22funnels%22%3A%20%7B%7D%7D))

I added some code which recodes %uXXXX to UTF-8 representation so that the cookie can be parsed correctly and thus the request can be processed.

@morenocarullo

+1. I also occurred in this issue.

On this, I looked at the RFC 2109, and it doesn't says that cookies are www-form-encoded in any kind.

@chneukirchen
Member

I have never seen this syntax before, where is it specified?

@mreinsch
mreinsch commented Mar 9, 2012

sorry, I don't know if there is any specification, I'm just working with the data we observe.

Anyway, the default behavior (throwing an exception, causing a 500) gives a pretty bad user experience in case something manages to set a malformed cookie...

@morenocarullo

@chneukirchen , as I replied to @mreinsch, the fact is that you should threat cookies as black boxes.

In my case for example we are working on a Ruby application that is reverseproxied on the same domain of an ASP.NET application, and some cookie values similar to @mreinsch's raise a 500 exception on the cookie jar deserialization code.

@gioele
Contributor
gioele commented Mar 9, 2012

@morenocarullo we should look at RFC 6265, but it basically says the same.

The problem is that Rack cookies values are escaped in set_cookie code (so that you can throw at it strings with any characters inside) and unescaped in cookies (so you can read back the strange string you set before) through this code

string = @env["HTTP_COOKIE"]
....
Utils.parse_query(string, ';,') ...
@morenocarullo

Any progress on this?

For now I have written a middleware (https://gist.github.com/2413136) that attempts to throw away unwanted chars from the cookie string, but of course is not the best option I think.

The problem arises for example when external resources (e.g. javascript) set cookie values we can't control.

@gioele, Yes -- the problem is that Rack assumes that all visible cookies have been previously set by Rack itself, that it's not always the case.

It seems that it's a pending issue, see #272 and #225.

Maybe some core Rack developers (@chneukirchen, @tenderlove) can say something on this?

@mig-hub
Contributor
mig-hub commented Dec 19, 2012

I've got the exact same problem using simpleCart.js
I read the cookie that is made through javascript and get a %-encoding issue.
Apparently @raggi 's solution was not merged because of a security issue.

Did you guys found a quick and dirty fix in the meantime?

@raggi
Member
raggi commented Dec 29, 2012

This is caused by use of escape() from ECMA-262 (javascript). This function is not a URI escape, but actually a javascript specific escape function, that is not compatible with URI for characters outside the BMP.

@raggi raggi added a commit that closed this pull request Dec 29, 2012
@raggi raggi Do not fail on cookies that are not URI escaped
 * Closes #360
8adaab4
@raggi raggi closed this in 8adaab4 Dec 29, 2012
@raggi raggi added a commit that referenced this pull request Jan 4, 2013
@raggi raggi Do not fail on cookies that are not URI escaped
 * Closes #360

Conflicts:
	test/spec_request.rb
9a98b44
@lzap
lzap commented Apr 16, 2014

Now tell me what should I do. Someone set incorrect header for whole enterprise domain *.blah.com and now all Rails apps underneeth fails instanlty. I need at least a workaround. Can you guys give me a snippet that would do it? I do not want to patch our Rack, it looks like you do not want to push this upstream and that would mean I need to keep patching it forever. Thanks.

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