diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java index 10630a37a3..0bfa199090 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java @@ -399,16 +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(); - ArrayList exceptions = new ArrayList<>(); try { + response.reset(); if (logger.isDebugEnabled()) { logger.debug("Using @ExceptionHandler " + exceptionHandlerMethod); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java index 41aadbade4..386891ddee 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java @@ -73,14 +73,13 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Kazuki Shimizu * @author Brian Clozel * @author Rodolphe Lecocq - * @since 3.1 */ @SuppressWarnings("unused") public class ExceptionHandlerExceptionResolverTests { - private static int RESOLVER_COUNT; + private static int DEFAULT_RESOLVER_COUNT; - private static int HANDLER_COUNT; + private static int DEFAULT_HANDLER_COUNT; private ExceptionHandlerExceptionResolver resolver; @@ -93,8 +92,8 @@ public class ExceptionHandlerExceptionResolverTests { public static void setupOnce() { ExceptionHandlerExceptionResolver resolver = new ExceptionHandlerExceptionResolver(); resolver.afterPropertiesSet(); - RESOLVER_COUNT = resolver.getArgumentResolvers().getResolvers().size(); - HANDLER_COUNT = resolver.getReturnValueHandlers().getHandlers().size(); + DEFAULT_RESOLVER_COUNT = resolver.getArgumentResolvers().getResolvers().size(); + DEFAULT_HANDLER_COUNT = resolver.getReturnValueHandlers().getHandlers().size(); } @BeforeEach @@ -117,21 +116,21 @@ public class ExceptionHandlerExceptionResolverTests { @Test void setCustomArgumentResolvers() { - HandlerMethodArgumentResolver resolver = new ServletRequestMethodArgumentResolver(); - this.resolver.setCustomArgumentResolvers(Collections.singletonList(resolver)); + HandlerMethodArgumentResolver argumentResolver = new ServletRequestMethodArgumentResolver(); + this.resolver.setCustomArgumentResolvers(Collections.singletonList(argumentResolver)); this.resolver.afterPropertiesSet(); - assertThat(this.resolver.getArgumentResolvers().getResolvers().contains(resolver)).isTrue(); - assertMethodProcessorCount(RESOLVER_COUNT + 1, HANDLER_COUNT); + assertThat(this.resolver.getArgumentResolvers().getResolvers().contains(argumentResolver)).isTrue(); + assertMethodProcessorCount(DEFAULT_RESOLVER_COUNT + 1, DEFAULT_HANDLER_COUNT); } @Test void setArgumentResolvers() { - HandlerMethodArgumentResolver resolver = new ServletRequestMethodArgumentResolver(); - this.resolver.setArgumentResolvers(Collections.singletonList(resolver)); + HandlerMethodArgumentResolver argumentResolver = new ServletRequestMethodArgumentResolver(); + this.resolver.setArgumentResolvers(Collections.singletonList(argumentResolver)); this.resolver.afterPropertiesSet(); - assertMethodProcessorCount(1, HANDLER_COUNT); + assertMethodProcessorCount(1, DEFAULT_HANDLER_COUNT); } @Test @@ -141,7 +140,7 @@ public class ExceptionHandlerExceptionResolverTests { this.resolver.afterPropertiesSet(); assertThat(this.resolver.getReturnValueHandlers().getHandlers().contains(handler)).isTrue(); - assertMethodProcessorCount(RESOLVER_COUNT, HANDLER_COUNT + 1); + assertMethodProcessorCount(DEFAULT_RESOLVER_COUNT, DEFAULT_HANDLER_COUNT + 1); } @Test @@ -158,7 +157,7 @@ public class ExceptionHandlerExceptionResolverTests { this.resolver.setReturnValueHandlers(Collections.singletonList(handler)); this.resolver.afterPropertiesSet(); - assertMethodProcessorCount(RESOLVER_COUNT, 1); + assertMethodProcessorCount(DEFAULT_RESOLVER_COUNT, 1); } @Test @@ -191,9 +190,7 @@ public class ExceptionHandlerExceptionResolverTests { this.resolver.afterPropertiesSet(); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("IllegalArgumentException"); + assertExceptionHandledAsBody(mav, "IllegalArgumentException"); } @Test // gh-26317 @@ -203,9 +200,7 @@ public class ExceptionHandlerExceptionResolverTests { this.resolver.afterPropertiesSet(); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("IllegalArgumentException"); + assertExceptionHandledAsBody(mav, "IllegalArgumentException"); } @Test @@ -215,9 +210,7 @@ public class ExceptionHandlerExceptionResolverTests { this.resolver.afterPropertiesSet(); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("IllegalArgumentException"); + assertExceptionHandledAsBody(mav, "IllegalArgumentException"); } @Test // SPR-13546 @@ -248,85 +241,63 @@ public class ExceptionHandlerExceptionResolverTests { @Test void resolveExceptionGlobalHandler() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); IllegalAccessException ex = new IllegalAccessException(); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("AnotherTestExceptionResolver: IllegalAccessException"); + assertExceptionHandledAsBody(mav, "AnotherTestExceptionResolver: IllegalAccessException"); } @Test void resolveExceptionGlobalHandlerOrdered() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); IllegalStateException ex = new IllegalStateException(); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("TestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "TestExceptionResolver: IllegalStateException"); } @Test // gh-26317 void resolveExceptionGlobalHandlerOrderedMatchingCauseLevel2() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); Exception ex = new Exception(new Exception(new IllegalStateException())); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("TestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "TestExceptionResolver: IllegalStateException"); } @Test // SPR-12605 void resolveExceptionWithHandlerMethodArg() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); ArrayIndexOutOfBoundsException ex = new ArrayIndexOutOfBoundsException(); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("HandlerMethod: handle"); + assertExceptionHandledAsBody(mav, "HandlerMethod: handle"); } @Test void resolveExceptionWithAssertionError() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); AssertionError err = new AssertionError("argh"); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, new ServletException("Handler dispatch failed", err)); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo(err.toString()); + assertExceptionHandledAsBody(mav, err.toString()); } @Test void resolveExceptionWithAssertionErrorAsRootCause() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyConfig.class); AssertionError rootCause = new AssertionError("argh"); FatalBeanException cause = new FatalBeanException("wrapped", rootCause); @@ -334,9 +305,7 @@ public class ExceptionHandlerExceptionResolverTests { HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo(rootCause.toString()); + assertExceptionHandledAsBody(mav, rootCause.toString()); } @Test //gh-27156 @@ -354,70 +323,52 @@ public class ExceptionHandlerExceptionResolverTests { HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); + assertExceptionHandledAsBody(mav, ""); assertThat(this.response.getStatus()).isEqualTo(HttpStatus.GATEWAY_TIMEOUT.value()); assertThat(this.response.getErrorMessage()).isEqualTo("Gateway Timeout"); - assertThat(this.response.getContentAsString()).isEmpty(); } @Test void resolveExceptionControllerAdviceHandler() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyControllerAdviceConfig.class); IllegalStateException ex = new IllegalStateException(); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("BasePackageTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "BasePackageTestExceptionResolver: IllegalStateException"); } @Test // gh-26317 void resolveExceptionControllerAdviceHandlerMatchingCauseLevel2() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyControllerAdviceConfig.class); Exception ex = new Exception(new IllegalStateException()); HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("BasePackageTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "BasePackageTestExceptionResolver: IllegalStateException"); } @Test void resolveExceptionControllerAdviceNoHandler() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyControllerAdviceConfig.class); IllegalStateException ex = new IllegalStateException(); ModelAndView mav = this.resolver.resolveException(this.request, this.response, null, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "DefaultTestExceptionResolver: IllegalStateException"); } @Test // SPR-16496 void resolveExceptionControllerAdviceAgainstProxy() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(ctx); - this.resolver.afterPropertiesSet(); + loadConfiguration(MyControllerAdviceConfig.class); IllegalStateException ex = new IllegalStateException(); HandlerMethod handlerMethod = new HandlerMethod(new ProxyFactory(new ResponseBodyController()).getProxy(), "handle"); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("BasePackageTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "BasePackageTestExceptionResolver: IllegalStateException"); } @Test // gh-22619 @@ -431,13 +382,11 @@ public class ExceptionHandlerExceptionResolverTests { ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); ModelAndView mav = this.resolver.resolveException(this.request, this.response, handler, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(this.response.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "DefaultTestExceptionResolver: IllegalStateException"); } @Test // gh-30702 - void attemptToResetResponseBeforeResolveException() throws UnsupportedEncodingException { + void attemptToResetResponseBeforeResolveException() throws Exception { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); this.resolver.setMappedHandlerClasses(HttpRequestHandler.class); this.resolver.setApplicationContext(ctx); @@ -445,13 +394,27 @@ public class ExceptionHandlerExceptionResolverTests { 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); + this.response.getWriter().print("test"); + ModelAndView mav = this.resolver.resolveException(this.request, this.response, handler, ex); - assertThat(mav).as("Exception was not handled").isNotNull(); - assertThat(mav.isEmpty()).isTrue(); - assertThat(mockHttpServletResponse.getContentAsString()).isEqualTo("DefaultTestExceptionResolver: IllegalStateException"); + assertExceptionHandledAsBody(mav, "DefaultTestExceptionResolver: IllegalStateException"); + } + + @Test // gh-30702 + void attemptToResetResponseBeforeResolveExceptionFails() throws Exception { + 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(); + this.response.getWriter().print("test"); + this.response.setCommitted(true); + ModelAndView mav = this.resolver.resolveException(this.request, this.response, handler, ex); + + assertThat(mav).as("Exception was handled").isNull(); + assertThat(this.response.getContentAsString()).isEqualTo("test"); } @@ -460,6 +423,18 @@ public class ExceptionHandlerExceptionResolverTests { assertThat(this.resolver.getReturnValueHandlers().getHandlers()).hasSize(handlerCount); } + private void assertExceptionHandledAsBody(ModelAndView mav, String expectedBody) throws UnsupportedEncodingException { + assertThat(mav).as("Exception was not handled").isNotNull(); + assertThat(mav.isEmpty()).isTrue(); + assertThat(this.response.getContentAsString()).isEqualTo(expectedBody); + } + + private void loadConfiguration(Class configClass) { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(configClass); + this.resolver.setApplicationContext(ctx); + this.resolver.afterPropertiesSet(); + } + @Controller static class ModelAndViewController {