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 #571 - clarify behaviour of Cookie.setAttribute(String, String) #572

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

markt-asf
Copy link
Contributor

No description provided.

Copy link

@joakime joakime left a comment

Choose a reason for hiding this comment

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

Simplify the verbiage a bit, make it more generic, yet still spec compliant.

api/src/main/java/jakarta/servlet/http/Cookie.java Outdated Show resolved Hide resolved
api/src/main/java/jakarta/servlet/http/Cookie.java Outdated Show resolved Hide resolved
@markt-asf
Copy link
Contributor Author

This does change the internal representation of the HttpOnly and Secure attributes but I think it is a change worth making as it provides us with a more generic solution for any similar attributes rather than relying on special handling within the container.

I want to allow time for feedback before merging since there is a behaviour change. But we are also short of time as we need to get the release out this month. I'll leave this as long as I can which probably means ~1 week as I have a bunch of TCK failures I need to work through (most likely configuration issues on my part rather than TCK bugs) and there are the updates / additions to the TCK tests for this release.

@markt-asf markt-asf mentioned this pull request Mar 20, 2024
@volosied
Copy link

@markt-asf Please include this in 6.1, if possible. Thank you

@markt-asf markt-asf merged commit 7faa086 into jakartaee:master Mar 26, 2024
2 checks passed
@markt-asf markt-asf deleted the fix-issue-571 branch March 26, 2024 16:26
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.

3 participants