Closed bclozel closed 3 months ago
RFC 6265 allows cookie values to be empty. Given that, I think we should take the following approach:
null
- remove attributeattr=
The above should be what is currently implemented.
Implementations will need to add special handling for attributes like Partioned
and ignore the value when writing the cookie header.
If we want consistency with how we implemented HttpOnly
then the special handling for Partioned
would be "include the attribute name only in the Set-Cookie
header iff Boolan.parseBoolean(attrValue)
returns true
.
Personally, I favour consistency with HttpOnly
.
With the Partition cookies feature being added, I encountered similar problems when I tried to added the Partitioned
attribute via cookie.setAttribute("Partitioned", "")
. My server only rendered it as partitioned=
.
With the propose change, I would need to usecookie.setAttribute("Partitioned", "true")
in order to have Partitioned;
rendered on a cookie?
Just to throw some other ideas out: Is setAttribute trying to do too much here? Should we other method such as setAttributeOnly()
and removeAttribute()
;
Lastly, could a solution for this be included in Servlet 6.1 rather than waiting for another whole release cycle? I'll send out an email in the servlet dev mailing list, too. Thanks!
Per the parsing steps in https://datatracker.ietf.org/doc/html/rfc6265#section-5.2
These are all equivalent Set-Cookie
strings.
Set-Cookie: test=value; Secure; HttpOnly; Partitioned
Set-Cookie: test=value; Secure=; HttpOnly=; Partitioned=
Set-Cookie: test=value; Partitioned=; Secure; HttpOnly=;
Why do we need special handling for Partitioned
? (or HttpOnly
and Secure
for that matter)
Alternatively, knowing the parsing rules, an empty string value used in cookie.setAttribtue("Name", "")
could always produce the attribute without an equals sign on the Set-Cookie
line too.
Thanks for pointing that out @joakime! I didn't realize that.
I saw Mark made a PR: https://github.com/jakartaee/servlet/pull/572/files
The changes look good to me. Could you also take a look? Perhaps it could be in the M2?
175 introduced the support for Cookie attributes in order to generally address the need for attributes such as "SameSite".
Right now, third party cookie restrictions are being rolled out in Google Chrome and many application will need to add the "Partitioned" attribute to keep things working. I have tried to use the attributes API for this purpose and failed:
Set-Cookie: test=value; attribute=attrvalue; Partitioned=
Set-Cookie: test=value; attribute=attrvalue
Set-Cookie: test=value
Note, I have removed the "nullable" attribute in my tests with Tomcat 10.1.17 since it currently fails for
null
attribute values with the following:Could you clarify the cookie attributes behavior for the following cases:
null
value - should this remove the attribute from the map or should it be rejected?Partitioned
case (so no additional=
sign)