Fix wrong handling of duplicate cookies #5918
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Handling duplicate cookies on server side has no clear specification. But if there is any hint or common-sense about it, it is the "longest path" wins, not the "longest value" wins. Section [5.4](https://www.rfc-editor.org/rfc/rfc6265#section-5.4), subsection 2 has this to say: ``` The user agent SHOULD sort the cookie-list in the following order: Cookies with longer paths are listed before cookies with shorter paths. NOTE: Not all user agents sort the cookie-list in this order, but this order reflects common practice when this document was written, and, historically, there have been servers that (erroneously) depended on this order. ``` though it is not for server side, but it does mentioned the path of cookie matters.
For duplicate cookies that has even same path, use the last value . For example: - Visit https://site1.domain, it sets cookieX=value1, domain=.domain, path=/ - Visit https://site2.subdomain.domain, it sets cookieX=value2, domain=.subdomain.domain, path=/ - Visit https://site2.subdomain.domain again, the Browser will just send cookies with the creation order (because the path are same, browsers have no idea which should be first), i.e., cookieX=value1;cookieX=value2 - Previous implementation of CookieParser will result in the cookieX=value1 being used, then the server side is not satisfied with the cookie value. (Actually in earlier version(2.22.1) it uses cookieX=value2.) Let us use the last cookie value, so both web sites will work, - Visit https://site2.subdomain.domain will use value2 - Visit https://site1.domain will use value1 because the domain of value2 can not be applied to this parent domain.
Contributor
|
Please see our wiki. |
Author
|
Thank you for your reply. This PR is just a suggestion, prompt, hope someone else can work for it. Actually I just wanted to create an issue but it is easier with a PR. |
Contributor
|
Having your PR as a PoC, I've created the #5919 / is it what you expect to be changed? |
Contributor
Contributor
|
closing as #5919 was merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Handling duplicate cookies on server side has no clear specification.
But if there is any hint or common-sense about it, it is the "longest path" wins, instead of currently implemented logic of "longest value" wins.
Section 5.4, subsection 2 has this to say:
though it is not for server side, it does mention how the path of cookie matters.
For duplicate cookies that has even same path, let us use the last value, as have been doing in earlier version (such as 2.22.1).
For example:
Visit https://site1.domain/, it sets cookieX=value1, domain=.domain, path=/
Visit https://site2.subdomain.domain/, it sets cookieX=value2, domain=.subdomain.domain, path=/
Visit https://site2.subdomain.domain/ again, the Browser will just send cookies with the creation order (because the path are same, browsers have no idea which should be first), i.e., cookieX=value1;cookieX=value2
Previous implementation of CookieParser will result in the cookieX=value1 being used, then the server side is not satisfied with the cookie value. (Actually in earlier version(2.22.1) it uses cookieX=value2.)
Let us use the last cookie value, so both web sites will work,