jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
262 stars 83 forks source link

TCK Challenge: Cookie setMaxAgePositiveTest using incorrect CookiePolicy for Date parsing #471

Closed brideck closed 6 months ago

brideck commented 1 year ago

Challenged Tests: com.sun.ts.tests.servlet.api.jakarta_servlet_http.cookie.URLClient#setMaxAgePositiveTest com.sun.ts.tests.servlet.pluggability.api.jakarta_servlet_http.cookie.URLClient#setMaxAgePositiveTest

TCK Version: Jakarta EE Platform TCK 10.0.0

Tested Implementation: Open Liberty

Description: We raised a challenge for this test in the EE 9.0.x timeframe, and this one will unfortunately look familiar to those who remember that one.

Per the Servlet 6.0 API, RFC 6265 has been named the sole supported cookie standard. However, the test as implemented is still relying on the ancient Netscape standard to handle Date parsing.

This results in the following exception when running against Open Liberty:

ERROR: org.apache.commons.httpclient.cookie.MalformedCookieException: Invalid expires attribute: Unparseable date: "Wed, 12 Oct 2022 15:18:45 GMT"
       at org.apache.commons.httpclient.cookie.NetscapeDraftSpec.parseAttribute(NetscapeDraftSpec.java:192)
       at org.apache.commons.httpclient.cookie.NetscapeDraftSpec.parse(NetscapeDraftSpec.java:151)
       at org.apache.commons.httpclient.cookie.CookieSpecBase.parse(CookieSpecBase.java:248)
       at com.sun.ts.tests.servlet.api.jakarta_servlet_http.cookie.URLClient.setMaxAgePositiveTest(URLClient.java:320)
       at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
       at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
       at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
       at java.base/java.lang.reflect.Method.invoke(Method.java:566)
       at com.sun.ts.lib.harness.EETest.run(EETest.java:596)
       at com.sun.ts.lib.harness.EETest.getPropsReady(EETest.java:486)
       at com.sun.ts.lib.harness.EETest.run(EETest.java:337)
       at com.sun.ts.tests.servlet.api.jakarta_servlet_http.cookie.URLClient.run(URLClient.java:63)
       at com.sun.ts.tests.servlet.api.jakarta_servlet_http.cookie.URLClient.main(URLClient.java:49)
       ...

As established in the prior challenge, Wed, 12 Oct 2022 15:18:45 GMT is a perfectly acceptable Date format that should be recognized by the TCK. Unfortunately, commons-httpclient-3.1 (currently being used by the test) doesn't support anything newer than the now obsoleted RFC 2965. However, I can confirm that updating the test to use the RFC_2965 CookiePolicy for parsing, as was suggested in the previous challenge, is sufficient to allow this test to pass with Open Liberty.

For future, we could also look at updating this test to use the httpclient-4.5.5 (already available in the TCK) which provides full support for RFC 6265.

markt-asf commented 1 year ago

I agree that that date should be acceptable. That it isn't makes this challenge valid. I am wondering how containers have passed the TCK. Something to look into but doesn't impact the status of this challenge.

brideck commented 1 year ago

I suspect that implementations might have some sort of compatibility setting. We certainly do, and have used it in the past, but it runs afoul of the 2-digit vs 4-digit year format that we discussed in the previous challenge. I would much prefer that this just work without such a setting, which, for us, is the format you see here.

scottmarlow commented 1 year ago

Sounds like we should exclude the test. A pull request is needed to update https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/install/servlet/bin/ts.jtx + https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/install/jakartaee/bin/ts.jtx to do the exclude and once merged, should be back ported to 10.0.x branch so we can stage the Servlet + Platform TCKs.

We should validate that the excluded tests don't run with the change and then do a service release of the two TCKs.

scottmarlow commented 1 year ago

@brideck please do attach the two jtr files for the identified tests.

scottmarlow commented 1 year ago

From my local testing:

#-----testdescription-----
$file=jakartaeetck/src/com/sun/ts/tests/servlet/pluggability/api/jakarta_servlet_http/cookie/URLClient.java
$root=jakartaeetck/src
assertion_ids=Servlet\:JAVADOC\:440  See assertion html documents.
classname=com.sun.ts.tests.servlet.pluggability.api.jakarta_servlet_http.cookie.URLClient
direction=forward
finder=cts
id=setMaxAgePositiveTest
keywords=all servlet javaee javaee_web_profile setMaxAgePositiveTest novehicle forward
service_eetest=no
testName=setMaxAgePositiveTest
testProps=\ webServerHost  webServerPort  ts_home
test_directory=com/sun/ts/tests/servlet/pluggability/api/jakarta_servlet_http/cookie
#-----testdescription-----
$file=jakartaeetck/src/com/sun/ts/tests/servlet/api/jakarta_servlet_http/cookie/URLClient.java
$root=jakartaeetck/src
assertion_ids=Servlet\:JAVADOC\:440  See assertion html documents.
classname=com.sun.ts.tests.servlet.api.jakarta_servlet_http.cookie.URLClient
direction=forward
finder=cts
id=setMaxAgePositiveTest
keywords=all servlet javaee javaee_web_profile setMaxAgePositiveTest novehicle forward
service_eetest=no
testName=setMaxAgePositiveTest
testProps=\ webServerHost  webServerPort  ts_home
test_directory=com/sun/ts/tests/servlet/api/jakarta_servlet_http/cookie

Exclude for Platform TCK ts.jtx should be:

com/sun/ts/tests/servlet/pluggability/api/jakarta_servlet_http/cookie/URLClient.java#setMaxAgePositiveTest
com/sun/ts/tests/servlet/api/jakarta_servlet_http/cookie/URLClient.java#setMaxAgePositiveTest
scottmarlow commented 1 year ago

https://ci.eclipse.org/jakartaee-tck/job/10/job/eftl-standalonetck-build-run-100/lastCompletedBuild/ passed with https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-servlet-tck-6.0.1.zip for this challenge (using https://download.eclipse.org/ee4j/glassfish/glassfish-7.0.0-M9.zip).

https://ci.eclipse.org/jakartaee-tck/job/10/job/eftl-jakartaeetck-run-100/153 passed using https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-jakartaeetck-10.0.1.zip + https://download.eclipse.org/ee4j/glassfish/glassfish-7.0.0-M9.zip.

scottmarlow commented 1 year ago

I think that this issue can be closed since https://github.com/jakartaee/specifications/pull/579 is merged and the challenged TCK tests excluded from https://download.eclipse.org/jakartaee/servlet/6.0/jakarta-servlet-tck-6.0.1.zip + https://download.eclipse.org/jakartaee/platform/10/jakarta-jakartaeetck-10.0.1.zip

markt-asf commented 6 months ago

Closing this issue as this has been addressed in the refactored TCK for Servlet 6.1. Tomcat uses the Wed, 12 Oct 2022 15:18:45 GMT format and passes the TCK.