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

Spring Web should process requests with empty/invalid mime type [SPR-14309] #18881

Closed
spring-projects-issues opened this issue May 27, 2016 · 6 comments
Assignees
Labels
in: web status: backported type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented May 27, 2016

Nazar Vishka opened SPR-14309 and commented

If a client is sending a POST message with an empty/invalid header 'Content-Type' the request fails with InvalidMediaTypeException.

org.springframework.http.InvalidMediaTypeException: Invalid mime type "null": does not contain '/'
        at org.springframework.http.MediaType.parseMediaType(MediaType.java:385) ~[spring-web-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.http.HttpHeaders.getContentType(HttpHeaders.java:722) ~[spring-web-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.http.server.ServletServerHttpRequest.getHeaders(ServletServerHttpRequest.java:116) ~[spring-web-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.util.WebUtils.isSameOrigin(WebUtils.java:810) ~[spring-web-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.cors.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:71) ~[spring-web-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.handler.AbstractHandlerMapping$CorsInterceptor.preHandle(AbstractHandlerMapping.java:503) ~[spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.HandlerExecutionChain.applyPreHandle(HandlerExecutionChain.java:134) ~[spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:954) ~[spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:893) ~[spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:968) [spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]
        at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:870) [spring-webmvc-4.2.5.RELEASE.jar!/:4.2.5.RELEASE]

I think that org.springframework.http.HttpHeaders#getContentType should handle InvalidMediaTypeException exception and return null in such cases or you should change behaviour of org.springframework.http.MediaType#parseMediaType


Affects: 4.2.6

Issue Links:

Backported to: 4.2.7

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 29, 2016

Juergen Hoeller commented

Catching InvalidMediaTypeException, simply not exposing the content type in HttpHeaders then, is easy enough... also for a backport to 4.2.7. After all, this fits nicely with our lenient processing of non-standard HTTP method etc.

@Maze-fr
Copy link

Maze-fr commented Feb 5, 2019

IMHO, according to RFC7231 section 3.1.1.5, if the content-type is missing, it should assume a media type of application/octet-stream ; the other option is to guess the content type, but I'm not a gambler.

The problem is, as the content type is not mandatory, HttpHeaders.getContentType() should never fail.
However, when the response from a REST request, for example, returns an error 4xx or 5xx, the DefaultResponseErrorHandler wants to read the body to get the message, so it asks for the content-type, and it fails.
So, instead of getting a HttpStatusCodeException, it is a InvalidMediaTypeException from the InvalidMimeTypeException, which break completely the dynamic of the flow, for nothing.

But... we are in Spring, and Spring is awesome at giving us options.
Maybe it would be interesting to have the content-type of the request as default content-type of the response if missing ?
Or maybe being able to define it in a property ?

That problem was blocking, for my company, because when accessing to our OpenText ECM API, sometimes the content-type is missing on 4xx or 5xx responses...
So for those who have that kind of problem, here is a "dirty" solution.
You need to create a ClientHttpRequestInterceptor to put in the RestTemplate you created or got from Spring instance. And you put something like that in that interceptor (I put JSON because it's what the ECM sends me) :

	@Override
	public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
		
		ClientHttpResponse response = execution.execute(request, body);

		try {
			response.getHeaders().getContentType();
		} catch (InvalidMediaTypeException exception) {
			response.getHeaders().put(HttpHeaders.CONTENT_TYPE, Collections.singletonList(MediaType.APPLICATION_JSON_UTF8_VALUE));
		}
		
		return response;
	}

Be careful to use a put and not an add, because getContentType do a getFirst and could never find your "default" content-type...

Have fun... waiting for the fix...

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 8, 2019

Where exactly is it failing, in DefaultResponseErrorHandler#getCharset? Indeed that's a spot where it should ignore parse errors.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 8, 2019

BTW you're commenting on issue that is related but in a very different part of the codebase. Can you please create a separate issue for it?

@Maze-fr
Copy link

Maze-fr commented Feb 8, 2019

Yes, it is the in DefaultResponseErrorHandler#getCharset, indeed.
But... as, from my opinion, it is a problem in 'HttpHeaders.getContentType()' that should send back application/octet-stream, whatever if it is a request or a response, instead of an InvalidMediaTypeException, then I put it here.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 8, 2019

Asking for further changes on an issue resolved almost 3 years ago isn't going to get this any further. Also keep in mind a change like that at the level of HttpHeaders isn't going to happen. The number of follow up reports rightfully claiming broken behavior is too easy to anticipate. Falling back on application/octet-stream is not the same as ignoring a parse failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: backported type: enhancement
Projects
None yet
Development

No branches or pull requests

4 participants