Skip to content

Commit 1d7cbfa

Browse files
aooohanbclozel
authored andcommitted
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
1 parent 376a898 commit 1d7cbfa

File tree

2 files changed

+23
-0
lines changed

2 files changed

+23
-0
lines changed

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java

+5
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ protected ModelAndView doResolveHandlerMethodException(HttpServletRequest reques
399399
exceptionHandlerMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
400400
}
401401

402+
// attempt to reset the response, as maybe a partial successful response is being written.
403+
if (!response.isCommitted()) {
404+
response.reset();
405+
}
406+
402407
ServletWebRequest webRequest = new ServletWebRequest(request, response);
403408
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
404409

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java

+18
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,24 @@ void resolveExceptionViaMappedHandler() throws Exception {
436436
assertThat(this.response.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException");
437437
}
438438

439+
@Test // gh-30702
440+
void attemptToResetResponseBeforeResolveException() throws UnsupportedEncodingException {
441+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
442+
this.resolver.setMappedHandlerClasses(HttpRequestHandler.class);
443+
this.resolver.setApplicationContext(ctx);
444+
this.resolver.afterPropertiesSet();
445+
446+
IllegalStateException ex = new IllegalStateException();
447+
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
448+
MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse();
449+
mockHttpServletResponse.getWriter().print("test");
450+
ModelAndView mav = this.resolver.resolveException(this.request, mockHttpServletResponse, handler, ex);
451+
452+
assertThat(mav).as("Exception was not handled").isNotNull();
453+
assertThat(mav.isEmpty()).isTrue();
454+
assertThat(mockHttpServletResponse.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException");
455+
}
456+
439457

440458
private void assertMethodProcessorCount(int resolverCount, int handlerCount) {
441459
assertThat(this.resolver.getArgumentResolvers().getResolvers()).hasSize(resolverCount);

0 commit comments

Comments
 (0)