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

Multiple cookies can have the same name #1458

Merged
merged 3 commits into from Sep 6, 2019

Conversation

sgenoud
Copy link

@sgenoud sgenoud commented Feb 14, 2019

Due to my own errors I ended up in a situation where two cookies had the same name. With a little bit of googling I realised this was something that is part of the RFC (linked in this stackoverflow answer).

But the default in Flask (from the defaults of werkzeug) is to support only one cookie with the same name. It is fairly easy to fix in the use code (subclassing the response object), but it feels like the default should follow the RFC.

Just one caveat. The ImmutableMultiDict and the ImmutableTypeConversionDict have different defaults on how they handle update to the same key. In the first case, the first value is returned, while in the second case the last value is returned.

>>> ImmutableMultiDict([['foo', 'bar'], ['foo', 'baz']]).get('foo')
'bar'

>>> ImmutableTypeConversionDict([['foo', 'bar'], ['foo', 'baz']]).get('foo')
'baz'

This might break some people's code (even if they should not have relied on the order of the cookies, according to the RFC). This could be changed easily, but make the code slightly more complex (and arcane).

@sgenoud sgenoud force-pushed the feature/same-name-cookies branch 2 times, most recently from 984d398 to 50e3867 Compare Feb 14, 2019
@davidism
Copy link
Member

@davidism davidism commented Mar 8, 2019

I've applied some global code style cleanup and changes, and as a result this PR will no longer merge. Would you mind recreating it against the current mater? Sorry for the inconvenience.

@sgenoud sgenoud force-pushed the feature/same-name-cookies branch from 50e3867 to e6f368e Compare Mar 9, 2019
@sgenoud
Copy link
Author

@sgenoud sgenoud commented Mar 10, 2019

No problem, I have rebased it!

@davidism davidism force-pushed the feature/same-name-cookies branch from e6f368e to 0ccb76a Compare Sep 6, 2019
@davidism
Copy link
Member

@davidism davidism commented Sep 6, 2019

Extended this to parse_cookie as well, as suggested in #1562.

@davidism davidism force-pushed the feature/same-name-cookies branch from b23f342 to 386fdae Compare Sep 6, 2019
@davidism davidism merged commit 6817cd0 into pallets:master Sep 6, 2019
11 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants