pillarjs / cookies

Signed and unsigned cookies based on Keygrip
MIT License
1.29k stars 152 forks source link

Strip quotes for cookie values for .get() #140

Closed naruaway closed 2 years ago

naruaway commented 3 years ago

According to RFC 2965, cookie value can be "quoted-string" and in practice, some web framework (e.g. Java Spring) sometimes uses quoted value. In such cases, web browser will send HTTP headers like Cookie: my_value_a=x; my_value_b="abc:def:xyz"; to the server and it will be more developer friendly to automatically strip quotes so that cookie.get('my_value_b') returns abc:def:xyz rather than "abc:def:xyz".

Is this this library's responsibility?

I guess so. I checked the behavior of Django and Express and both of them strip quotes under the hood.

What's the impact of this change?

koa is relying on this library. After this fix, koa can get the same behavior as Django and Express, which seems to be more reasonable and common behavior. I created an issue for koa for the discussion in koa side.

Note that this change will be breaking change so we need to bump up the major version of the library.

naruaway commented 2 years ago

@dougwilson Thank you so much! If possible, could you publish the new version? It looks like the latest one is still 0.8.0, published 3 years ago :pray:

dougwilson commented 2 years ago

Hi @naruaway apologies, I didn't realize I never made a release πŸ˜‚ I'll get it out today/tomorrow πŸ‘

naruaway commented 2 years ago

Hi @dougwilson, thanks for the quick response! Are you going to release patch or minor, or major?

Strictly speaking this could be "breaking change" but on the other hand we could say this is a bug fix πŸ€” One benefit of releasing this as a patch version would be, koa will get this change automatically without changing any koa code (they are specifying ~0.8.0)

My personal feeling is that patch version would be fine (I think no one should rely on the previous behavior considering the behavior of other frameworks like Django...) but it's your call of course πŸ™

dougwilson commented 2 years ago

Hi @naruaway this is not the only change in the release. It will be included in the 0.9.0 release.

naruaway commented 2 years ago

Hi @dougwilson, oh right, it makes sense. Thanks for all your work πŸ™

naruaway commented 2 years ago

Hi @dougwilson, are you going to release 0.9.0? Sorry for pinging you but a dev from koa mentioned that they will bump up the version of cookies after 0.9.0 gets released