Skip to content
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

Order of mediaTypesToUse list is different from the order of message converters #27724

Closed
scruel opened this issue Nov 24, 2021 · 1 comment
Closed
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

@scruel
Copy link

scruel commented Nov 24, 2021

In AbstractMessageConverterMethodProcessor#writeWithMessageConverters method, it will create a list called mediaTypesToUse by acceptableTypes and producibleTypes, its order is not added as we excepted, so it will cause choosing the wrong selectedMediaType that will affect which converter to use.
By the way, the HTTP Header Accept Values will be reordered by HeaderContentNegotiationStrategy, this strategy may can solve this problem, even though this is not a same problem, and change this strategy is not such easy as #27488 mentioned.

Steps:

  1. set MappingJackson2HttpMessageConverter as the first converter(order of converters)
  2. send a request with headers have:
    Accept: text/html, */*
    or:
    Accept: text/html, application/json
  3. Controller return CharSequence type

Will treat as a string type:

if (value instanceof CharSequence) {
	body = value.toString();
	valueType = String.class;
	targetType = String.class;
}

Will produce a mediaTypesToUse:
image

Both MappingJackson2HttpMessageConverter and StringHttpMessageConverter can write the result value, but it will use StringHttpMessageConverter, even though MappingJackson2HttpMessageConverter is the first converter of converters now.

Environment:
Windows 10 Pro, Spring version 5.3.8

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 24, 2021
scruel added a commit to scruel/spring-framework that referenced this issue Nov 24, 2021
@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 24, 2021
@jhoeller jhoeller added this to the Triage Queue milestone Nov 24, 2021
@poutsma
Copy link
Contributor

poutsma commented Nov 24, 2021

The acceptable media types are more significant than the producible ones by design, because otherwise the client would end up with a less desirable media type. From this, it follows that we iterate over the acceptable types before the producible ones.

This might have unforeseen consequences, like you describe above, but the alternative would definitely be worse. Changing this order, like you propose in #27725, breaks the ServletAnnotationControllerHandlerMethodTests, and would break many applications as well, because of backward compatibility reasons.

Instead of relying on the message converter order, it works better to specify the desirable media types explicitly by using the produces element of your @RequestMapping annotation, see this section of the reference documentation.

@poutsma poutsma self-assigned this Nov 24, 2021
@poutsma poutsma removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 24, 2021
@poutsma poutsma removed this from the Triage Queue milestone Nov 24, 2021
@poutsma poutsma added the status: declined A suggestion or change that we don't feel we should currently apply label Nov 24, 2021
@poutsma poutsma closed this as completed Nov 24, 2021
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

4 participants