From 6fd3042462547da098a9d8c3d5eaae30f7dea808 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 22 Jun 2015 16:12:17 +0100 Subject: [PATCH] Align servlet container error handling with executable jar/war behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when an exception was thrown by a Controller in an application deployed to a servlet container the exception that was handled would be Spring Framework’s NestedServletException rather than the exception thrown by the application. Furthermore, when an exception was thrown or the response was used to send an error, the javax.servlet.error.request_uri request attribute would not be set. This differed from the behaviour in an executable jar/war where the exception would be the one thrown by the application, and the request_uri attribute would be set. This commit updates ErrorPageFilter, which is only involved in a servlet container, to unwrap a NestedServletException so that it’s the application’s exception that’s handled, and to set the request_uri attribute in the event of an exception being thrown or an error being sent. Closes gh-3249 --- .../boot/context/web/ErrorPageFilter.java | 12 +++++- .../context/web/ErrorPageFilterTests.java | 39 ++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java index f6af4d60ce..c772c3ba2a 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java @@ -39,6 +39,7 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.util.NestedServletException; /** * A special {@link AbstractConfigurableEmbeddedServletContainer} for non-embedded @@ -69,6 +70,8 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine private static final String ERROR_MESSAGE = "javax.servlet.error.message"; + public static final String ERROR_REQUEST_URI = "javax.servlet.error.request_uri"; + private static final String ERROR_STATUS_CODE = "javax.servlet.error.status_code"; private String global; @@ -121,7 +124,11 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine } } catch (Throwable ex) { - handleException(request, response, wrapped, ex); + Throwable exceptionToHandle = ex; + if (ex instanceof NestedServletException) { + exceptionToHandle = ((NestedServletException) ex).getRootCause(); + } + handleException(request, response, wrapped, exceptionToHandle); response.flushBuffer(); } } @@ -225,9 +232,10 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine return this.global; } - private void setErrorAttributes(ServletRequest request, int status, String message) { + private void setErrorAttributes(HttpServletRequest request, int status, String message) { request.setAttribute(ERROR_STATUS_CODE, status); request.setAttribute(ERROR_MESSAGE, message); + request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI()); } private void rethrow(Throwable ex) throws IOException, ServletException { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java index 2e5d18e485..393889fa5e 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java @@ -38,6 +38,7 @@ import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.context.request.async.StandardServletAsyncWebRequest; import org.springframework.web.context.request.async.WebAsyncManager; import org.springframework.web.context.request.async.WebAsyncUtils; +import org.springframework.web.util.NestedServletException; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -62,7 +63,8 @@ public class ErrorPageFilterTests { private ErrorPageFilter filter = new ErrorPageFilter(); - private MockHttpServletRequest request = new MockHttpServletRequest(); + private MockHttpServletRequest request = new MockHttpServletRequest("GET", + "/test/path"); private MockHttpServletResponse response = new MockHttpServletResponse(); @@ -199,6 +201,9 @@ public class ErrorPageFilterTests { equalTo((Object) 400)); assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE), equalTo((Object) "BAD")); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_REQUEST_URI), + equalTo((Object) "/test/path")); + assertTrue(this.response.isCommitted()); assertThat(this.response.getForwardedUrl(), equalTo("/error")); } @@ -221,6 +226,8 @@ public class ErrorPageFilterTests { equalTo((Object) 400)); assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE), equalTo((Object) "BAD")); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_REQUEST_URI), + equalTo((Object) "/test/path")); assertTrue(this.response.isCommitted()); assertThat(this.response.getForwardedUrl(), equalTo("/400")); } @@ -264,6 +271,8 @@ public class ErrorPageFilterTests { equalTo((Object) "BAD")); assertThat(this.request.getAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE), equalTo((Object) RuntimeException.class.getName())); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_REQUEST_URI), + equalTo((Object) "/test/path")); assertTrue(this.response.isCommitted()); assertThat(this.response.getForwardedUrl(), equalTo("/500")); } @@ -319,6 +328,8 @@ public class ErrorPageFilterTests { equalTo((Object) "BAD")); assertThat(this.request.getAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE), equalTo((Object) IllegalStateException.class.getName())); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_REQUEST_URI), + equalTo((Object) "/test/path")); assertTrue(this.response.isCommitted()); } @@ -465,6 +476,32 @@ public class ErrorPageFilterTests { assertThat(this.output.toString(), containsString("request [/test/alpha]")); } + @Test + public void nestedServletExceptionIsUnwrapped() throws Exception { + this.filter.addErrorPages(new ErrorPage(RuntimeException.class, "/500")); + this.chain = new MockFilterChain() { + @Override + public void doFilter(ServletRequest request, ServletResponse response) + throws IOException, ServletException { + super.doFilter(request, response); + throw new NestedServletException("Wrapper", new RuntimeException("BAD")); + } + }; + this.filter.doFilter(this.request, this.response, this.chain); + assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(), + equalTo(500)); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE), + equalTo((Object) 500)); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE), + equalTo((Object) "BAD")); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE), + equalTo((Object) RuntimeException.class.getName())); + assertThat(this.request.getAttribute(RequestDispatcher.ERROR_REQUEST_URI), + equalTo((Object) "/test/path")); + assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), equalTo("/500")); + } + private void setUpAsyncDispatch() throws Exception { this.request.setAsyncSupported(true); this.request.setAsyncStarted(true);