Attempt to reset Servlet response before calling ExceptionHandlers
Prior to this commit, the `ExceptionHandlerExceptionResolver` would resolve exceptions and handle them by writing to the HTTP response body, even if the request was already partially handled and content was written to the response body. This could result in HTTP responses with some content for the intended application response, then other content for the handled exception. This would happen especially when the error would be raised while writing to the response (for example when serializing content). This commit attempts to reset the HTTP response before handling the exception. This effectively resets the response buffer for the body as well as response headers. If the response is already committed, the Servlet container raises an exception and the exception handling is skipped altogether in order to avoid garbled responses. Closes gh-31104
This commit is contained in:
@@ -399,6 +399,11 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce
|
||||
exceptionHandlerMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
|
||||
}
|
||||
|
||||
// attempt to reset the response, as maybe a partial successful response is being written.
|
||||
if (!response.isCommitted()) {
|
||||
response.reset();
|
||||
}
|
||||
|
||||
ServletWebRequest webRequest = new ServletWebRequest(request, response);
|
||||
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
|
||||
|
||||
|
||||
@@ -436,6 +436,24 @@ public class ExceptionHandlerExceptionResolverTests {
|
||||
assertThat(this.response.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException");
|
||||
}
|
||||
|
||||
@Test // gh-30702
|
||||
void attemptToResetResponseBeforeResolveException() throws UnsupportedEncodingException {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
|
||||
this.resolver.setMappedHandlerClasses(HttpRequestHandler.class);
|
||||
this.resolver.setApplicationContext(ctx);
|
||||
this.resolver.afterPropertiesSet();
|
||||
|
||||
IllegalStateException ex = new IllegalStateException();
|
||||
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
|
||||
MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse();
|
||||
mockHttpServletResponse.getWriter().print("test");
|
||||
ModelAndView mav = this.resolver.resolveException(this.request, mockHttpServletResponse, handler, ex);
|
||||
|
||||
assertThat(mav).as("Exception was not handled").isNotNull();
|
||||
assertThat(mav.isEmpty()).isTrue();
|
||||
assertThat(mockHttpServletResponse.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException");
|
||||
}
|
||||
|
||||
|
||||
private void assertMethodProcessorCount(int resolverCount, int handlerCount) {
|
||||
assertThat(this.resolver.getArgumentResolvers().getResolvers()).hasSize(resolverCount);
|
||||
|
||||
Reference in New Issue
Block a user