search

jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
250 stars 79 forks source link

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

Open janbartel opened 2 weeks ago

janbartel commented 2 weeks ago

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?

markt-asf commented 1 week ago

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.