rack icon indicating copy to clipboard operation
rack copied to clipboard

Does round-tripping cookie names work as expected?

Open ioquatix opened this issue 2 years ago • 2 comments

While working through the cookie utils methods adding documentation and examples, I noticed the following:

Rack::Utils.set_cookie_header("a&b", ":)")
# => "a%26b=%3A%29"

cookies = Rack::Utils.parse_cookies_header("a%26b=%3A%29")
# => {"a%26b"=>":)"}

Rack::Utils.set_cookie_header("a%26b", ":)")
# => "a%2526b=%3A%29"

URL encoding the cookie name on the way to the client but not on the way back from the client means that if a server application tries to set the same cookie based on the incoming name, it will have it's percent escapes encoded again, never reaching a fixed point/symmetry.

Not sure if this is the desired behaviour or not. I know there is a security issue with decoding cookies which are URL encoded. Just wondering if we've made the right trade off here.

Looking at the relevant RFCs:

         CTL            =  %x00-1F / %x7F
                                ; controls
       token          = 1*<any CHAR except CTLs or separators>
       separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT
 set-cookie-header = "Set-Cookie:" SP set-cookie-string
 set-cookie-string = cookie-pair *( ";" SP cookie-av )
 cookie-pair       = cookie-name "=" cookie-value
 cookie-name       = token

Maybe rather than general URL encoded cookie key names, we should either:

  1. Reject any cookie key that doesn't match the token pattern, or
  2. Have a token specific encoded form which encodes and decodes only CTLs or separators as defined by the parser.

Sorry to get pedantic about this but I'm a little concerned that URL encoding cookie keys is probably not the right behaviour and maybe we should follow the RFCs here more closely.

ioquatix avatar Apr 09 '22 23:04 ioquatix

So thinking this out a bit more, the following are valid cookie key characters:

!#$%&'*+-.^_`|~

And should not be encoded, but they are:

Rack::Utils.set_cookie_header("!#$%&'*+-.^_`|~", ":)")
# => "%21%23%24%25%26%27*%2B-.%5E_%60%7C%7E=%3A%29"

This means that if someone writes:

headers = {'set-cookie' => '~=~'}
Rack::Utils.set_cookie_header!(headers, '~', '~')
=> ["~=~", "%7E=%7E"]

This will result in the browser storing two distinct cookies. Obviously an edge case but an edge case non-the-less.

ioquatix avatar Apr 09 '22 23:04 ioquatix

Just encountered this today in a situation wherein cookie keys included # as a separator for additional namespacing. This was fine when only created & parsed in JS, but but they are overzealously escaped when coming from Rack, breaking compatibility.

Somewhat related (though maybe should file a different issue) is the use of Rack::Utils.escape to encode the value portion. It uses URI.encode_www_form_component under the hood and produces + for whitespace.

Would Rack::Utils.escape_path instead be better? I haven't verified, but I wonder if it would align with JS encodeURI() or encodeURIComponent().

robinetmiller avatar Apr 18 '23 03:04 robinetmiller