"invalid %-encoding" error leaves rack.request.cookie_hash == nil #225

Closed
codesnik opened this Issue Aug 23, 2011 · 21 comments

Comments

Projects
None yet
@codesnik

this causes a problem in rails: rails/rails#2622

I'm not really sure, which side should be fixed, and how. Should rails catch that exception? then probably using plain ArgumentError isn't specific enough.

any ideas?

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

Is the issue that Rack::Request#cookies returns nil, if so, I cannot tell from the code why. Or is it really that env['rack.request.cookie_hash'] is nil? In that case, I would consider it a Rails bug, since, you know, that's an implementation detail Rails should not rely on (it should rather use Rack::Request#cookies).

Member

rkh commented Aug 23, 2011

Is the issue that Rack::Request#cookies returns nil, if so, I cannot tell from the code why. Or is it really that env['rack.request.cookie_hash'] is nil? In that case, I would consider it a Rails bug, since, you know, that's an implementation detail Rails should not rely on (it should rather use Rack::Request#cookies).

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

Sorry, understood your issue now. What exception should we raise? I think it can be fixed by setting env["rack.request.cookie_string"] after the hash has been generated (thus not setting it when an exception is raised).

Member

rkh commented Aug 23, 2011

Sorry, understood your issue now. What exception should we raise? I think it can be fixed by setting env["rack.request.cookie_string"] after the hash has been generated (thus not setting it when an exception is raised).

@codesnik

This comment has been minimized.

Show comment
Hide comment
@codesnik

codesnik Aug 23, 2011

@rkh, I don't know, looks like a legit solution to me.

and about exception, I imagined something like class CookieEncodingError < ArgumentError, it would explain better what happened and easier to "rescue". what do you think?
though, error's raised from the same code that parses URI-s, so it could be not that straighforward to do.

@rkh, I don't know, looks like a legit solution to me.

and about exception, I imagined something like class CookieEncodingError < ArgumentError, it would explain better what happened and easier to "rescue". what do you think?
though, error's raised from the same code that parses URI-s, so it could be not that straighforward to do.

@codesnik codesnik closed this Aug 23, 2011

@codesnik codesnik reopened this Aug 23, 2011

@rkh rkh closed this in 84a9d3e Aug 23, 2011

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

I will give the exception issue a thought.

Member

rkh commented Aug 23, 2011

I will give the exception issue a thought.

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

Could you try running your Rails setup (where the issue surfaced) against Rack master?

Member

rkh commented Aug 23, 2011

Could you try running your Rails setup (where the issue surfaced) against Rack master?

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

Also, it would have been possible to rescue the exception and return an empty hash (or all the cookies we've got so far). However, this decision should be left to the framework.

Member

rkh commented Aug 23, 2011

Also, it would have been possible to rescue the exception and return an empty hash (or all the cookies we've got so far). However, this decision should be left to the framework.

@codesnik

This comment has been minimized.

Show comment
Hide comment
@codesnik

codesnik Aug 23, 2011

yes. I've patched rails HEAD's actionpack.gemspec for using rack version 1.4.0.
rails now show much saner exception: "ArgumentError: invalid %-encoding (%)"

still, without disabling rails backtrace silencer, it's hard to guess, where it comes from. Subclassing of ArgumentError could help.

yes. I've patched rails HEAD's actionpack.gemspec for using rack version 1.4.0.
rails now show much saner exception: "ArgumentError: invalid %-encoding (%)"

still, without disabling rails backtrace silencer, it's hard to guess, where it comes from. Subclassing of ArgumentError could help.

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Aug 23, 2011

Member

Maybe, but we would have to rescue the exception and raise a new one (we don't raise the exception, it's raised by the uri lib that ships with Ruby).

Member

rkh commented Aug 23, 2011

Maybe, but we would have to rescue the exception and raise a new one (we don't raise the exception, it's raised by the uri lib that ships with Ruby).

@andrewtheis

This comment has been minimized.

Show comment
Hide comment
@andrewtheis

andrewtheis Nov 1, 2011

So I'm experiencing the error mentioned (Rails 3.1):

ArgumentError (cannot parse Cookie header: invalid %-encoding (79330568.1320172650.8.2.utmcsr=Net%2BSolutions%2B|utmccn=Leads|utmcmd=PPC|utmctr=Mobile%20App%20Developer|utmcct=World%20Renown%20Developer%iPhone)):

What is happening is Google Analytics is storing campaign tracking information as a cookie, but Rails appears to not understand the %2B (+ sign) when the Browser sends the cookies back - Causing any Rails application to not only fail, but return an error on every subsequence request unless the user clears their cookies.

Any way to handle this error gracefully? Ideally Rails should be able to handle cookies with the + symbol.

So I'm experiencing the error mentioned (Rails 3.1):

ArgumentError (cannot parse Cookie header: invalid %-encoding (79330568.1320172650.8.2.utmcsr=Net%2BSolutions%2B|utmccn=Leads|utmcmd=PPC|utmctr=Mobile%20App%20Developer|utmcct=World%20Renown%20Developer%iPhone)):

What is happening is Google Analytics is storing campaign tracking information as a cookie, but Rails appears to not understand the %2B (+ sign) when the Browser sends the cookies back - Causing any Rails application to not only fail, but return an error on every subsequence request unless the user clears their cookies.

Any way to handle this error gracefully? Ideally Rails should be able to handle cookies with the + symbol.

@lawrencepit

This comment has been minimized.

Show comment
Hide comment
@lawrencepit

lawrencepit Nov 9, 2011

Contributor

I think this is a bug in rack for sure. The cookie value I receive is e.g. %u4E2D. This is a valid encoding for the Chinese character . However, in rack/backports/uri/common.rb method decode_www_form_component it will raise the ArgumentError because it does not recognize the form %uXXXX where XXXX is the hex unicode value.

Update: is %u4E2D a valid cookie value encoding? I assumed it was, but can't find docs on the semantics of cookies values. (RFC 6265 isn't too clear I find)

Contributor

lawrencepit commented Nov 9, 2011

I think this is a bug in rack for sure. The cookie value I receive is e.g. %u4E2D. This is a valid encoding for the Chinese character . However, in rack/backports/uri/common.rb method decode_www_form_component it will raise the ArgumentError because it does not recognize the form %uXXXX where XXXX is the hex unicode value.

Update: is %u4E2D a valid cookie value encoding? I assumed it was, but can't find docs on the semantics of cookies values. (RFC 6265 isn't too clear I find)

