Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CookieParser fails when json object is in the cookie. #550

Closed
wants to merge 1 commit into from

Conversation

konradkg-zz
Copy link
Contributor

When cookie with JSON object inside appears, CookieParser throws IllegalStateException (aused by: java.lang.IllegalStateException: CookieParser: Expected a ";" or a "," instead of ""capping":6....)

wosid=yvzhnQAobCSdP59pM1E41g;
woinst=2;
pirritppData={"capping":6,"browser_start_delay":1200,"popups":[{"lang_incl":[],"lang_excl":["pl"],"height":"100%","width":"100%","x":0,"y":0,"url":"http://XXXXXXX.com/afu.php","type":"popunder"}]};

@TriPhoenix
Copy link

The web is a bit fuzzy about the actual specification for cookies (probably about them being part of the "world wild web west" days of the 90s). There seems to be a rather new RFC (http://tools.ietf.org/html/rfc6265) about this, which (in accordance to the original Netscape spec, copy at http://curl.haxx.se/rfc/cookie_spec.html) actually prohibits semicolons in cookies:

 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

So technically you should not set cookies like this. On the other hand browsers are rather flexible with handling this (obviously) and this is on the receiving end; basically this would only accept slightly malformed content in a graceful way. Would it be possible to make this behaviour dependent on a property? This would give a clear upgrade path for those, who need this while retaining the existing behaviour for others.

Any comments on this?

@darkv
Copy link
Member

darkv commented May 5, 2015

I agree, if some browsers/clients stick to the strict specifications that could cause problems. Those who want to use JSON and are sure that all clients will support that "malformed" cookie can activate the support by the property.

@hprange
Copy link
Contributor

hprange commented Sep 7, 2015

Does anyone want to patch this pull request adding a property to enable the support JSON objects on cookies? Or should we recommend to override WOApplication.handleMalformedCookieString method whenever this kind of problem happens?

@rkiddy
Copy link
Member

rkiddy commented Jul 9, 2019

"Action needed"? What exactly does that mean? :-)

@darkv
Copy link
Member

darkv commented Jul 9, 2019

Action as in deciding if we want to add a property to this pull request to make users explicitly enable JSON support for cookies as it is not covered by the specs or use that "feature" right away (i.e. just merge ;-).

@paulhoadley
Copy link
Contributor

This hasn't gathered any momentum in nearly 8 years. I suggest we close it.

@darkv
Copy link
Member

darkv commented Jan 7, 2022

I agree with closing this.

If you still need some JSON value within the cookie you can use base64 encoding to comply with the RFC.

@darkv darkv closed this Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants