Mojo::Cookie::Response may produce malformed values

dakkar commented 3 years ago

Mojolicious version: 9.19
Perl version: 5.30
Operating system: Gentoo Linux

Steps to reproduce the behavior

$ perl -MMojo::Cookie::Response -E 'say Mojo::Cookie::Response->new(name=>"foo",value=>"foo,bar")->to_string'
foo="foo,bar"

Expected behavior

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie says:

A <cookie-value> can optionally be wrapped in double quotes and include any US-ASCII characters excluding control characters, Whitespace, double quotes, comma, semicolon, and backslash.

Actual behavior

Commas and some other characters cause the cookie value to be enclosed in double quotes, but that's not enough to make them well-formed.

Combined with browser behaviours like https://stackoverflow.com/questions/45985970/safari-cookie-value-strips-space-after-the-commas, it makes some signed cookies become invalid

Maybe cookie values should be url-encoded or something? I'm not sure how to do that in a fully back-compatible way, though.

kraih commented 3 years ago

We follow the RFCs, not Mozilla.

dakkar commented 3 years ago

fair. https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1

cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
 cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

it says the same thing.

adamlounds commented 3 years ago

Came across this bug recently - using post/redirect/get pattern I added a signed cookie for a flash message to display on the result page. If you're using safari and the flash message has a comma in it, it is considered to have a bad signature and does not display (because safari converts a, b to a,b)

kraih commented 3 years ago

Flash messages are stored in the session, which is Base64 encoded and therefore cannot contain commas.

adamlounds commented 3 years ago

Sorry for confusion: I'm not using Mojolicious::Sessions, just a standard signed cookie via $c->signed_cookie. I could base64-encode the data manually, but for now I just rephrased the message to omit the comma.

mojolicious / mojo