@codesnik

This comment has been minimized.

Show comment
Hide comment
@codesnik

codesnik Nov 9, 2011

@lawrencepit quoting wikipedia: ( http://en.wikipedia.org/wiki/Url_encoding )

The digits, preceded by a percent sign ("%"), are then used in the URI in place of the reserved character. (For a non-ASCII character, it is typically converted to its byte sequence in UTF-8, and then each byte value is represented as above.)

...

There exists a non-standard encoding for Unicode characters: %uxxxx, where xxxx is a Unicode value represented as four hexadecimal digits. This behavior is not specified by any RFC and has been rejected by the W3C. The third edition of ECMA-262 still includes an escape(string) function that uses this syntax, but also an encodeURI(uri) function that converts to UTF-8 and percent-encodes each octet.

codesnik commented Nov 9, 2011

@lawrencepit quoting wikipedia: ( http://en.wikipedia.org/wiki/Url_encoding )

The digits, preceded by a percent sign ("%"), are then used in the URI in place of the reserved character. (For a non-ASCII character, it is typically converted to its byte sequence in UTF-8, and then each byte value is represented as above.)

...

There exists a non-standard encoding for Unicode characters: %uxxxx, where xxxx is a Unicode value represented as four hexadecimal digits. This behavior is not specified by any RFC and has been rejected by the W3C. The third edition of ECMA-262 still includes an escape(string) function that uses this syntax, but also an encodeURI(uri) function that converts to UTF-8 and percent-encodes each octet.

@atd

This comment has been minimized.

Show comment
Hide comment
@atd

atd Nov 14, 2011

We are experiencing the same problem with google analytics and UTF-8 encoding.

Should rack support this syntax or it is an analytics issue?

atd commented Nov 14, 2011

We are experiencing the same problem with google analytics and UTF-8 encoding.

Should rack support this syntax or it is an analytics issue?

@lawrencepit

This comment has been minimized.

Show comment
Hide comment
@lawrencepit

lawrencepit Nov 15, 2011

Contributor

Based on @codesnik quote above, GA probably uses a non-standard encoding for unicode characters.

I'd consider that a bug in their code. I had a similar issue with another hosted 3rd party javascript which does something similar as the GA javascript functions. They used the javascript function escape(), instead of encodeURIComponent(). They fixed it, and now it works fine for me. I think the GA case is probably the same.

On the other hand, one could argue that http servers such as rack SHOULD be tolerant about what is acceptable input. As paragraph 19.3 of the HTTP1.1 spec states: "We therefore recommend that operational applications be tolerant of deviations whenever those deviations can be interpreted unambiguously."

Contributor

lawrencepit commented Nov 15, 2011

Based on @codesnik quote above, GA probably uses a non-standard encoding for unicode characters.

I'd consider that a bug in their code. I had a similar issue with another hosted 3rd party javascript which does something similar as the GA javascript functions. They used the javascript function escape(), instead of encodeURIComponent(). They fixed it, and now it works fine for me. I think the GA case is probably the same.

On the other hand, one could argue that http servers such as rack SHOULD be tolerant about what is acceptable input. As paragraph 19.3 of the HTTP1.1 spec states: "We therefore recommend that operational applications be tolerant of deviations whenever those deviations can be interpreted unambiguously."

@cailinanne

This comment has been minimized.

Show comment
Hide comment
@cailinanne

cailinanne Nov 29, 2011

If you are looking for an immediate workaround solution and you are using Apache, you can use the Apache Headers module to edit the problematic cookie before it is passed on to Rack. To do this:

  1. Enable the headers module (sudo a2enmode)
  2. Add the following to the your Apache config
RequestHeader edit Cookie "You've%20Got%20%%BALANCE%%!%20" "BALANCE"

or, more generically

RequestHeader edit Cookie "problem_value" "new_value"

The specific example cleans up one particular campaign that was driving me insane, but I believe that both problem_value and new_value could be more generic regular expressions as well.

See full documentation in the RequestHeader directive section here: http://httpd.apache.org/docs/current/mod/mod_headers.html

If you are looking for an immediate workaround solution and you are using Apache, you can use the Apache Headers module to edit the problematic cookie before it is passed on to Rack. To do this:

  1. Enable the headers module (sudo a2enmode)
  2. Add the following to the your Apache config
RequestHeader edit Cookie "You've%20Got%20%%BALANCE%%!%20" "BALANCE"

or, more generically

RequestHeader edit Cookie "problem_value" "new_value"

The specific example cleans up one particular campaign that was driving me insane, but I believe that both problem_value and new_value could be more generic regular expressions as well.

See full documentation in the RequestHeader directive section here: http://httpd.apache.org/docs/current/mod/mod_headers.html

@atd

This comment has been minimized.

Show comment
Hide comment
@atd

atd Nov 30, 2011

Thank you Cailin!

Unfortunatelly, that solution does not work for us, as the encoded "problem values" are related to utf-8 encoded personal names, and do not follow a pattern

atd commented Nov 30, 2011

Thank you Cailin!

Unfortunatelly, that solution does not work for us, as the encoded "problem values" are related to utf-8 encoded personal names, and do not follow a pattern

@codesnik

This comment has been minimized.

Show comment
Hide comment
@codesnik

codesnik Nov 30, 2011

@atd
RequestHeader directive accepts regexes. But I don't know if they're "/g"
try something along the lines:

RequestHeader edit Cookie "%u(..)(..)" "%\1%\2"

(I'm not sure about syntax, either)

@atd
RequestHeader directive accepts regexes. But I don't know if they're "/g"
try something along the lines:

RequestHeader edit Cookie "%u(..)(..)" "%\1%\2"

(I'm not sure about syntax, either)

@swrobel

This comment has been minimized.

Show comment
Hide comment
@swrobel

swrobel Aug 9, 2012

Was this actually fixed? If there was a fix, is there an official release of rack coming soon with the fix included?

swrobel commented Aug 9, 2012

Was this actually fixed? If there was a fix, is there an official release of rack coming soon with the fix included?

@alienlifeform

This comment has been minimized.

Show comment
Hide comment
@alienlifeform

alienlifeform Nov 25, 2012

Any word on this? Omniture analytics also faces this problem, at least in the version we're using for a client.

Any word on this? Omniture analytics also faces this problem, at least in the version we're using for a client.

uxp pushed a commit to uxp/rack that referenced this issue Oct 23, 2013

@wheagle

This comment has been minimized.

Show comment
Hide comment
@wheagle

wheagle Feb 11, 2014

Same problem here in 2014 on thedaywefightback, Feb 11th. thedaywefightback.org embeds a cookie with the same problem. cailinanne's workaround worked for me. FYI.

Specifically:

RequestHeader edit Cookie "(^thedaywefightback=[^;]*; |; thedaywefightback=[^;]*)" ""

wheagle commented Feb 11, 2014

Same problem here in 2014 on thedaywefightback, Feb 11th. thedaywefightback.org embeds a cookie with the same problem. cailinanne's workaround worked for me. FYI.

Specifically:

RequestHeader edit Cookie "(^thedaywefightback=[^;]*; |; thedaywefightback=[^;]*)" ""
@mpapper

This comment has been minimized.

Show comment
Hide comment
@mpapper

mpapper May 27, 2014

I see this problem in rack too. does anyone have a monkey patch I could put into my rails (3.1.1X) server?

mpapper commented May 27, 2014

I see this problem in rack too. does anyone have a monkey patch I could put into my rails (3.1.1X) server?

@eirc

This comment has been minimized.

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