Add RFC 10008 (QUERY HTTP method) support

[desiderantes]

This change introduces support for RFC 10008: The HTTP QUERY Method.

Although initially described as just a "safe GET with a body", the QUERY method unlocks the ability to send complex payloads that do not represent a mutation, and can be handled in a safe, idempotent way. Think of it as a replacement for usecases like complex query parameters in URLs, POST requests that do not mutate anything (but have to be treated as such by all other components in the network roundtrip, so no caching, etc.), and non-mutating HTML Form submission that can use an appropriate encoding for the body.

Fixes #32975

[desiderantes]

Will keep this one in draft until the standard is published, but I'd appreciate any review

[desiderantes]

the draft has been approved for publication, so I'm marking this PR as ready for review.

[bclozel]

the draft has been approved for publication, so I'm marking this PR as ready for review.

That's a 404 now, we can use https://www.rfc-editor.org/info/rfc10008/ instead as a reference.

[anupamsingh24]

In which spring boot version will this change be available?

[bclozel]

Thanks @desiderantes for tackling this. As you can see, touching HTTP method support in Spring can involve quite a few different places.

Before we can merge, I'd like to cut the scope down substantially. The core of this change should be about teaching the framework that HTTP QUERY exists and that it carries a request body. Anything else should be considered in later stages. This is a brand new standard; we want to collect developer feedback and see how the industry adopts this before significantly expanding the API surface.

I started an inline review but given the many comments, I thought I would summarize my opinion here to make things easier on your side.

Focus this PR on the following

Please keep the following changes:

• everything in HttpMethod, HttpHeaders and RequestMethod is needed
• Accept-Query support on the two "unsupported media type" exceptions, HttpMediaTypeNotSupportedException, UnsupportedMediaTypeStatusException
• support for reading request bodies from QUERY requests, so that @RequestBody and HttpEntity work. Keep AbstractMessageConverterMethodArgumentResolver and AbstractMessageReaderArgumentResolver changes.
• the client-side support in HttpComponentsClientHttpRequestFactory is correct
• The Accept-Query / OPTIONS wiring in both RequestMappingInfoHandlerMapping types
• I think the form body parsing for QUERY in FormContentFilter is useful
• all related test updates

Please revert the CORS-related changes

The changes you made in CorsConfiguration are problematic. Expanding DEFAULT_METHODS is dangerous here because we're permitting additional HTTP methods for CORS by default, without letting our users know - all for a brand new HTTP method that isn't widely supported yet. Some might see this as a security problem. Definitely not minor release material.

The DEFAULT_PERMIT_METHODS part is the Fetch-spec simple method set. I'm not the CORS resident expert but I think that GET, HEAD and POST are the only ones allowed here. Other methods will be sent with a preflight request by browsers. Until the CORS spec evolves, we shouldn't change this.

Please revert the caching / conditional-request changes

This PR adds QUERY to various lists of "safe methods".

While QUERY is technically a safe method, this isn't what those lists mean. They drive conditional request support in Spring Framework and only look at the actual URL for validation. For QUERY, the request body must be encoded in the caching key when considering conditional requests. This is why QUERY endpoints might want to issue Location headers with a stable URL for a particular resource. This allows clients to send further conditional requests without sending the body again.

Supporting conditional requests and caching in general for QUERY methods would require substantial changes. Right now this doesn't make sense since we don't have use cases for this and I'm not aware of QUERY support for CDNs and intermediaries.

Please revert the new annotations

Namely @QueryMapping and @QueryExchange. I'd like to hold off on both.
We should aim to unlock the use of QUERY in Spring Framework, not make it a first class citizen right away. This is a new standard and we don't know much about adoption in the wider industry. We've been burnt by this recently with the "Forwarded" header support (well known standard, poorly adopted in reality). @QueryMapping is also already widely used by Spring for GraphQL applications; adding a new annotation with the same name would create IDE import issues in existing applications.

Please revert the new DSL and builder shortcut methods

The use cases are already possible with HttpMethod and other method variants. There is no need to add public API for this right now.
This includes changes in (and related to) RestClient, WebClient, WebTestClient, RestTestClient, RouterFunctions.Builder, RequestEntity, MockServerHttpRequest (including in tests).

Other reverts

• SimpleClientHttpRequestFactory, the url connection JDK client does not support QUERY methods and rejects them with protocol exceptions. It's not needed.
• HiddenHttpMethodFilter - this filter exists so that HTML forms can express PUT/DELETE/PATCH (using GET or POST). I don't think we should support tunnelling a POST into a safe method. I'd leave both lists alone, but I'm open to hearing the use case if you had one in mind.

Last remarks

The changes in RequestMappingInfoHandlerMapping are questionable.
I understand that HEAD is considered as "a GET without a response body", but here I'm not sure there is such an equivalent for QUERY. With the current implementation, sending HEAD to a QUERY endpoint would probably return "405 method not allowed", whereas an OPTIONS request to that same endpoint would reply with QUERY as an allowed method. Maybe this needs to be refined? Can you explain the rationale here?

[bclozel]

Please apply the suggested changes in my comment above.

[desiderantes]

Hi @bclozel ! Thanks for the review, I've been going through the code fro a while, after all, this PR is a bit old so I had to refresh myself on the changes. I'll address each point:

• CORS-related changes: No problem, and I agree, I'll remove them.
• caching / conditional-request changes: I think that in this case, it is the client who should be in charge of caching for most situations, but i can also envision caching in a QUERY /orders with a json body just like people expect it when they issue a GET /orders?from=2024&to=2026&orderBy=DATE&orderDirection=ASC. Will remove it anyways.
• revert the new annotations: While this certainly diminishes the appeal of the PR, I understand the concerns.
• revert the new DSL and builder shortcut methods: Same as before
• SimpleClientHttpRequestFactory: I guess this is the same situation as with PATCH which was added before. Will remove it.
• HiddenHttpMethodFilter: I disagree. The point is that it is reasonable for someone building a QUERY endpoint to want to support both non-web users as web as web users, and then deal with the same restriction that made this filter exist in the first place. I don't understand the issue with a POST request that does not produce effects, as this is certainly what most people have done before QUERY.
• RequestMappingInfoHandlerMapping: This is certainly an error on my side. According to A.3. Discovery of QUERY Formats HEAD would be ideal to probe for the Accept-Query header and check the supported formats, so I'll instead implement this behaviour

[bclozel]

Thanks for your feedback.

HiddenHttpMethodFilter: I disagree. The point is that it is reasonable for someone building a QUERY endpoint to want to support both non-web users as web as web users, and then deal with the same restriction that made this filter exist in the first place. I don't understand the issue with a POST request that does not produce effects, as this is certainly what most people have done before QUERY.

Fair enough, let's keep that one in then.

[bclozel]

This is now merged and scheduled for the next milestone. You can use our SNAPSHOTs in the meantime to test things out. Cheers @desiderantes for tackling this.