jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
266 stars 87 forks source link

TCK Challenge: doHeadTest shouldn't validate optional payload header fields #472

Closed brideck closed 7 months ago

brideck commented 2 years ago

Challenged Tests: com.sun.ts.tests.servlet.api.jakarta_servlet_http.httpservletrequest.URLClient#doHeadTest

TCK Version: Jakarta EE Platform TCK 10.0.0

Tested Implementation: Open Liberty

Description: The doHeadTest performs a GET request and a HEAD request and proceeds to verify that the header fields returned by each request is exactly the same.

However, Section 4.3.2 of the RFC 7231 HTTP/1.1 standard document states "The server SHOULD send the same header fields in response to a HEAD request as it would have sent if the request had been a GET, except that the payload header fields (Section 3.3) MAY be omitted."

Section 3.3 specifies which fields these are:

Open Liberty includes a Transfer-Encoding field in its GET response, but not in its HEAD response, which results in failure:

ERROR: HEAD request did not contain header that was present for GET:Transfer-Encoding: chunked

The TCK should be allowing for the fact that the Transfer-Encoding field (among the others listed) is optional in the HEAD response and not failing the test if it is not present.

JTR: URLClient_doHeadTest.jtr

joakime commented 1 year ago

The prior HTTP/1.1 spec had no such exception for "payload header fields"

https://www.rfc-editor.org/rfc/rfc2616#section-9.4

And the newest HTTP/1.1 spec actually removes the exemption on the payload header fields, and instead clarifies what RFC7230 actually meant.

https://www.rfc-editor.org/rfc/rfc9110#HEAD

brideck commented 1 year ago

And the newest HTTP/1.1 spec actually removes the exemption on the payload header fields, and instead clarifies what RFC7230 actually meant.

https://www.rfc-editor.org/rfc/rfc9110#HEAD

From the linked section in RFC 9110: "However, a server MAY omit header fields for which a value is determined only while generating the content." In Open Liberty, determining that a response has chunked encoding fits into this category.

markt-asf commented 1 year ago

I agree the TCK challenge is valid. We'll need to exclude this test for now. Exactly how we fix this in the TCK needs some thought.

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

joakime commented 1 year ago

Before we close this, do we have a replacement issue to re-enable this test with less strict assertions?

scottmarlow commented 1 year ago

Before we close this, do we have a replacement issue to re-enable this test with less strict assertions?

+1 for adding a tracking issue or pull request for updating this test.