DefaultHandlerExceptionResolver handling of media type exception on Tomcat 7+

[shanman190]

Affects: 3.0.0-latest

To start out with this is referring specifically to this little piece of code found in DefaultHandlerExceptionResolver:
https://github.com/spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java#L278-L287

So given this as contextual information, we can observe the test that backs this code found here:
https://github.com/spring-projects/spring-framework/blob/master/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java#L79-L88

The behavior should be as defined in the test which I do agree completely with. However, at least when running a Tomcat embedded served Spring application this particular piece of code fails to satisfy the behavior as defined by the test. The reason behind this is that the response.sendError(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE) commits the response, so when the response.setHeader(String, String) method is called just a few statements later the header containing the accepted MediaTypes is thrown away.

You can see in the Tomcat code over the last few versions that this behavior has been in place since 7.x and still exists within 10.x
Tomcat 7: https://github.com/apache/tomcat/blob/7.0.108/java/org/apache/catalina/connector/ResponseFacade.java#L527-L536
Tomcat 8: https://github.com/apache/tomcat/blob/8.5.62/java/org/apache/catalina/connector/ResponseFacade.java#L467-L480
Tomcat 9: https://github.com/apache/tomcat/blob/9.0.42/java/org/apache/catalina/connector/ResponseFacade.java#L466-L479
Tomcat 10: https://github.com/apache/tomcat/blob/10.0.1/java/org/apache/catalina/connector/ResponseFacade.java#L466-L479

I believe the resolution here is that in the DefaultHandlerExceptionResolver the sendError method call should be moved down to just before the return as is the case in the similarly constructed handleHttpRequestMethodNotSupported (https://github.com/spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java#L254-L263).

[shanman190]

If necessary, I can put together a sample+test that shows this issue manifesting.

[sbrannen]

@shanman190, thanks for raising the issue and providing such detail.

Your analysis and comparison to the implementation of handleHttpRequestMethodNotSupported() sound reasonable, especially when taking into account the Javadoc for javax.servlet.http.HttpServletResponse.sendError(int, String):

After using this method, the response should be considered to be committed and should not be written to.

---

If necessary, I can put together a sample+test that shows this issue manifesting.

That would be great!

But... if you are already going to put together a test, would you be willing to submit the fix along with the test as a PR?

[rstoyanchev]

I'll take care of this.