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

Accept header with trailing comma produces HTTP "406 Not Acceptable" #23241

Closed
j3r0lin opened this issue Jul 5, 2019 · 4 comments
Closed

Accept header with trailing comma produces HTTP "406 Not Acceptable" #23241

j3r0lin opened this issue Jul 5, 2019 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@j3r0lin
Copy link

j3r0lin commented Jul 5, 2019

Spring version: 5.1.8.RELEASE

Send a http request with header like this, containing , at the end.

Accept: application/json, 

HeaderContentNegotiationStrategy will produce a HttpMediaTypeNotAcceptableException.

org.springframework.web.HttpMediaTypeNotAcceptableException: Could not parse 'Accept' header [application/json,]: Invalid mime type "": 'mimeType' must not be empty
	at org.springframework.web.accept.HeaderContentNegotiationStrategy.resolveMediaTypes(HeaderContentNegotiationStrategy.java:59) ~[spring-web-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]
	at org.springframework.web.accept.ContentNegotiationManager.resolveMediaTypes(ContentNegotiationManager.java:124) ~[spring-web-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodProcessor.getAcceptableMediaTypes(AbstractMessageConverterMethodProcessor.java:391) ~[spring-webmvc-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodProcessor.writeWithMessageConverters(AbstractMessageConverterMethodProcessor.java:229) ~[spring-webmvc-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.HttpEntityMethodProcessor.handleReturnValue(HttpEntityMethodProcessor.java:225) ~[spring-webmvc-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]
	at org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite.handleReturnValue(HandlerMethodReturnValueHandlerComposite.java:82) ~[spring-web-5.1.8.RELEASE.jar!/:5.1.8.RELEASE]

That http request works before 5.1.2.RELEASE.

Maybe the cause of this is the parsing method refactoring in this commit f4b05dc.

StringUtils.tokenizeToStringArray(mimeTypes, ",") will trim tokens and ignore empty tokens by default, but now, the replacement method not support that anymore.

@sbrannen
Copy link
Member

sbrannen commented Jul 5, 2019

Send a http request with header like this, containing , at the end.

What is your use case for that?

What client is sending the Accept request header with a trailing comma?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 5, 2019
@j3r0lin
Copy link
Author

j3r0lin commented Jul 5, 2019

I'm using an HTTP client named Paw. It sends that request from its built in oauth2 client.

Although it seems to be an invalid header, it did work before.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 5, 2019
@sbrannen sbrannen changed the title Accept header ends with "," produces HTTP "406 Not Acceptable" Accept header with trailing comma produces HTTP "406 Not Acceptable" Jul 6, 2019
@sbrannen sbrannen added the type: regression A bug that is also a regression label Jul 6, 2019
@sbrannen sbrannen added this to the 5.1.9 milestone Jul 6, 2019
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 6, 2019
@sbrannen
Copy link
Member

sbrannen commented Jul 6, 2019

Thanks for the feedback. I've labeled this as a regression.

@sbrannen
Copy link
Member

sbrannen commented Jul 6, 2019

The following test passes against the v5.1.2.RELEASE tag and fails on the current 5.1.x branch (beginning with the v5.1.3.RELEASE tag).

@Test
public void parseMediaTypesWithTrailingComma() {
	List<MediaType> mediaTypes = MediaType.parseMediaTypes("text/plain, text/html, ");
	assertNotNull("No media types returned", mediaTypes);
	assertEquals("Incorrect number of media types", 2, mediaTypes.size());
}

The following fails on master.

@Test
public void parseMimeTypesWithTrailingComma() {
	List<MimeType> mimeTypes = MimeTypeUtils.parseMimeTypes("text/plain, text/html,");
	assertThat(mimeTypes).as("No mime types returned").isNotNull();
	assertThat(mimeTypes.size()).as("Invalid number of mime types").isEqualTo(2);
}

@sbrannen sbrannen self-assigned this Jul 6, 2019
@rstoyanchev rstoyanchev removed the status: feedback-provided Feedback has been provided label Dec 20, 2019
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: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants