From 5f250ebbc16ba32d026060565860cc1c7bf25820 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 11 Sep 2015 12:25:48 -0700 Subject: [PATCH 1/3] Add exception endpoints to tomcat-jsp sample Update `spring-boot-sample-tomcat-jsp` to include endpoints that trigger exceptions. Primarily to aid testing of the ErrorPageFilter. See gh-2745 --- .../src/main/java/sample/jsp/MyException.java | 25 +++++++++++++++ .../main/java/sample/jsp/MyRestResponse.java | 31 +++++++++++++++++++ .../java/sample/jsp/WelcomeController.java | 20 ++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyException.java create mode 100644 spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyRestResponse.java diff --git a/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyException.java b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyException.java new file mode 100644 index 00000000000..2974aa9b41c --- /dev/null +++ b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyException.java @@ -0,0 +1,25 @@ +/* + * Copyright 2012-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package sample.jsp; + +public class MyException extends RuntimeException { + + public MyException(String message) { + super(message); + } + +} diff --git a/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyRestResponse.java b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyRestResponse.java new file mode 100644 index 00000000000..c45fdc77d35 --- /dev/null +++ b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/MyRestResponse.java @@ -0,0 +1,31 @@ +/* + * Copyright 2012-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package sample.jsp; + +public class MyRestResponse { + + private String message; + + public MyRestResponse(String message) { + this.message = message; + } + + public String getMessage() { + return this.message; + } + +} diff --git a/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/WelcomeController.java b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/WelcomeController.java index 03d439caa3c..0c90e86ad32 100644 --- a/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/WelcomeController.java +++ b/spring-boot-samples/spring-boot-sample-tomcat-jsp/src/main/java/sample/jsp/WelcomeController.java @@ -20,8 +20,12 @@ import java.util.Date; import java.util.Map; import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; @Controller public class WelcomeController { @@ -36,4 +40,20 @@ public class WelcomeController { return "welcome"; } + @RequestMapping("/fail") + public String fail() { + throw new MyException("Oh dear!"); + } + + @RequestMapping("/fail2") + public String fail2() { + throw new IllegalStateException(); + } + + @ExceptionHandler(MyException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public @ResponseBody MyRestResponse handleMyRuntimeException(MyException exception) { + return new MyRestResponse("Some data I want to send back to the client."); + } + } From de48223a2ebd8da2677c9f5faa0bc63a6f0bbeb3 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 11 Sep 2015 12:27:39 -0700 Subject: [PATCH 2/3] Only handle status errors when sendError is called Update ErrorPageFilter to only handle errors when `response.sendError` has been called. This should allow custom @ExceptionHandlers to completely handle errors and return custom status codes without triggering the "Cannot forward to error page" log message. The Javadoc for sendError states: "The server defaults to creating the response to look like an HTML-formatted server error page containing the specified message" Where as setStatus states "This method is used to set the return status code when there is no error " Fixes gh-2745 --- .../boot/context/web/ErrorPageFilter.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 c772c3ba2ac..2c247507c66 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 @@ -114,9 +114,9 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine ErrorWrapperResponse wrapped = new ErrorWrapperResponse(response); try { chain.doFilter(request, wrapped); - int status = wrapped.getStatus(); - if (status >= 400) { - handleErrorStatus(request, response, status, wrapped.getMessage()); + if (wrapped.hasErrorToSend()) { + handleErrorStatus(request, response, wrapped.getStatus(), + wrapped.getMessage()); response.flushBuffer(); } else if (!request.isAsyncStarted() && !response.isCommitted()) { @@ -140,7 +140,6 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine handleCommittedResponse(request, null); return; } - String errorPath = getErrorPath(this.statuses, status); if (errorPath == null) { response.sendError(status, message); @@ -321,6 +320,10 @@ public class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContaine return this.message; } + public boolean hasErrorToSend() { + return this.hasErrorToSend; + } + } } From 11d59df3fd71a656a7869ace4303a8d72e6f777c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 11 Sep 2015 13:01:14 -0700 Subject: [PATCH 3/3] Add registerErrorPageFilter option flag Update SpringBootServletInitializer with a registerErrorPageFilter flag that can be used to disable ErrorPageFilter registration. Fixes gh-3603 --- .../web/SpringBootServletInitializer.java | 16 +++++++++++++- .../SpringBootServletInitializerTests.java | 22 +++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/web/SpringBootServletInitializer.java b/spring-boot/src/main/java/org/springframework/boot/context/web/SpringBootServletInitializer.java index 3e711be5bdb..f30897780e4 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/web/SpringBootServletInitializer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/web/SpringBootServletInitializer.java @@ -64,6 +64,18 @@ public abstract class SpringBootServletInitializer implements WebApplicationInit protected final Log logger = LogFactory.getLog(getClass()); + private boolean registerErrorPageFilter = true; + + /** + * Set if the {@link ErrorPageFilter} should be registered. Set to {@code false} if + * error page mappings should be handled via the Servlet container and not Spring + * Boot. + * @param registerErrorPageFilter if the {@link ErrorPageFilter} should be registered. + */ + protected final void setRegisterErrorPageFilter(boolean registerErrorPageFilter) { + this.registerErrorPageFilter = registerErrorPageFilter; + } + @Override public void onStartup(ServletContext servletContext) throws ServletException { WebApplicationContext rootAppContext = createRootApplicationContext(servletContext); @@ -106,7 +118,9 @@ public abstract class SpringBootServletInitializer implements WebApplicationInit "No SpringApplication sources have been defined. Either override the " + "configure method or add an @Configuration annotation"); // Ensure error pages are registered - application.getSources().add(ErrorPageFilter.class); + if (this.registerErrorPageFilter) { + application.getSources().add(ErrorPageFilter.class); + } return run(application); } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/web/SpringBootServletInitializerTests.java b/spring-boot/src/test/java/org/springframework/boot/context/web/SpringBootServletInitializerTests.java index ddd796b6bef..643f970eb36 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/web/SpringBootServletInitializerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/web/SpringBootServletInitializerTests.java @@ -38,7 +38,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; /** - * Tests for {@link SpringBootServletInitializerTests}. + * Tests for {@link SpringBootServletInitializer}. * * @author Phillip Webb * @author Andy Wilkinson @@ -75,8 +75,8 @@ public class SpringBootServletInitializerTests { equalToSet(Config.class, ErrorPageFilter.class)); } - @SuppressWarnings("rawtypes") @Test + @SuppressWarnings("rawtypes") public void mainClassHasSensibleDefault() throws Exception { new WithConfigurationAnnotation() .createRootApplicationContext(this.servletContext); @@ -86,6 +86,14 @@ public class SpringBootServletInitializerTests { is(equalTo((Class) WithConfigurationAnnotation.class))); } + @Test + public void withErrorPageFilterNotRegistered() throws Exception { + new WithErrorPageFilterNotRegistered() + .createRootApplicationContext(this.servletContext); + assertThat(this.application.getSources(), + equalToSet(WithErrorPageFilterNotRegistered.class)); + } + private Matcher> equalToSet(Object... items) { Set set = new LinkedHashSet(); Collections.addAll(set, items); @@ -115,6 +123,16 @@ public class SpringBootServletInitializerTests { } + @Configuration + public class WithErrorPageFilterNotRegistered extends + MockSpringBootServletInitializer { + + public WithErrorPageFilterNotRegistered() { + setRegisterErrorPageFilter(false); + } + + } + @Configuration public static class Config {