servlet icon indicating copy to clipboard operation
servlet copied to clipboard

TCK for servlet 6.1 servlet/tck/spec/serverpush /ServerPushTests#serverPushCookieTest

Open janbartel opened this issue 1 year ago • 1 comments

https://github.com/jakartaee/servlet/blame/6.1.x/tck/tck-runtime/src/main/java/servlet/tck/spec/serverpush/ServerPushTests.java#L338

This line says that if a cookie with Max-Age <= 0 was added on the server side, a subsequently created PushBuilder should not contain the corresponding Cookie. However, a value of Max-Age=0 indicates to a User-Agent that it should immediately delete a Cookie, but a value of -1 indicates that the Cookie should be considered valid for the duration of the current "session". As the "session" is still clearly ongoing at the point at which the PushBuilder is created, then it should contain the Cookie with Max-Age=-1. Should this test be testing a value of Max-Age=0 instead?

janbartel avatar Jun 20 '24 09:06 janbartel

While I think there is a case to be made for the behaviour you describe, the Javadoc for PushBuilder has the same language

If HttpServletResponse.addCookie(Cookie) has been called on the associated response, then a corresponding Cookie header will be added to the PushBuilder, unless the Cookie.getMaxAge() is<=0, in which case the Cookie will be removed from the builder.

Therefore, I think the current 6.1 TCK code is correct.

Whether we want to address this for Servlet 6.2 is TBD. PushBuilder is already deprecated and is rarely used. There are also backwards compatibility issues. I'm not sure there is much benefit to changing this behaviour.

markt-asf avatar Jun 24 '24 16:06 markt-asf

See #837 for a PR that should address this.

markt-asf avatar Apr 01 '25 17:04 markt-asf

#837 has been merged

markt-asf avatar Apr 11 '25 14:04 markt-asf