Repository navigation
CXF-9072: Support the SameSite attribute in NewCookieHeaderProvider - #3616
Merged
Merged
Conversation
fromString treated any attribute it did not recognize, such as SameSite, as the cookie's name=value pair, so "foo=bar;SameSite=Lax" was parsed as a cookie named "SameSite". The cookie's own name=value pair now comes first and later unrecognized attributes are ignored. SameSite is parsed case-insensitively into NewCookie.SameSite, an unrecognized SameSite value is ignored, and toString writes the attribute so it survives a round trip through Response.getCookies().
With the first name=value pair now taken as the cookie, an empty leading segment such as " ; foo=bar" would have become a cookie with an empty name.
Member
All good, thanks @sharanggupta ! |
coheigea
requested changes
Oct 22, 2026
coheigea
left a comment
coheigea
left a comment
Contributor
There was a problem hiding this comment.
The comment in the code says “the cookie’s own name=value pair comes first”, but the code doesn’t actually do that. Any pair whose name matches a known attribute is still treated as that attribute, even when it’s the first segment. Under RFC 6265 §5.2, the first segment is always the cookie’s name=value, whatever the name is. Two cases show the difference:
A cookie named after an attribute (path=abc; Path=/, or version=2): this is still parsed as attributes and then rejected with IllegalArgumentException. That was already true before this PR.
A cookie named SameSite (SameSite=Lax with nothing else): it used to parse as a cookie called SameSite. With this PR it throws. This is a small new regression, though a real cookie with that name is unlikely.
The simplest fix is to take the first non-empty segment as name=value unconditionally, then run the attribute switch only on the segments after it. That makes the code match its comment and RFC 6265, and it closes both cases above.
Per RFC 6265 section 5.2 the first segment is the cookie's name=value pair whatever its name, and only the segments after it are attributes. A cookie named after an attribute (path=abc; Path=/) or named SameSite was parsed as attributes and rejected as malformed. A first segment without '=' is still rejected.
Contributor
Author
|
Thanks, you're right. Done in f616912: the first non-empty segment is now always the cookie's name=value pair, and only the segments after it go through the attribute switch. |
coheigea
approved these changes
Oct 22, 2026
reta
reviewed
Oct 22, 2026
Replace the switch with StringUtils.capitalize on the lower-cased enum name. Lower-case with Locale.ROOT: under a Turkish default locale, STRICT would otherwise become "Strıct" with a dotless i.
reta
approved these changes
Oct 22, 2026
reta
pushed a commit
that referenced
this pull request
Oct 22, 2026
…3616) * CXF-9072: Support the SameSite attribute in NewCookieHeaderProvider fromString treated any attribute it did not recognize, such as SameSite, as the cookie's name=value pair, so "foo=bar;SameSite=Lax" was parsed as a cookie named "SameSite". The cookie's own name=value pair now comes first and later unrecognized attributes are ignored. SameSite is parsed case-insensitively into NewCookie.SameSite, an unrecognized SameSite value is ignored, and toString writes the attribute so it survives a round trip through Response.getCookies(). * CXF-9072: Skip empty Set-Cookie segments With the first name=value pair now taken as the cookie, an empty leading segment such as " ; foo=bar" would have become a cookie with an empty name. * CXF-9072: Always take the first Set-Cookie segment as the cookie Per RFC 6265 section 5.2 the first segment is the cookie's name=value pair whatever its name, and only the segments after it are attributes. A cookie named after an attribute (path=abc; Path=/) or named SameSite was parsed as attributes and rejected as malformed. A first segment without '=' is still rejected. * CXF-9072: Write the SameSite value with StringUtils.capitalize Replace the switch with StringUtils.capitalize on the lower-cased enum name. Lower-case with Locale.ROOT: under a Turkish default locale, STRICT would otherwise become "Strıct" with a dotless i. (cherry picked from commit 663cf11)
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
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.
https://issues.apache.org/jira/browse/CXF-9072
NewCookieHeaderProvider.fromStringtreats every attribute it doesn't recognize as the cookie's name=value pair, sofoo=bar;SameSite=Laxis parsed as a cookie namedSameSitewith valueLax.toStringdrops SameSite, so aNewCookiebuilt withsameSite(...)loses the attribute in the Set-Cookie header and inResponse.getCookies().Changes:
SameSiteis parsed case-insensitively intoNewCookie.SameSite. An unrecognized value is ignored.toStringwritesSameSite=Strict|Lax|None.Tests: new
NewCookieHeaderProviderTestcases cover parsing, case-insensitivity, unknown SameSite values, unknown attributes,toString, empty segments, and a round trip. The SameSite and unknown-attribute cases fail without the change.mvn -pl rt/frontend/jaxrs -am testwithNewCookieHeaderProviderTest,CookieHeaderProviderTestandResponseImplTestpasses, including checkstyle and PMD.The JIRA issue is assigned to @reta. I'm happy to close this if you already have a fix in progress.