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

Fix #3433 - Multiple cookies with same name are not supported #4390

Merged
merged 6 commits into from
Feb 18, 2020
Merged

Fix #3433 - Multiple cookies with same name are not supported #4390

merged 6 commits into from
Feb 18, 2020

Conversation

tvallin
Copy link
Contributor

@tvallin tvallin commented Feb 7, 2020

Signed-off-by: tvallin thibault.vallin@oracle.com

NewCookie :
- pick the latest expiration date
- if same date, take the longest path string

Cookie :
- pick the longest string

Signed-off-by: tvallin <thibault.vallin@oracle.com>
@tvallin tvallin requested a review from jansupol February 7, 2020 14:23
Signed-off-by: tvallin <thibault.vallin@oracle.com>
@tvallin tvallin changed the title Fix 3433 - Multiple cookies with same name are not supported Fix #3433 - Multiple cookies with same name are not supported Feb 12, 2020
*/
public static NewCookie getPreferedNewCookie(NewCookie one, NewCookie second) {

if (!one.getExpiry().equals(second.getExpiry())){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expiry date can be NULL. Isn't it better to use maxAge value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a difference between expiry and maxAge.
IE does not support maxAge, so most of the servers set expiry rather than maxAge. It is possible to set one or the other.
NewCookie is what comes from the server, and even though maxAge is recommended, expires seems to be more often. But it seems to be one or the other, so we should compare the later from both.

But yes, we should check for NULL, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well ,regarding check of expiry VS max-age it's good to check them both :)

Regarding setting one or another attribute in request - actually server shall put both. For now I've found that the only browser which does not support max-age attribute is IE.

Per RFC 6265 max-age attribute has precedence over expiry attribute if both are present in request. In my opinion this shall be respected while choosing preferred cookie.

Signed-off-by: tvallin <thibault.vallin@oracle.com>
Signed-off-by: tvallin <thibault.vallin@oracle.com>
Signed-off-by: tvallin <thibault.vallin@oracle.com>
Signed-off-by: tvallin <thibault.vallin@oracle.com>
@tvallin tvallin requested a review from senivam February 18, 2020 08:11
@senivam senivam merged commit e5af7d6 into eclipse-ee4j:master Feb 18, 2020
@senivam senivam added this to the 2.30.1 milestone Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants