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

StringHttpMessageConverter addDefaultHeaders() should check media type for wildcard before setting it into headers #24283

Closed
artembilan opened this issue Jan 2, 2020 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@artembilan
Copy link
Member

artembilan commented Jan 2, 2020

I have just started to fail after this change 9b30d46.

My content type candidate is like this one:

if (CollectionUtils.isEmpty(acceptTypes)) {
		acceptTypes = Collections.singletonList(MediaType.ALL);
}

So, MediaType.ALL is really compatible with MediaType.APPLICATION_JSON, but it is wrong value for the Content-Type header:

java.lang.IllegalArgumentException: Content-Type cannot contain wildcard type '*'

	at org.springframework.util.Assert.isTrue(Assert.java:118)
	at org.springframework.http.HttpHeaders.setContentType(HttpHeaders.java:949)
	at org.springframework.http.converter.StringHttpMessageConverter.addDefaultHeaders(StringHttpMessageConverter.java:109)
	at org.springframework.http.converter.StringHttpMessageConverter.addDefaultHeaders(StringHttpMessageConverter.java:44)
	at org.springframework.http.converter.AbstractHttpMessageConverter.write(AbstractHttpMessageConverter.java:211)

Any clues what to use instead of MediaType.ALL when no Accept header in the request?

Or maybe this fix should be improved to skip MediaType.ALL as it is done in the super class:

if (contentType == null || contentType.isWildcardType() || contentType.isWildcardSubtype()) {
		contentTypeToUse = getDefaultContentType(t);
}

Note that AbstractMessageConverterMethodProcessor is not used in Spring Integration.
There logic in the HttpRequestHandlingMessagingGateway is like this: https://github.com/spring-projects/spring-integration/blob/master/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGateway.java#L171

So, instead of falling back to the MediaType.APPLICATION_OCTET_STREAM we try to rely on the Content-Type populated by the converted which fits to the payload we would like to return into the response.

Originally based on discussion.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 2, 2020
artembilan referenced this issue Jan 2, 2020
This commit restores the interpretation of JSON as UTF-8 by default that
was removed in #bc205e0 and also ensures a charset is not appended
automatically to "application/json".

Closes gh-24123
@rstoyanchev rstoyanchev self-assigned this Jan 2, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 2, 2020
@rstoyanchev rstoyanchev added this to the 5.2.3 milestone Jan 2, 2020
@rstoyanchev rstoyanchev changed the title The StringHttpMessageConverter.addDefaultHeaders() should check for wildcard media type before setting it into headers StringHttpMessageConverter addDefaultHeaders() should check media type for wildcard before setting it into headers Jan 7, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 7, 2020

Resolved via 9d963ab which erroneously refers to another issue.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants