search

jakartaee / rest

Jakarta RESTful Web Services
Other
351 stars 114 forks source link

TCK requires specific response header case #1267

Open yawkat opened 3 weeks ago

yawkat commented 3 weeks ago

ee.jakarta.tck.ws.rs.spec.resource.requestmatching.JAXRSClientIT#optionsOnSubResourceTest requires that the Allow response header is uppercase: https://github.com/jakartaee/rest/blob/main/jaxrs-tck/src/main/java/ee/jakarta/tck/ws/rs/spec/resource/requestmatching/JAXRSClientIT.java#L432

jansupol commented 3 weeks ago

RFC 9110:

The type and subtype tokens are case-insensitive.

jansupol commented 3 weeks ago

@yawkat Do you really discuss the "Accept" header? The link points to the code handling the "Allow" header.

yawkat commented 3 weeks ago

Yep, I meant allow

jansupol commented 3 weeks ago

@yawkat So you say that the test expects the "Allow" but not "allow", or do you say the test expects "GET" but not "get"?

yawkat commented 3 weeks ago

It expects Allow and does not accept allow. The startsWith I linked to causes this issue because it's not case-insensitive.

I think method is case sensitive so the matching logic there is fine.

jansupol commented 3 weeks ago

Yes, field names are case-insensitive and the test should allow "allow".

spericas commented 3 weeks ago

@alwin-joseph Do you have time to fix this one? Not sure why startsWith is used in this case.

alwin-joseph commented 3 weeks ago

@alwin-joseph Do you have time to fix this one? Not sure why startsWith is used this case.

Sure. This challenge can be resolved by fixing the test (instead of excluding) as per https://jakarta.ee/committees/specification/tckprocess/ .

@yawkat Which version of restful-ws TCK test is challenged here ?

alwin-joseph commented 3 weeks ago

@spericas @jansupol Considering that we need to address this test challenge for TCK 4.0, which branch do you suggest we make the changes ? We had release-3.1.x branch from where we released 3.1.x TCKs with test challenges. So for 4.0.x we would need a branch that has all changes of 4.0. Do you think 4.0.0 branch is the right place for this or another branch required ?

spericas commented 3 weeks ago

Branch 4.0.0 is set up for a 4.0.1 release. Seems like the correct branch to me. Any suggestions @jim-krueger?

jim-krueger commented 3 weeks ago

Seems to me that this needs to go into two branches (dual maintenance). 4.0.0 for the service release of the TCK and also the main branch. Correct?

jamezp commented 3 weeks ago

+1 to two branches, 4.0.0 and main.

spericas commented 3 weeks ago

Seems to me that this needs to go into two branches (dual maintenance). 4.0.0 for the service release of the TCK and also the main branch. Correct?

Indeed.