From a1e0f01103f2ba1d2417e946850ac3e47f820b9b Mon Sep 17 00:00:00 2001 From: Michael Stummvoll Date: Fri, 16 Jan 2015 11:07:00 +0100 Subject: [PATCH] Support `error.include-stacktrace` configuration Add `error.include-stacktrace` configuration option to determine when stack trace attributes should be added. Options are `never`, `always` or `on-trace-param`. Prior to this commit stacktrace attributes were never includes in HTML responses, and only included in JSON responses when a `trace` request parameter was set to `true`. This has now been changed so that, by default, stacktrace attributes are never included. The BasicErrorController includes a protected method that can be overridden if includes should be different depending on the `produces` mapping. Closes gh-2371 --- .../web/AbstractErrorController.java | 18 +-- .../web/BasicErrorController.java | 63 ++++++++- .../web/ErrorMvcAutoConfiguration.java | 12 +- .../autoconfigure/web/ErrorProperties.java | 79 +++++++++++ .../BasicErrorControllerIntegrationTests.java | 128 ++++++++++++++---- .../appendix-application-properties.adoc | 4 + 6 files changed, 252 insertions(+), 52 deletions(-) create mode 100644 spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorProperties.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/AbstractErrorController.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/AbstractErrorController.java index ec77f77cf4..0c42240fb6 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/AbstractErrorController.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/AbstractErrorController.java @@ -43,6 +43,13 @@ public abstract class AbstractErrorController implements ErrorController { this.errorAttributes = errorAttributes; } + protected Map getErrorAttributes(HttpServletRequest request, + boolean includeStackTrace) { + RequestAttributes requestAttributes = new ServletRequestAttributes(request); + return this.errorAttributes.getErrorAttributes(requestAttributes, + includeStackTrace); + } + protected boolean getTraceParameter(HttpServletRequest request) { String parameter = request.getParameter("trace"); if (parameter == null) { @@ -51,17 +58,6 @@ public abstract class AbstractErrorController implements ErrorController { return !"false".equals(parameter.toLowerCase()); } - protected Map getErrorAttributes(HttpServletRequest request) { - return getErrorAttributes(request, getTraceParameter(request)); - } - - protected Map getErrorAttributes(HttpServletRequest request, - boolean includeStackTrace) { - RequestAttributes requestAttributes = new ServletRequestAttributes(request); - return this.errorAttributes.getErrorAttributes(requestAttributes, - includeStackTrace); - } - protected HttpStatus getStatus(HttpServletRequest request) { Integer statusCode = (Integer) request .getAttribute("javax.servlet.error.status_code"); diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java index 6fa3278fbb..11b82f7d4f 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java @@ -20,11 +20,13 @@ import java.util.Map; import javax.servlet.http.HttpServletRequest; -import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.web.ErrorProperties.IncludeStacktrace; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; +import org.springframework.util.Assert; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.servlet.ModelAndView; @@ -37,35 +39,84 @@ import org.springframework.web.servlet.ModelAndView; * * @author Dave Syer * @author Phillip Webb + * @author Michael Stummvoll + * @author Stephane Nicoll * @see ErrorAttributes + * @see ErrorProperties */ @Controller @RequestMapping("${error.path:/error}") public class BasicErrorController extends AbstractErrorController { - @Value("${error.path:/error}") - private String errorPath; + private final ErrorProperties errorProperties; + /** + * Create a new {@link BasicErrorController} instance. + * @param errorAttributes the error attributes + * @deprecated since 1.3.0 in favor of + * {@link #BasicErrorController(ErrorAttributes, ErrorProperties)} + */ + @Deprecated public BasicErrorController(ErrorAttributes errorAttributes) { + this(errorAttributes, new ErrorProperties()); + } + + /** + * Create a new {@link BasicErrorController} instance. + * @param errorAttributes the error attributes + * @param errorProperties configuration properties + */ + public BasicErrorController(ErrorAttributes errorAttributes, + ErrorProperties errorProperties) { super(errorAttributes); + Assert.notNull(errorProperties, "ErrorProperties must not be null"); + this.errorProperties = errorProperties; } @Override public String getErrorPath() { - return this.errorPath; + return this.errorProperties.getPath(); } @RequestMapping(produces = "text/html") public ModelAndView errorHtml(HttpServletRequest request) { - return new ModelAndView("error", getErrorAttributes(request, false)); + Map model = getErrorAttributes(request, + isIncludeStackTrace(request, MediaType.TEXT_HTML)); + return new ModelAndView("error", model); } @RequestMapping @ResponseBody public ResponseEntity> error(HttpServletRequest request) { - Map body = getErrorAttributes(request); + Map body = getErrorAttributes(request, + isIncludeStackTrace(request, MediaType.ALL)); HttpStatus status = getStatus(request); return new ResponseEntity>(body, status); } + /** + * Determine if the stacktrace attribute should be included. + * @param request the source request + * @param produces the media type produced (or {@code MediaType.ALL}) + * @return if the stacktrace attribute should be included + */ + protected boolean isIncludeStackTrace(HttpServletRequest request, MediaType produces) { + IncludeStacktrace include = getErrorProperties().getIncludeStacktrace(); + if (include == IncludeStacktrace.ALWAYS) { + return true; + } + if (include == IncludeStacktrace.ON_TRACE_PARAM) { + return getTraceParameter(request); + } + return false; + } + + /** + * Provide access to the error properties. + * @return the error properties + */ + protected final ErrorProperties getErrorProperties() { + return this.errorProperties; + } + } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorMvcAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorMvcAutoConfiguration.java index 20be08cfd8..6caa84d1ed 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorMvcAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorMvcAutoConfiguration.java @@ -25,7 +25,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionOutcome; @@ -39,6 +38,7 @@ import org.springframework.boot.autoconfigure.template.TemplateAvailabilityProvi import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainer; import org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer; import org.springframework.boot.context.embedded.ErrorPage; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ConditionContext; import org.springframework.context.annotation.Conditional; @@ -63,18 +63,20 @@ import org.springframework.web.util.HtmlUtils; * * @author Dave Syer * @author Andy Wilkinson + * @author Stephane Nicoll */ @ConditionalOnClass({ Servlet.class, DispatcherServlet.class }) @ConditionalOnWebApplication // Ensure this loads before the main WebMvcAutoConfiguration so that the error View is // available @AutoConfigureBefore(WebMvcAutoConfiguration.class) +@EnableConfigurationProperties(ErrorProperties.class) @Configuration public class ErrorMvcAutoConfiguration implements EmbeddedServletContainerCustomizer, Ordered { - @Value("${error.path:/error}") - private String errorPath = "/error"; + @Autowired + private ErrorProperties errorProperties; @Autowired private ServerProperties properties; @@ -93,13 +95,13 @@ public class ErrorMvcAutoConfiguration implements EmbeddedServletContainerCustom @Bean @ConditionalOnMissingBean(value = ErrorController.class, search = SearchStrategy.CURRENT) public BasicErrorController basicErrorController(ErrorAttributes errorAttributes) { - return new BasicErrorController(errorAttributes); + return new BasicErrorController(errorAttributes, this.errorProperties); } @Override public void customize(ConfigurableEmbeddedServletContainer container) { container.addErrorPages(new ErrorPage(this.properties.getServletPrefix() - + this.errorPath)); + + this.errorProperties.getPath())); } @Configuration diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorProperties.java new file mode 100644 index 0000000000..c7974a841a --- /dev/null +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ErrorProperties.java @@ -0,0 +1,79 @@ +/* + * 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 org.springframework.boot.autoconfigure.web; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Configuration properties for web error handling. + * + * @author Michael Stummvoll + * @author Stephane Nicoll + * @since 1.3.0 + */ +@ConfigurationProperties("error") +public class ErrorProperties { + + /** + * Path of the error controller. + */ + private String path = "/error"; + + /** + * When to include a "stacktrace" attribute. + */ + private IncludeStacktrace includeStacktrace = IncludeStacktrace.NEVER; + + public String getPath() { + return this.path; + } + + public void setPath(String path) { + this.path = path; + } + + public IncludeStacktrace getIncludeStacktrace() { + return this.includeStacktrace; + } + + public void setIncludeStacktrace(IncludeStacktrace includeStacktrace) { + this.includeStacktrace = includeStacktrace; + } + + /** + * Include Stacktrace attribute options. + */ + public enum IncludeStacktrace { + + /** + * Never add stacktrace information. + */ + NEVER, + + /** + * Always add stacktrace information. + */ + ALWAYS, + + /** + * Add stacktrace information when the "trace" request parameter is "true". + */ + ON_TRACE_PARAM + + } + +} diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerIntegrationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerIntegrationTests.java index e68aa2ef77..6360b7a02b 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerIntegrationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * 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. @@ -17,20 +17,24 @@ package org.springframework.boot.autoconfigure.web; import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.web.BasicErrorControllerIntegrationTests.TestConfiguration; import org.springframework.boot.autoconfigure.web.BasicErrorControllerMockMvcTests.MinimalWebConfiguration; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; +import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; @@ -48,15 +52,17 @@ import org.springframework.web.servlet.View; import org.springframework.web.servlet.view.AbstractView; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.endsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** - * Tests for {@link BasicErrorController} using {@link IntegrationTest} that hit a real - * HTTP server. + * Tests for {@link BasicErrorController} using a real HTTP server. * * @author Phillip Webb * @author Dave Syer + * @author Stephane Nicoll */ @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(TestConfiguration.class) @@ -65,50 +71,87 @@ import static org.junit.Assert.assertThat; @IntegrationTest("server.port=0") public class BasicErrorControllerIntegrationTests { - @Value("${local.server.port}") - private int port; + private ConfigurableApplicationContext context; + + @After + public void closeContext() { + if (this.context != null) { + this.context.close(); + } + } @Test @SuppressWarnings("rawtypes") public void testErrorForMachineClient() throws Exception { + load(); + ResponseEntity entity = new TestRestTemplate().getForEntity( + createUrl("?trace=true"), Map.class); + assertErrorAttributes(entity.getBody(), "500", "" + "Internal Server Error", + IllegalStateException.class, "Expected!", "/"); + assertFalse("trace parameter should not be set", + entity.getBody().containsKey("trace")); + } + + @Test + @SuppressWarnings("rawtypes") + public void testErrorForMachineClientTracePramamStacktrace() throws Exception { + load("--error.include-stacktrace=on-trace-param"); + ResponseEntity entity = new TestRestTemplate().getForEntity( + createUrl("?trace=true"), Map.class); + assertErrorAttributes(entity.getBody(), "500", "" + "Internal Server Error", + IllegalStateException.class, "Expected!", "/"); + assertTrue("trace parameter should be set", entity.getBody().containsKey("trace")); + } + + @Test + @SuppressWarnings("rawtypes") + public void testErrorForMachineClientNoStacktrace() throws Exception { + load("--error.include-stacktrace=never"); ResponseEntity entity = new TestRestTemplate().getForEntity( - "http://localhost:" + this.port, Map.class); - String body = entity.getBody().toString(); - assertThat(body, endsWith("status=500, " + "error=Internal Server Error, " - + "exception=java.lang.IllegalStateException, " + "message=Expected!, " - + "path=/}")); + createUrl("?trace=true"), Map.class); + assertErrorAttributes(entity.getBody(), "500", "" + "Internal Server Error", + IllegalStateException.class, "Expected!", "/"); + assertFalse("trace parameter should not be set", + entity.getBody().containsKey("trace")); + } + + @Test + @SuppressWarnings("rawtypes") + public void testErrorForMachineClientAlwaysStacktrace() throws Exception { + load("--error.include-stacktrace=always"); + ResponseEntity entity = new TestRestTemplate().getForEntity( + createUrl("?trace=false"), Map.class); + assertErrorAttributes(entity.getBody(), "500", "" + "Internal Server Error", + IllegalStateException.class, "Expected!", "/"); + assertTrue("trace parameter should be set", entity.getBody().containsKey("trace")); } @Test @SuppressWarnings("rawtypes") public void testErrorForAnnotatedException() throws Exception { + load(); ResponseEntity entity = new TestRestTemplate().getForEntity( - "http://localhost:" + this.port + "/annotated", Map.class); - assertThat( - entity.getBody().toString(), - endsWith("status=400, " - + "error=Bad Request, " - + "exception=org.springframework.boot.autoconfigure.web.BasicErrorControllerIntegrationTests$TestConfiguration$Errors$ExpectedException, " - + "message=Expected!, " + "path=/annotated}")); + createUrl("/annotated"), Map.class); + assertErrorAttributes(entity.getBody(), "400", "Bad Request", + TestConfiguration.Errors.ExpectedException.class, "Expected!", + "/annotated"); } @Test public void testErrorForAnnotatedNoReasonException() throws Exception { - ResponseEntity entity = new TestRestTemplate().getForEntity( - "http://localhost:" + this.port + "/annotatedNoReason", Object.class); - assertThat( - entity.getBody().toString(), - endsWith("status=406, " - + "error=Not Acceptable, " - + "exception=org.springframework.boot.autoconfigure.web.BasicErrorControllerIntegrationTests$TestConfiguration$Errors$NoReasonExpectedException, " - + "message=Expected message, " + "path=/annotatedNoReason}")); + load(); + ResponseEntity entity = new TestRestTemplate().getForEntity( + createUrl("/annotatedNoReason"), Map.class); + assertErrorAttributes(entity.getBody(), "406", "Not Acceptable", + TestConfiguration.Errors.NoReasonExpectedException.class, + "Expected message", "/annotatedNoReason"); } @Test @SuppressWarnings("rawtypes") public void testBindingExceptionForMachineClient() throws Exception { - RequestEntity request = RequestEntity - .get(URI.create("http://localhost:" + this.port + "/bind")) + load(); + RequestEntity request = RequestEntity.get(URI.create(createUrl("/bind"))) .accept(MediaType.APPLICATION_JSON).build(); ResponseEntity entity = new TestRestTemplate().exchange(request, Map.class); String resp = entity.getBody().toString(); @@ -118,6 +161,31 @@ public class BasicErrorControllerIntegrationTests { assertThat(resp, containsString("org.springframework.validation.BindException")); } + private void assertErrorAttributes(Map content, String status, String error, + Class exception, String message, String path) { + assertEquals("Wrong status", status, content.get("status")); + assertEquals("Wrong error", error, content.get("error")); + assertEquals("Wrong exception", exception.getName(), content.get("exception")); + assertEquals("Wrong message", message, content.get("message")); + assertEquals("Wrong path", path, content.get("path")); + } + + private String createUrl(String path) { + int port = this.context.getEnvironment().getProperty("local.server.port", + int.class); + return "http://localhost:" + port + path; + } + + private void load(String... arguments) { + List args = new ArrayList(); + args.add("--server.port=0"); + if (arguments != null) { + args.addAll(Arrays.asList(arguments)); + } + this.context = SpringApplication.run(TestConfiguration.class, + args.toArray(new String[args.size()])); + } + @Configuration @MinimalWebConfiguration public static class TestConfiguration { @@ -162,7 +230,7 @@ public class BasicErrorControllerIntegrationTests { } @RequestMapping("/bind") - public String bind(HttpServletRequest request) throws Exception { + public String bind() throws Exception { BindException error = new BindException(this, "test"); error.rejectValue("foo", "bar.error"); throw error; diff --git a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 17c10a3c41..0b08a3fcb6 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -136,6 +136,10 @@ content into your application; rather pick only the properties that you need. server.undertow.io-threads= # number of I/O threads to create for the worker server.undertow.worker-threads= # number of worker threads + # ERROR HANDLING ({sc-spring-boot-autoconfigure}/web/ErrorProperties.{sc-ext}[ErrorProperties]) + error.path=/error # the error path + error.include-stacktrace=never # when to include a stacktrace attribute (never/alway/on-trace-param) + # SPRING MVC ({sc-spring-boot-autoconfigure}/web/WebMvcProperties.{sc-ext}[WebMvcProperties]) spring.mvc.locale= # set fixed locale, e.g. en_UK spring.mvc.date-format= # set fixed date format, e.g. dd/MM/yyyy