Skip to content

Content Negotiation with media type parameters: Method producing exact match of Accept content type is not called. #27999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
thake opened this issue Feb 3, 2022 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@thake
Copy link

thake commented Feb 3, 2022

I'm working on an API that produces application/hal+json documents that use the media type profile parameter to support versioning.
The structure of the controller looks like this:

@RestController
@RequestMapping("/hal-documents")
class MyController {
    @GetMapping(produces = ["application/hal+json;charset=UTF-8"])
    fun getWithoutProfile() : ResponseEntity<MyResource> {...}

    @GetMapping(produces = ["""application/hal+json;profile="my-resource-v1""""])
    fun getV1() : ResponseEntity<MyResource> {...}

    @GetMapping(produces = ["""application/hal+json;profile="my-resource-v2""""])
    fun getV2() : ResponseEntity<MyResource> {...}
}

If I execute a request, which Accept-Header exactly matches the media type produced by getV1, the method is not being called. Instead, the request is being routed to getWithoutProfile.

This seems to be wrong. I would expect that the best matching method is being called. Routing works correctly if the charset parameter is removed from the getWithoutProfile method.

Affects: 5.3.15

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 3, 2022
@thake
Copy link
Author

thake commented Feb 3, 2022

A similar problem seems to exist for the consumes property. Given the following controller:

@RestController
@RequestMapping("/hal-documents")
class MyController {
    @PostMapping(
        consumes = ["""application/hal+json;profile="my-resource-v1""""],
        produces = ["""application/hal+json;profile="my-resource-v1""""]
    )
    fun postVersion1(@RequestBody request : String) = "version-1"

    @PostMapping(
        consumes = ["""application/hal+json;profile="my-resource-v2""""],
        produces = ["""application/hal+json;profile="my-resource-v2""""]
    )
    fun postVersion2(@RequestBody request : String) = "version-2";
}

A request that provides a request body with the content type application/hal+json;profile="my-resource-v2" is being routed to postVersion1 but should be routed to postVersion2.

@rstoyanchev rstoyanchev self-assigned this Feb 7, 2022
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Feb 7, 2022
@rstoyanchev rstoyanchev added this to the Triage Queue milestone Feb 7, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 8, 2022

There is a prior, related discussion in this #17949 (comment).

Generally speaking, a produces format is chosen mainly given the type and sub-type of the media type. Parameters provide additional information, but their meaning and relevance is unknown to the framework.

If produces declares a media type with a parameter in it, and if the same parameter is also present in the media type from the Accept header, we'll make sure the two match. In all other cases (parameter present in produces but not in Accept header or vice versa), parameters have no impact and effectively considered a match.

So even though charset is not in the Accept header (and shouldn't be I think, see #22788), it's still considered a match, and so effectively there is one matching parameter for each mapping. It's unclear which is or should be a more specific match. More generally, given a full range of cases with 1 or more of the same or different parameters on either side, it would be hard to imagine a good solution for matching and sorting based on parameters.

Note that for consumes we currently do not have the same matching where if the same parameter is present in both the consumes and the request content-type. It's something we could consider. On the produces side it was added for the case of app/atom+xml;type=feed vs app/atom+xml;type=entry, see #21670, but again for this to work, both produces conditions have to have the same parameter, and Accept header has to have it too.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Feb 8, 2022
@thake
Copy link
Author

thake commented Feb 10, 2022

@rstoyanchev thanks for your fast response and for pointing out the discussion of #17949. I've not found this thread in my research.

Generally speaking, a produces format is chosen mainly given the type and sub-type of the media type. Parameters provide > additional information, but their meaning and relevance is unknown to the framework.

Is there an easy way to provide the additional meaning of media type parameters to the framework? Somehow stating that the media type parameter should be part of content negotiation. Meaning that an Accept or Content-Type media type that has a parameter value different to the consumes/produces media type is not compatible. The only way I identified is by creating a custom ProducesRequestCondition which basically copy/pastes most of the ProducesRequestCondition and ConsumesRequestCondition and adds the special media type handling. This seems rather cumbersome and error-prone due to the complex matter of content negotiation.

So even though charset is not in the Accept header (and shouldn't be I think, see #22788), it's still considered a match, and so effectively there is one matching parameter for each mapping. It's unclear which is or should be a more specific match. More generally, given a full range of cases with 1 or more of the same or different parameters on either side, it would be hard to imagine a good solution for matching and sorting based on parameters.

I agree that the Accept / Content-Type media type for "+json" types implicitly contains the UTF-8 charset as json is by default UTF-8 (see https://datatracker.ietf.org/doc/html/rfc7159#section-8.1) if not otherwise explicitly stated. If we follow this line of argumentation, we can also assume, that specifying a "+json" produces/consumes media type without a charset matches the UTF-8 charset parameter. Thus application/hal+json;profile="my-resource-v1" matches 2 parameters from the Accept media type application/hal+json;profile="my-resource-v1". The first one is explicitly stated with profile and the second one charset is implicit. As application/hal+json;charset=UTF-8 only matches one parameter, it should be ranked lower.

Note that for consumes we currently do not have the same matching where if the same parameter is present in both the consumes and the request content-type. It's something we could consider. On the produces side it was added for the case of app/atom+xml;type=feed vs app/atom+xml;type=entry, see #21670, but again for this to work, both produces conditions have to have the same parameter, and Accept header has to have it too.

I'll open a separate issue for the consumes part, as the cause of the problem seems to be different. (update: created #28024)

@rstoyanchev
Copy link
Contributor

Short of something like the params condition, I don't see a way to expose this. This is however not something we're looking to introduce. Aside from introducing additional syntax and/or annotation attributes, what's lacking is an algorithm that would handle this in the general case.

@rstoyanchev rstoyanchev added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 14, 2022
@rstoyanchev rstoyanchev removed this from the Triage Queue milestone Feb 14, 2022
@bclozel bclozel changed the title Content Negotiation with media type parameters: Method producing exact match of Access content type is not called. Content Negotiation with media type parameters: Method producing exact match of Accept content type is not called. Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

3 participants