From 158933c3e57e921a0339cdb7635b12ae0ad89832 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Wed, 6 May 2020 16:31:30 -0500 Subject: [PATCH] Improve API of ErrorAttributes and DefaultErrorAttributes This commit improves the backward-compatibility of the ErrorAttributes interfaces by providing a default implementation of a new method. It also encapsulates several parameters that control the inclusion or exclusion of error attributes into a new ErrorAttributeOptions type to make it easier and less intrusive to add additional options in the future. This encapsulation also makes the handling of the includeException option more similar to other options. Fixes gh-21324 --- .../web/servlet/ManagementErrorEndpoint.java | 22 ++- .../servlet/ManagementErrorEndpointTests.java | 61 ++++++++ .../AbstractErrorWebExceptionHandler.java | 17 +-- .../DefaultErrorWebExceptionHandler.java | 31 ++-- .../error/ErrorWebFluxAutoConfiguration.java | 5 +- .../error/AbstractErrorController.java | 13 +- .../servlet/error/BasicErrorController.java | 27 +++- .../error/ErrorMvcAutoConfiguration.java | 5 +- ...orWebExceptionHandlerIntegrationTests.java | 69 ++++++++- .../DefaultErrorWebExceptionHandlerTests.java | 4 +- .../error/ErrorMvcAutoConfigurationTests.java | 14 +- .../boot/web/error/ErrorAttributeOptions.java | 140 ++++++++++++++++++ .../boot/web/error/package-info.java | 20 +++ .../error/DefaultErrorAttributes.java | 69 +++++---- .../web/reactive/error/ErrorAttributes.java | 19 ++- .../servlet/error/DefaultErrorAttributes.java | 79 +++++----- .../web/servlet/error/ErrorAttributes.java | 18 ++- .../error/DefaultErrorAttributesTests.java | 60 ++++---- .../error/DefaultErrorAttributesTests.java | 78 ++++++---- src/checkstyle/import-control.xml | 10 ++ 20 files changed, 586 insertions(+), 175 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/ErrorAttributeOptions.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/package-info.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java index 81f3e9099f..38c6337755 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.autoconfigure.web.servlet; import java.util.Map; import org.springframework.boot.autoconfigure.web.ErrorProperties; +import org.springframework.boot.web.error.ErrorAttributeOptions; import org.springframework.boot.web.servlet.error.ErrorAttributes; import org.springframework.boot.web.servlet.error.ErrorController; import org.springframework.stereotype.Controller; @@ -53,10 +54,27 @@ public class ManagementErrorEndpoint { @RequestMapping("${server.error.path:${error.path:/error}}") @ResponseBody public Map invoke(ServletWebRequest request) { - return this.errorAttributes.getErrorAttributes(request, includeStackTrace(request), includeMessage(request), - includeBindingErrors(request)); + return this.errorAttributes.getErrorAttributes(request, getErrorAttributeOptions(request)); } + private ErrorAttributeOptions getErrorAttributeOptions(ServletWebRequest request) { + ErrorAttributeOptions options = ErrorAttributeOptions.defaults(); + if (this.errorProperties.isIncludeException()) { + options = options.including(ErrorAttributeOptions.Include.EXCEPTION); + } + if (includeStackTrace(request)) { + options = options.including(ErrorAttributeOptions.Include.STACK_TRACE); + } + if (includeMessage(request)) { + options = options.including(ErrorAttributeOptions.Include.MESSAGE); + } + if (includeBindingErrors(request)) { + options = options.including(ErrorAttributeOptions.Include.BINDING_ERRORS); + } + return options; + } + + @SuppressWarnings("deprecation") private boolean includeStackTrace(ServletWebRequest request) { switch (this.errorProperties.getIncludeStacktrace()) { case ALWAYS: diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java index e9ace84d8d..1d11874189 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.autoconfigure.web.servlet; +import java.util.HashMap; import java.util.Map; import org.junit.jupiter.api.BeforeEach; @@ -26,6 +27,7 @@ import org.springframework.boot.web.servlet.error.DefaultErrorAttributes; import org.springframework.boot.web.servlet.error.ErrorAttributes; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.context.request.WebRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -103,4 +105,63 @@ class ManagementErrorEndpointTests { assertThat(response).doesNotContainKey("trace"); } + @Test + void errorResponseWithCustomErrorAttributesUsingDeprecatedApi() { + ErrorAttributes attributes = new ErrorAttributes() { + @Override + public Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) { + Map response = new HashMap<>(); + response.put("message", "An error occurred"); + return response; + } + + @Override + public Throwable getError(WebRequest webRequest) { + return null; + } + }; + ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties); + Map response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest())); + assertThat(response).hasSize(1); + assertThat(response).containsEntry("message", "An error occurred"); + } + + @Test + void errorResponseWithDefaultErrorAttributesSubclassUsingDeprecatedApiAndDelegation() { + ErrorAttributes attributes = new DefaultErrorAttributes() { + @Override + @SuppressWarnings("deprecation") + public Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) { + Map response = super.getErrorAttributes(webRequest, includeStackTrace); + response.put("error", "custom error"); + response.put("custom", "value"); + response.remove("path"); + return response; + } + }; + ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties); + Map response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest())); + assertThat(response).containsEntry("error", "custom error"); + assertThat(response).containsEntry("custom", "value"); + assertThat(response).doesNotContainKey("path"); + assertThat(response).containsKey("timestamp"); + } + + @Test + void errorResponseWithDefaultErrorAttributesSubclassUsingDeprecatedApiWithoutDelegation() { + ErrorAttributes attributes = new DefaultErrorAttributes() { + @Override + @SuppressWarnings("deprecation") + public Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) { + Map response = new HashMap<>(); + response.put("error", "custom error"); + return response; + } + }; + ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties); + Map response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest())); + assertThat(response).hasSize(1); + assertThat(response).containsEntry("error", "custom error"); + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java index 05f3b63550..86e87c38d0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java @@ -29,6 +29,8 @@ import reactor.core.publisher.Mono; import org.springframework.beans.factory.InitializingBean; import org.springframework.boot.autoconfigure.template.TemplateAvailabilityProviders; import org.springframework.boot.autoconfigure.web.ResourceProperties; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.boot.web.reactive.error.ErrorWebExceptionHandler; import org.springframework.context.ApplicationContext; @@ -134,26 +136,23 @@ public abstract class AbstractErrorWebExceptionHandler implements ErrorWebExcept * @param includeStackTrace whether to include the error stacktrace information * @return the error attributes as a Map * @deprecated since 2.3.0 in favor of - * {@link #getErrorAttributes(ServerRequest, boolean, boolean, boolean)} + * {@link #getErrorAttributes(ServerRequest, ErrorAttributeOptions)} */ @Deprecated protected Map getErrorAttributes(ServerRequest request, boolean includeStackTrace) { - return this.errorAttributes.getErrorAttributes(request, includeStackTrace, false, false); + return getErrorAttributes(request, + (includeStackTrace) ? ErrorAttributeOptions.of(Include.STACK_TRACE) : ErrorAttributeOptions.defaults()); } /** * Extract the error attributes from the current request, to be used to populate error * views or JSON payloads. * @param request the source request - * @param includeStackTrace whether to include the stacktrace attribute - * @param includeMessage whether to include the message attribute - * @param includeBindingErrors whether to include the errors attribute + * @param options options to control error attributes * @return the error attributes as a Map */ - protected Map getErrorAttributes(ServerRequest request, boolean includeStackTrace, - boolean includeMessage, boolean includeBindingErrors) { - return this.errorAttributes.getErrorAttributes(request, includeStackTrace, includeMessage, - includeBindingErrors); + protected Map getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) { + return this.errorAttributes.getErrorAttributes(request, options); } /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java index cacf6ce5bf..ea6cd7edf9 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java @@ -28,6 +28,8 @@ import reactor.core.publisher.Mono; import org.springframework.boot.autoconfigure.web.ErrorProperties; import org.springframework.boot.autoconfigure.web.ResourceProperties; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.context.ApplicationContext; import org.springframework.http.HttpStatus; @@ -113,11 +115,7 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa * @return a {@code Publisher} of the HTTP response */ protected Mono renderErrorView(ServerRequest request) { - boolean includeStackTrace = isIncludeStackTrace(request, MediaType.TEXT_HTML); - boolean includeMessage = isIncludeMessage(request, MediaType.TEXT_HTML); - boolean includeBindingErrors = isIncludeBindingErrors(request, MediaType.TEXT_HTML); - Map error = getErrorAttributes(request, includeStackTrace, includeMessage, - includeBindingErrors); + Map error = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.TEXT_HTML)); int errorStatus = getHttpStatus(error); ServerResponse.BodyBuilder responseBody = ServerResponse.status(errorStatus).contentType(TEXT_HTML_UTF8); return Flux.just(getData(errorStatus).toArray(new String[] {})) @@ -144,15 +142,28 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa * @return a {@code Publisher} of the HTTP response */ protected Mono renderErrorResponse(ServerRequest request) { - boolean includeStackTrace = isIncludeStackTrace(request, MediaType.ALL); - boolean includeMessage = isIncludeMessage(request, MediaType.ALL); - boolean includeBindingErrors = isIncludeBindingErrors(request, MediaType.ALL); - Map error = getErrorAttributes(request, includeStackTrace, includeMessage, - includeBindingErrors); + Map error = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.ALL)); return ServerResponse.status(getHttpStatus(error)).contentType(MediaType.APPLICATION_JSON) .body(BodyInserters.fromValue(error)); } + protected ErrorAttributeOptions getErrorAttributeOptions(ServerRequest request, MediaType mediaType) { + ErrorAttributeOptions options = ErrorAttributeOptions.defaults(); + if (this.errorProperties.isIncludeException()) { + options = options.including(Include.EXCEPTION); + } + if (isIncludeStackTrace(request, mediaType)) { + options = options.including(Include.STACK_TRACE); + } + if (isIncludeMessage(request, mediaType)) { + options = options.including(Include.MESSAGE); + } + if (isIncludeBindingErrors(request, mediaType)) { + options = options.including(Include.BINDING_ERRORS); + } + return options; + } + /** * Determine if the stacktrace attribute should be included. * @param request the source request diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java index 7b95da860b..c7b3ca7a82 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -45,6 +45,7 @@ import org.springframework.web.reactive.result.view.ViewResolver; * {@link org.springframework.web.server.WebExceptionHandler}. * * @author Brian Clozel + * @author Scott Frederick * @since 2.0.0 */ @Configuration(proxyBeanMethods = false) @@ -77,7 +78,7 @@ public class ErrorWebFluxAutoConfiguration { @Bean @ConditionalOnMissingBean(value = ErrorAttributes.class, search = SearchStrategy.CURRENT) public DefaultErrorAttributes errorAttributes() { - return new DefaultErrorAttributes(this.serverProperties.getError().isIncludeException()); + return new DefaultErrorAttributes(); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java index 33c6b09bc8..edbce0d914 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java @@ -24,6 +24,8 @@ import javax.servlet.RequestDispatcher; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.boot.web.servlet.error.ErrorAttributes; import org.springframework.boot.web.servlet.error.ErrorController; import org.springframework.core.annotation.AnnotationAwareOrderComparator; @@ -74,18 +76,17 @@ public abstract class AbstractErrorController implements ErrorController { * @param includeStackTrace if stack trace elements should be included * @return the error attributes * @deprecated since 2.3.0 in favor of - * {@link #getErrorAttributes(HttpServletRequest, boolean, boolean, boolean)} + * {@link #getErrorAttributes(HttpServletRequest, ErrorAttributeOptions)} */ @Deprecated protected Map getErrorAttributes(HttpServletRequest request, boolean includeStackTrace) { - return getErrorAttributes(request, includeStackTrace, false, false); + return getErrorAttributes(request, + (includeStackTrace) ? ErrorAttributeOptions.of(Include.STACK_TRACE) : ErrorAttributeOptions.defaults()); } - protected Map getErrorAttributes(HttpServletRequest request, boolean includeStackTrace, - boolean includeMessage, boolean includeBindingErrors) { + protected Map getErrorAttributes(HttpServletRequest request, ErrorAttributeOptions options) { WebRequest webRequest = new ServletWebRequest(request); - return this.errorAttributes.getErrorAttributes(webRequest, includeStackTrace, includeMessage, - includeBindingErrors); + return this.errorAttributes.getErrorAttributes(webRequest, options); } protected boolean getTraceParameter(HttpServletRequest request) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java index fceec6f519..5486cf9f17 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java @@ -24,6 +24,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.boot.autoconfigure.web.ErrorProperties; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.boot.web.servlet.error.ErrorAttributes; import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory; import org.springframework.http.HttpStatus; @@ -87,9 +89,8 @@ public class BasicErrorController extends AbstractErrorController { @RequestMapping(produces = MediaType.TEXT_HTML_VALUE) public ModelAndView errorHtml(HttpServletRequest request, HttpServletResponse response) { HttpStatus status = getStatus(request); - Map model = Collections.unmodifiableMap(getErrorAttributes(request, - isIncludeStackTrace(request, MediaType.TEXT_HTML), isIncludeMessage(request, MediaType.TEXT_HTML), - isIncludeBindingErrors(request, MediaType.TEXT_HTML))); + Map model = Collections + .unmodifiableMap(getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.TEXT_HTML))); response.setStatus(status.value()); ModelAndView modelAndView = resolveErrorView(request, response, status, model); return (modelAndView != null) ? modelAndView : new ModelAndView("error", model); @@ -101,8 +102,7 @@ public class BasicErrorController extends AbstractErrorController { if (status == HttpStatus.NO_CONTENT) { return new ResponseEntity<>(status); } - Map body = getErrorAttributes(request, isIncludeStackTrace(request, MediaType.ALL), - isIncludeMessage(request, MediaType.ALL), isIncludeBindingErrors(request, MediaType.TEXT_HTML)); + Map body = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.ALL)); return new ResponseEntity<>(body, status); } @@ -112,6 +112,23 @@ public class BasicErrorController extends AbstractErrorController { return ResponseEntity.status(status).build(); } + protected ErrorAttributeOptions getErrorAttributeOptions(HttpServletRequest request, MediaType mediaType) { + ErrorAttributeOptions options = ErrorAttributeOptions.defaults(); + if (this.errorProperties.isIncludeException()) { + options = options.including(Include.EXCEPTION); + } + if (isIncludeStackTrace(request, mediaType)) { + options = options.including(Include.STACK_TRACE); + } + if (isIncludeMessage(request, mediaType)) { + options = options.including(Include.MESSAGE); + } + if (isIncludeBindingErrors(request, mediaType)) { + options = options.including(Include.BINDING_ERRORS); + } + return options; + } + /** * Determine if the stacktrace attribute should be included. * @param request the source request diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java index f46ab0ef11..be8058662c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -81,6 +81,7 @@ import org.springframework.web.util.HtmlUtils; * @author Andy Wilkinson * @author Stephane Nicoll * @author Brian Clozel + * @author Scott Frederick * @since 1.0.0 */ @Configuration(proxyBeanMethods = false) @@ -100,7 +101,7 @@ public class ErrorMvcAutoConfiguration { @Bean @ConditionalOnMissingBean(value = ErrorAttributes.class, search = SearchStrategy.CURRENT) public DefaultErrorAttributes errorAttributes() { - return new DefaultErrorAttributes(this.serverProperties.getError().isIncludeException()); + return new DefaultErrorAttributes(); } @Bean diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java index a0209bec51..109558946a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java @@ -17,6 +17,8 @@ package org.springframework.boot.autoconfigure.web.reactive.error; import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; import javax.validation.Valid; @@ -34,6 +36,9 @@ import org.springframework.boot.test.context.assertj.AssertableReactiveWebApplic import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.boot.web.reactive.error.DefaultErrorAttributes; +import org.springframework.boot.web.reactive.error.ErrorAttributes; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -43,6 +48,7 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; @@ -64,7 +70,7 @@ class DefaultErrorWebExceptionHandlerIntegrationTests { private final LogIdFilter logIdFilter = new LogIdFilter(); - private ReactiveWebApplicationContextRunner contextRunner = new ReactiveWebApplicationContextRunner() + private final ReactiveWebApplicationContextRunner contextRunner = new ReactiveWebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class, HttpHandlerAutoConfiguration.class, WebFluxAutoConfiguration.class, ErrorWebFluxAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, @@ -343,6 +349,26 @@ class DefaultErrorWebExceptionHandlerIntegrationTests { }); } + @Test + void defaultErrorAttributesSubclassUsingDeprecatedApiAndDelegation() { + this.contextRunner.withUserConfiguration(CustomErrorAttributesWithDelegation.class).run((context) -> { + WebTestClient client = getWebClient(context); + client.get().uri("/badRequest").exchange().expectStatus().isBadRequest().expectBody().jsonPath("status") + .isEqualTo("400").jsonPath("error").isEqualTo("custom error").jsonPath("newAttribute") + .isEqualTo("value").jsonPath("path").doesNotExist(); + }); + } + + @Test + void defaultErrorAttributesSubclassUsingDeprecatedApiWithoutDelegation() { + this.contextRunner.withUserConfiguration(CustomErrorAttributesWithoutDelegation.class).run((context) -> { + WebTestClient client = getWebClient(context); + client.get().uri("/badRequest").exchange().expectStatus().isBadRequest().expectBody().jsonPath("status") + .isEqualTo("400").jsonPath("timestamp").doesNotExist().jsonPath("error").isEqualTo("custom error") + .jsonPath("path").doesNotExist(); + }); + } + private String getErrorTemplatesLocation() { String packageName = getClass().getPackage().getName(); return "classpath:/" + packageName.replace('.', '/') + "/templates/"; @@ -405,4 +431,45 @@ class DefaultErrorWebExceptionHandlerIntegrationTests { } + @Configuration(proxyBeanMethods = false) + static class CustomErrorAttributesWithDelegation { + + @Bean + ErrorAttributes errorAttributes() { + return new DefaultErrorAttributes() { + @Override + @SuppressWarnings("deprecation") + public Map getErrorAttributes(ServerRequest request, boolean includeStackTrace) { + Map errorAttributes = super.getErrorAttributes(request, includeStackTrace); + errorAttributes.put("error", "custom error"); + errorAttributes.put("newAttribute", "value"); + errorAttributes.remove("path"); + return errorAttributes; + } + + }; + } + + } + + @Configuration(proxyBeanMethods = false) + static class CustomErrorAttributesWithoutDelegation { + + @Bean + ErrorAttributes errorAttributes() { + return new DefaultErrorAttributes() { + @Override + @SuppressWarnings("deprecation") + public Map getErrorAttributes(ServerRequest request, boolean includeStackTrace) { + Map errorAttributes = new HashMap<>(); + errorAttributes.put("status", 400); + errorAttributes.put("error", "custom error"); + return errorAttributes; + } + + }; + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java index feaefe8c8b..3ad649c184 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java @@ -38,7 +38,6 @@ import org.springframework.web.server.adapter.HttpWebHandlerAdapter; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -66,8 +65,7 @@ class DefaultErrorWebExceptionHandlerTests { ResourceProperties resourceProperties = new ResourceProperties(); ErrorProperties errorProperties = new ErrorProperties(); ApplicationContext context = new AnnotationConfigReactiveWebApplicationContext(); - given(errorAttributes.getErrorAttributes(any(), anyBoolean(), anyBoolean(), anyBoolean())) - .willReturn(getErrorAttributes()); + given(errorAttributes.getErrorAttributes(any(), any())).willReturn(getErrorAttributes()); DefaultErrorWebExceptionHandler exceptionHandler = new DefaultErrorWebExceptionHandler(errorAttributes, resourceProperties, errorProperties, context); setupViewResolver(exceptionHandler); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfigurationTests.java index 631e7b97ab..e44fbb8ee8 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfigurationTests.java @@ -24,6 +24,8 @@ import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoC import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.boot.web.servlet.error.ErrorAttributes; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -37,11 +39,12 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link ErrorMvcAutoConfiguration}. * * @author Brian Clozel + * @author Scott Frederick */ @ExtendWith(OutputCaptureExtension.class) class ErrorMvcAutoConfigurationTests { - private WebApplicationContextRunner contextRunner = new WebApplicationContextRunner().withConfiguration( + private final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner().withConfiguration( AutoConfigurations.of(DispatcherServletAutoConfiguration.class, ErrorMvcAutoConfiguration.class)); @Test @@ -51,7 +54,7 @@ class ErrorMvcAutoConfigurationTests { ErrorAttributes errorAttributes = context.getBean(ErrorAttributes.class); DispatcherServletWebRequest webRequest = createWebRequest(new IllegalStateException("Exception message"), false); - errorView.render(errorAttributes.getErrorAttributes(webRequest, true, true, true), webRequest.getRequest(), + errorView.render(errorAttributes.getErrorAttributes(webRequest, withAllOptions()), webRequest.getRequest(), webRequest.getResponse()); assertThat(webRequest.getResponse().getContentType()).isEqualTo("text/html;charset=UTF-8"); String responseString = ((MockHttpServletResponse) webRequest.getResponse()).getContentAsString(); @@ -69,7 +72,7 @@ class ErrorMvcAutoConfigurationTests { ErrorAttributes errorAttributes = context.getBean(ErrorAttributes.class); DispatcherServletWebRequest webRequest = createWebRequest(new IllegalStateException("Exception message"), true); - errorView.render(errorAttributes.getErrorAttributes(webRequest, true, true, true), webRequest.getRequest(), + errorView.render(errorAttributes.getErrorAttributes(webRequest, withAllOptions()), webRequest.getRequest(), webRequest.getResponse()); assertThat(output).contains("Cannot render error page for request [/path] " + "and exception [Exception message] as the response has " @@ -89,4 +92,9 @@ class ErrorMvcAutoConfigurationTests { return webRequest; } + private ErrorAttributeOptions withAllOptions() { + return ErrorAttributeOptions.of(Include.EXCEPTION, Include.STACK_TRACE, Include.MESSAGE, + Include.BINDING_ERRORS); + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/ErrorAttributeOptions.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/ErrorAttributeOptions.java new file mode 100644 index 0000000000..b930aa346f --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/ErrorAttributeOptions.java @@ -0,0 +1,140 @@ +/* + * Copyright 2012-2020 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 + * + * https://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.web.error; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; + +/** + * Options controlling the contents of {@code ErrorAttributes}. + * + * @author Scott Frederick + * @author Phillip Webb + * @since 2.3.0 + */ +public final class ErrorAttributeOptions { + + private final Set includes; + + private ErrorAttributeOptions(Set includes) { + this.includes = includes; + } + + /** + * Get the option for including the specified attribute in the error response. + * @param include error attribute to get + * @return {@code true} if the {@code Include} attribute is included in the error + * response, {@code false} otherwise + */ + public boolean isIncluded(Include include) { + return this.includes.contains(include); + } + + /** + * Get all options for including attributes in the error response. + * @return {@code true} if the {@code Include} attribute is included in the error + * response, {@code false} otherwise + */ + public Set getIncludes() { + return this.includes; + } + + /** + * Return an {@code ErrorAttributeOptions} that includes the specified attribute + * {@link Include} options. + * @param includes error attributes to include + * @return an {@code ErrorAttributeOptions} + */ + public ErrorAttributeOptions including(Include... includes) { + EnumSet updated = (this.includes.isEmpty()) ? EnumSet.noneOf(Include.class) + : EnumSet.copyOf(this.includes); + updated.addAll(Arrays.asList(includes)); + return new ErrorAttributeOptions(Collections.unmodifiableSet(updated)); + } + + /** + * Return an {@code ErrorAttributeOptions} that excludes the specified attribute + * {@link Include} options. + * @param excludes error attributes to exclude + * @return an {@code ErrorAttributeOptions} + */ + public ErrorAttributeOptions excluding(Include... excludes) { + EnumSet updated = EnumSet.copyOf(this.includes); + updated.removeAll(Arrays.asList(excludes)); + return new ErrorAttributeOptions(Collections.unmodifiableSet(updated)); + } + + /** + * Create an {@code ErrorAttributeOptions} with defaults. + * @return an {@code ErrorAttributeOptions} + */ + public static ErrorAttributeOptions defaults() { + return of(); + } + + /** + * Create an {@code ErrorAttributeOptions} that includes the specified attribute + * {@link Include} options. + * @param includes error attributes to include + * @return an {@code ErrorAttributeOptions} + */ + public static ErrorAttributeOptions of(Include... includes) { + return of(Arrays.asList(includes)); + } + + /** + * Create an {@code ErrorAttributeOptions} that includes the specified attribute + * {@link Include} options. + * @param includes error attributes to include + * @return an {@code ErrorAttributeOptions} + */ + public static ErrorAttributeOptions of(Collection includes) { + return new ErrorAttributeOptions( + (includes.isEmpty()) ? Collections.emptySet() : Collections.unmodifiableSet(EnumSet.copyOf(includes))); + } + + /** + * Error attributes that can be included in an error response. + */ + public enum Include { + + /** + * Include the exception class name attribute. + */ + EXCEPTION, + + /** + * Include the stack trace attribute. + */ + STACK_TRACE, + + /** + * Include the message attribute. + */ + MESSAGE, + + /** + * Include the binding errors attribute. + */ + BINDING_ERRORS + + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/package-info.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/package-info.java new file mode 100644 index 0000000000..a82e9684ee --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright 2012-2020 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 + * + * https://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. + */ + +/** + * Spring Web error handling infrastructure. + */ +package org.springframework.boot.web.error; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java index 72168b8b97..4a323ea100 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java @@ -22,6 +22,8 @@ import java.util.Date; import java.util.LinkedHashMap; import java.util.Map; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; @@ -42,9 +44,10 @@ import org.springframework.web.server.ServerWebExchange; *
  • status - The status code
  • *
  • error - The error reason
  • *
  • exception - The class name of the root exception (if configured)
  • - *
  • message - The exception message
  • - *
  • errors - Any {@link ObjectError}s from a {@link BindingResult} exception - *
  • trace - The exception stack trace
  • + *
  • message - The exception message (if configured)
  • + *
  • errors - Any {@link ObjectError}s from a {@link BindingResult} exception (if + * configured)
  • + *
  • trace - The exception stack trace (if configured)
  • *
  • path - The URL path when the exception was raised
  • *
  • requestId - Unique ID associated with the current request
  • * @@ -60,20 +63,22 @@ public class DefaultErrorAttributes implements ErrorAttributes { private static final String ERROR_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR"; - private final boolean includeException; + private final Boolean includeException; /** - * Create a new {@link DefaultErrorAttributes} instance that does not include the - * "exception" attribute. + * Create a new {@link DefaultErrorAttributes} instance. */ public DefaultErrorAttributes() { - this(false); + this.includeException = null; } /** * Create a new {@link DefaultErrorAttributes} instance. * @param includeException whether to include the "exception" attribute + * @deprecated since 2.3.0 in favor of + * {@link ErrorAttributeOptions#including(Include...)} */ + @Deprecated public DefaultErrorAttributes(boolean includeException) { this.includeException = includeException; } @@ -81,12 +86,6 @@ public class DefaultErrorAttributes implements ErrorAttributes { @Override @Deprecated public Map getErrorAttributes(ServerRequest request, boolean includeStackTrace) { - return getErrorAttributes(request, includeStackTrace, false, false); - } - - @Override - public Map getErrorAttributes(ServerRequest request, boolean includeStackTrace, - boolean includeMessage, boolean includeBindingErrors) { Map errorAttributes = new LinkedHashMap<>(); errorAttributes.put("timestamp", new Date()); errorAttributes.put("path", request.path()); @@ -96,9 +95,30 @@ public class DefaultErrorAttributes implements ErrorAttributes { HttpStatus errorStatus = determineHttpStatus(error, responseStatusAnnotation); errorAttributes.put("status", errorStatus.value()); errorAttributes.put("error", errorStatus.getReasonPhrase()); - errorAttributes.put("message", determineMessage(error, responseStatusAnnotation, includeMessage)); + errorAttributes.put("message", determineMessage(error, responseStatusAnnotation)); errorAttributes.put("requestId", request.exchange().getRequest().getId()); - handleException(errorAttributes, determineException(error), includeStackTrace, includeBindingErrors); + handleException(errorAttributes, determineException(error)); + return errorAttributes; + } + + @Override + public Map getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) { + Map errorAttributes = getErrorAttributes(request, options.isIncluded(Include.STACK_TRACE)); + if (this.includeException != null) { + options = options.including(Include.EXCEPTION); + } + if (!options.isIncluded(Include.EXCEPTION)) { + errorAttributes.remove("exception"); + } + if (!options.isIncluded(Include.STACK_TRACE)) { + errorAttributes.remove("trace"); + } + if (!options.isIncluded(Include.MESSAGE) && errorAttributes.get("message") != null) { + errorAttributes.put("message", ""); + } + if (!options.isIncluded(Include.BINDING_ERRORS)) { + errorAttributes.remove("errors"); + } return errorAttributes; } @@ -109,11 +129,7 @@ public class DefaultErrorAttributes implements ErrorAttributes { return responseStatusAnnotation.getValue("code", HttpStatus.class).orElse(HttpStatus.INTERNAL_SERVER_ERROR); } - private String determineMessage(Throwable error, MergedAnnotation responseStatusAnnotation, - boolean includeMessage) { - if (!includeMessage) { - return ""; - } + private String determineMessage(Throwable error, MergedAnnotation responseStatusAnnotation) { if (error instanceof BindingResult) { return error.getMessage(); } @@ -141,15 +157,10 @@ public class DefaultErrorAttributes implements ErrorAttributes { errorAttributes.put("trace", stackTrace.toString()); } - private void handleException(Map errorAttributes, Throwable error, boolean includeStackTrace, - boolean includeBindingErrors) { - if (this.includeException) { - errorAttributes.put("exception", error.getClass().getName()); - } - if (includeStackTrace) { - addStackTrace(errorAttributes, error); - } - if (includeBindingErrors && (error instanceof BindingResult)) { + private void handleException(Map errorAttributes, Throwable error) { + errorAttributes.put("exception", error.getClass().getName()); + addStackTrace(errorAttributes, error); + if (error instanceof BindingResult) { BindingResult result = (BindingResult) error; if (result.hasErrors()) { errorAttributes.put("errors", result.getAllErrors()); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java index b84346caff..c1cd5d707e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java @@ -16,8 +16,11 @@ package org.springframework.boot.web.reactive.error; +import java.util.Collections; import java.util.Map; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; import org.springframework.web.server.ServerWebExchange; @@ -39,21 +42,23 @@ public interface ErrorAttributes { * @param includeStackTrace if stack trace attribute should be included * @return a map of error attributes * @deprecated since 2.3.0 in favor of - * {@link #getErrorAttributes(ServerRequest, boolean, boolean, boolean)} + * {@link #getErrorAttributes(ServerRequest, ErrorAttributeOptions)} */ - Map getErrorAttributes(ServerRequest request, boolean includeStackTrace); + @Deprecated + default Map getErrorAttributes(ServerRequest request, boolean includeStackTrace) { + return Collections.emptyMap(); + } /** * Return a {@link Map} of the error attributes. The map can be used as the model of * an error page, or returned as a {@link ServerResponse} body. * @param request the source request - * @param includeStackTrace if stack trace attribute should be included - * @param includeMessage if message attribute should be included - * @param includeBindingErrors if errors attribute should be included + * @param options options for error attribute contents * @return a map of error attributes */ - Map getErrorAttributes(ServerRequest request, boolean includeStackTrace, boolean includeMessage, - boolean includeBindingErrors); + default Map getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) { + return getErrorAttributes(request, options.isIncluded(Include.STACK_TRACE)); + } /** * Return the underlying cause of the error or {@code null} if the error cannot be diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java index f48edeec3c..addd854e78 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java @@ -27,6 +27,8 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; @@ -67,20 +69,22 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException private static final String ERROR_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR"; - private final boolean includeException; + private final Boolean includeException; /** - * Create a new {@link DefaultErrorAttributes} instance that does not include the - * "exception" attribute. + * Create a new {@link DefaultErrorAttributes} instance. */ public DefaultErrorAttributes() { - this(false); + this.includeException = null; } /** * Create a new {@link DefaultErrorAttributes} instance. * @param includeException whether to include the "exception" attribute + * @deprecated since 2.3.0 in favor of + * {@link ErrorAttributeOptions#including(Include...)} */ + @Deprecated public DefaultErrorAttributes(boolean includeException) { this.includeException = includeException; } @@ -104,20 +108,35 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException @Override @Deprecated public Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) { - return getErrorAttributes(webRequest, includeStackTrace, false, false); - } - - @Override - public Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace, - boolean includeMessage, boolean includeBindingErrors) { Map errorAttributes = new LinkedHashMap<>(); errorAttributes.put("timestamp", new Date()); addStatus(errorAttributes, webRequest); - addErrorDetails(errorAttributes, webRequest, includeStackTrace, includeMessage, includeBindingErrors); + addErrorDetails(errorAttributes, webRequest); addPath(errorAttributes, webRequest); return errorAttributes; } + @Override + public Map getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) { + Map errorAttributes = getErrorAttributes(webRequest, options.isIncluded(Include.STACK_TRACE)); + if (this.includeException != null) { + options = options.including(Include.EXCEPTION); + } + if (!options.isIncluded(Include.EXCEPTION)) { + errorAttributes.remove("exception"); + } + if (!options.isIncluded(Include.STACK_TRACE)) { + errorAttributes.remove("trace"); + } + if (!options.isIncluded(Include.MESSAGE) && errorAttributes.get("message") != null) { + errorAttributes.put("message", ""); + } + if (!options.isIncluded(Include.BINDING_ERRORS)) { + errorAttributes.remove("errors"); + } + return errorAttributes; + } + private void addStatus(Map errorAttributes, RequestAttributes requestAttributes) { Integer status = getAttribute(requestAttributes, RequestDispatcher.ERROR_STATUS_CODE); if (status == null) { @@ -135,40 +154,29 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException } } - private void addErrorDetails(Map errorAttributes, WebRequest webRequest, boolean includeStackTrace, - boolean includeMessage, boolean includeBindingErrors) { + private void addErrorDetails(Map errorAttributes, WebRequest webRequest) { Throwable error = getError(webRequest); if (error != null) { while (error instanceof ServletException && error.getCause() != null) { error = error.getCause(); } - if (this.includeException) { - errorAttributes.put("exception", error.getClass().getName()); - } - if (includeStackTrace) { - addStackTrace(errorAttributes, error); - } + errorAttributes.put("exception", error.getClass().getName()); + addStackTrace(errorAttributes, error); } - addErrorMessage(errorAttributes, webRequest, error, includeMessage, includeBindingErrors); + addErrorMessage(errorAttributes, webRequest, error); } - private void addErrorMessage(Map errorAttributes, WebRequest webRequest, Throwable error, - boolean includeMessage, boolean includeBindingErrors) { + private void addErrorMessage(Map errorAttributes, WebRequest webRequest, Throwable error) { BindingResult result = extractBindingResult(error); if (result == null) { - addExceptionErrorMessage(errorAttributes, webRequest, error, includeMessage); + addExceptionErrorMessage(errorAttributes, webRequest, error); } else { - addBindingResultErrorMessage(errorAttributes, result, includeMessage, includeBindingErrors); + addBindingResultErrorMessage(errorAttributes, result); } } - private void addExceptionErrorMessage(Map errorAttributes, WebRequest webRequest, Throwable error, - boolean includeMessage) { - if (!includeMessage) { - errorAttributes.put("message", ""); - return; - } + private void addExceptionErrorMessage(Map errorAttributes, WebRequest webRequest, Throwable error) { Object message = getAttribute(webRequest, RequestDispatcher.ERROR_MESSAGE); if (StringUtils.isEmpty(message) && error != null) { message = error.getMessage(); @@ -179,13 +187,10 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException errorAttributes.put("message", message); } - private void addBindingResultErrorMessage(Map errorAttributes, BindingResult result, - boolean includeMessage, boolean includeBindingErrors) { - errorAttributes.put("message", (includeMessage) ? "Validation failed for object='" + result.getObjectName() - + "'. " + "Error count: " + result.getErrorCount() : ""); - if (includeBindingErrors && result.hasErrors()) { - errorAttributes.put("errors", result.getAllErrors()); - } + private void addBindingResultErrorMessage(Map errorAttributes, BindingResult result) { + errorAttributes.put("message", "Validation failed for object='" + result.getObjectName() + "'. " + + "Error count: " + result.getErrorCount()); + errorAttributes.put("errors", result.getAllErrors()); } private BindingResult extractBindingResult(Throwable error) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java index d59b53f234..f3ae6e38c8 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java @@ -16,8 +16,11 @@ package org.springframework.boot.web.servlet.error; +import java.util.Collections; import java.util.Map; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.context.request.WebRequest; import org.springframework.web.servlet.ModelAndView; @@ -40,24 +43,25 @@ public interface ErrorAttributes { * @param includeStackTrace if stack trace element should be included * @return a map of error attributes * @deprecated since 2.3.0 in favor of - * {@link #getErrorAttributes(WebRequest, boolean, boolean, boolean)} + * {@link #getErrorAttributes(WebRequest, ErrorAttributeOptions)} */ @Deprecated - Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace); + default Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) { + return Collections.emptyMap(); + } /** * Returns a {@link Map} of the error attributes. The map can be used as the model of * an error page {@link ModelAndView}, or returned as a * {@link ResponseBody @ResponseBody}. * @param webRequest the source request - * @param includeStackTrace if stack trace element should be included - * @param includeMessage if message element should be included - * @param includeBindingErrors if errors element should be included + * @param options options for error attribute contents * @return a map of error attributes * @since 2.3.0 */ - Map getErrorAttributes(WebRequest webRequest, boolean includeStackTrace, boolean includeMessage, - boolean includeBindingErrors); + default Map getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) { + return getErrorAttributes(webRequest, options.isIncluded(Include.STACK_TRACE)); + } /** * Return the underlying cause of the error or {@code null} if the error cannot be diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java index cf3d7e4cf6..5966056319 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java @@ -24,6 +24,8 @@ import java.util.Map; import org.junit.jupiter.api.Test; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; import org.springframework.core.MethodParameter; import org.springframework.http.HttpStatus; import org.springframework.http.codec.HttpMessageReader; @@ -55,14 +57,14 @@ class DefaultErrorAttributesTests { private DefaultErrorAttributes errorAttributes = new DefaultErrorAttributes(); - private List> readers = ServerCodecConfigurer.create().getReaders(); + private final List> readers = ServerCodecConfigurer.create().getReaders(); @Test void missingExceptionAttribute() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test").build()); ServerRequest request = ServerRequest.create(exchange, this.readers); assertThatIllegalStateException() - .isThrownBy(() -> this.errorAttributes.getErrorAttributes(request, false, false, false)) + .isThrownBy(() -> this.errorAttributes.getErrorAttributes(request, ErrorAttributeOptions.defaults())) .withMessageContaining("Missing exception attribute in ServerWebExchange"); } @@ -70,7 +72,7 @@ class DefaultErrorAttributesTests { void includeTimeStamp() { MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, NOT_FOUND), - false, false, false); + ErrorAttributeOptions.defaults()); assertThat(attributes.get("timestamp")).isInstanceOf(Date.class); } @@ -79,7 +81,7 @@ class DefaultErrorAttributesTests { Error error = new OutOfMemoryError("Test error"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, error), - false, false, false); + ErrorAttributeOptions.defaults()); assertThat(attributes.get("error")).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.getReasonPhrase()); assertThat(attributes.get("status")).isEqualTo(500); } @@ -89,7 +91,7 @@ class DefaultErrorAttributesTests { Exception error = new CustomException(); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, error), - false, false, false); + ErrorAttributeOptions.defaults()); assertThat(attributes.get("error")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.getReasonPhrase()); assertThat(attributes.get("message")).isEqualTo(""); assertThat(attributes.get("status")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.value()); @@ -100,7 +102,7 @@ class DefaultErrorAttributesTests { Exception error = new CustomException("Test Message"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, error), - false, true, false); + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(attributes.get("error")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.getReasonPhrase()); assertThat(attributes.get("message")).isEqualTo("Test Message"); assertThat(attributes.get("status")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.value()); @@ -111,7 +113,7 @@ class DefaultErrorAttributesTests { Exception error = new Custom2Exception(); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, error), - false, true, false); + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(attributes.get("error")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.getReasonPhrase()); assertThat(attributes.get("status")).isEqualTo(HttpStatus.I_AM_A_TEAPOT.value()); assertThat(attributes.get("message")).isEqualTo("Nope!"); @@ -121,7 +123,7 @@ class DefaultErrorAttributesTests { void includeStatusCode() { MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, NOT_FOUND), - false, false, false); + ErrorAttributeOptions.defaults()); assertThat(attributes.get("error")).isEqualTo(HttpStatus.NOT_FOUND.getReasonPhrase()); assertThat(attributes.get("status")).isEqualTo(404); } @@ -131,7 +133,8 @@ class DefaultErrorAttributesTests { Error error = new OutOfMemoryError("Test error"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, error); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); assertThat(attributes.get("exception")).isNull(); assertThat(attributes.get("message")).isEqualTo("Test error"); @@ -142,7 +145,8 @@ class DefaultErrorAttributesTests { Error error = new OutOfMemoryError("Test error"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, error); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.defaults()); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); assertThat(attributes.get("message")).isEqualTo(""); } @@ -150,10 +154,11 @@ class DefaultErrorAttributesTests { @Test void includeException() { RuntimeException error = new RuntimeException("Test"); - this.errorAttributes = new DefaultErrorAttributes(true); + this.errorAttributes = new DefaultErrorAttributes(); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, error); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE)); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); assertThat(attributes.get("exception")).isEqualTo(RuntimeException.class.getName()); assertThat(attributes.get("message")).isEqualTo("Test"); @@ -163,10 +168,11 @@ class DefaultErrorAttributesTests { void processResponseStatusException() { RuntimeException nested = new RuntimeException("Test"); ResponseStatusException error = new ResponseStatusException(HttpStatus.BAD_REQUEST, "invalid request", nested); - this.errorAttributes = new DefaultErrorAttributes(true); + this.errorAttributes = new DefaultErrorAttributes(); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, error); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE)); assertThat(attributes.get("status")).isEqualTo(400); assertThat(attributes.get("message")).isEqualTo("invalid request"); assertThat(attributes.get("exception")).isEqualTo(RuntimeException.class.getName()); @@ -177,10 +183,11 @@ class DefaultErrorAttributesTests { void processResponseStatusExceptionWithNoNestedCause() { ResponseStatusException error = new ResponseStatusException(HttpStatus.NOT_ACCEPTABLE, "could not process request"); - this.errorAttributes = new DefaultErrorAttributes(true); + this.errorAttributes = new DefaultErrorAttributes(); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, error); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE)); assertThat(attributes.get("status")).isEqualTo(406); assertThat(attributes.get("message")).isEqualTo("could not process request"); assertThat(attributes.get("exception")).isEqualTo(ResponseStatusException.class.getName()); @@ -191,8 +198,8 @@ class DefaultErrorAttributesTests { void notIncludeTrace() { RuntimeException ex = new RuntimeException("Test"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); - Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), false, - false, false); + Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), + ErrorAttributeOptions.defaults()); assertThat(attributes.get("trace")).isNull(); } @@ -200,8 +207,8 @@ class DefaultErrorAttributesTests { void includeTrace() { RuntimeException ex = new RuntimeException("Test"); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); - Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), true, - false, false); + Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), + ErrorAttributeOptions.of(Include.STACK_TRACE)); assertThat(attributes.get("trace").toString()).startsWith("java.lang"); } @@ -209,7 +216,7 @@ class DefaultErrorAttributesTests { void includePath() { MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, NOT_FOUND), - false, false, false); + ErrorAttributeOptions.defaults()); assertThat(attributes.get("path")).isEqualTo("/test"); } @@ -217,7 +224,8 @@ class DefaultErrorAttributesTests { void includeLogPrefix() { MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); ServerRequest serverRequest = buildServerRequest(request, NOT_FOUND); - Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.get("requestId")).isEqualTo(serverRequest.exchange().getRequest().getId()); } @@ -229,8 +237,8 @@ class DefaultErrorAttributesTests { bindingResult.addError(new ObjectError("c", "d")); Exception ex = new WebExchangeBindException(stringParam, bindingResult); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); - Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), false, - true, true); + Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), + ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS)); assertThat(attributes.get("message")).asString() .startsWith("Validation failed for argument at index 0 in method: " + "int org.springframework.boot.web.reactive.error.DefaultErrorAttributesTests" @@ -246,8 +254,8 @@ class DefaultErrorAttributesTests { bindingResult.addError(new ObjectError("c", "d")); Exception ex = new WebExchangeBindException(stringParam, bindingResult); MockServerHttpRequest request = MockServerHttpRequest.get("/test").build(); - Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), false, - false, false); + Map attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex), + ErrorAttributeOptions.defaults()); assertThat(attributes.get("message")).isEqualTo(""); assertThat(attributes.containsKey("errors")).isFalse(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java index 5d91643eda..22ac9eee84 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.web.servlet.error; +import java.lang.reflect.Method; import java.util.Collections; import java.util.Date; import java.util.Map; @@ -24,8 +25,12 @@ import javax.servlet.ServletException; import org.junit.jupiter.api.Test; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.error.ErrorAttributeOptions.Include; +import org.springframework.core.MethodParameter; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.util.ReflectionUtils; import org.springframework.validation.BindException; import org.springframework.validation.BindingResult; import org.springframework.validation.MapBindingResult; @@ -54,21 +59,24 @@ class DefaultErrorAttributesTests { @Test void includeTimeStamp() { - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.get("timestamp")).isInstanceOf(Date.class); } @Test void specificStatusCode() { this.request.setAttribute("javax.servlet.error.status_code", 404); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.get("error")).isEqualTo(HttpStatus.NOT_FOUND.getReasonPhrase()); assertThat(attributes.get("status")).isEqualTo(404); } @Test void missingStatusCode() { - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.get("error")).isEqualTo("None"); assertThat(attributes.get("status")).isEqualTo(999); } @@ -78,7 +86,8 @@ class DefaultErrorAttributesTests { RuntimeException ex = new RuntimeException("Test"); ModelAndView modelAndView = this.errorAttributes.resolveException(this.request, null, null, ex); this.request.setAttribute("javax.servlet.error.exception", new RuntimeException("Ignored")); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); assertThat(modelAndView).isNull(); assertThat(attributes.containsKey("exception")).isFalse(); @@ -89,7 +98,8 @@ class DefaultErrorAttributesTests { void servletErrorWithMessage() { RuntimeException ex = new RuntimeException("Test"); this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("Test"); @@ -99,7 +109,8 @@ class DefaultErrorAttributesTests { void servletErrorWithoutMessage() { RuntimeException ex = new RuntimeException("Test"); this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message").toString()).contains(""); @@ -108,7 +119,8 @@ class DefaultErrorAttributesTests { @Test void servletMessageWithMessage() { this.request.setAttribute("javax.servlet.error.message", "Test"); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("Test"); } @@ -116,7 +128,8 @@ class DefaultErrorAttributesTests { @Test void servletMessageWithoutMessage() { this.request.setAttribute("javax.servlet.error.message", "Test"); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).asString().contains(""); } @@ -125,7 +138,8 @@ class DefaultErrorAttributesTests { void nullExceptionMessage() { this.request.setAttribute("javax.servlet.error.exception", new RuntimeException()); this.request.setAttribute("javax.servlet.error.message", "Test"); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("Test"); } @@ -133,7 +147,8 @@ class DefaultErrorAttributesTests { @Test void nullExceptionMessageAndServletMessage() { this.request.setAttribute("javax.servlet.error.exception", new RuntimeException()); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("No message available"); } @@ -143,7 +158,8 @@ class DefaultErrorAttributesTests { RuntimeException ex = new RuntimeException("Test"); ServletException wrapped = new ServletException(new ServletException(ex)); this.request.setAttribute("javax.servlet.error.exception", wrapped); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(wrapped); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("Test"); @@ -153,7 +169,8 @@ class DefaultErrorAttributesTests { void getError() { Error error = new OutOfMemoryError("Test error"); this.request.setAttribute("javax.servlet.error.exception", error); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(error); assertThat(attributes.containsKey("exception")).isFalse(); assertThat(attributes.get("message")).isEqualTo("Test error"); @@ -164,7 +181,7 @@ class DefaultErrorAttributesTests { BindingResult bindingResult = new MapBindingResult(Collections.singletonMap("a", "b"), "objectName"); bindingResult.addError(new ObjectError("c", "d")); Exception ex = new BindException(bindingResult); - testBindingResult(bindingResult, ex, true); + testBindingResult(bindingResult, ex, ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS)); } @Test @@ -172,38 +189,44 @@ class DefaultErrorAttributesTests { BindingResult bindingResult = new MapBindingResult(Collections.singletonMap("a", "b"), "objectName"); bindingResult.addError(new ObjectError("c", "d")); Exception ex = new BindException(bindingResult); - testBindingResult(bindingResult, ex, false); + testBindingResult(bindingResult, ex, ErrorAttributeOptions.defaults()); } @Test void withMethodArgumentNotValidExceptionBindingErrors() { + Method method = ReflectionUtils.findMethod(String.class, "substring", int.class); + MethodParameter parameter = new MethodParameter(method, 0); BindingResult bindingResult = new MapBindingResult(Collections.singletonMap("a", "b"), "objectName"); bindingResult.addError(new ObjectError("c", "d")); - Exception ex = new MethodArgumentNotValidException(null, bindingResult); - testBindingResult(bindingResult, ex, true); + Exception ex = new MethodArgumentNotValidException(parameter, bindingResult); + testBindingResult(bindingResult, ex, ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS)); } - private void testBindingResult(BindingResult bindingResult, Exception ex, boolean includeMessageAndErrors) { + private void testBindingResult(BindingResult bindingResult, Exception ex, ErrorAttributeOptions options) { this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, - includeMessageAndErrors, includeMessageAndErrors); - if (includeMessageAndErrors) { + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, options); + if (options.isIncluded(Include.MESSAGE)) { assertThat(attributes.get("message")) .isEqualTo("Validation failed for object='objectName'. Error count: 1"); - assertThat(attributes.get("errors")).isEqualTo(bindingResult.getAllErrors()); } else { assertThat(attributes.get("message")).isEqualTo(""); + } + if (options.isIncluded(Include.BINDING_ERRORS)) { + assertThat(attributes.get("errors")).isEqualTo(bindingResult.getAllErrors()); + } + else { assertThat(attributes.containsKey("errors")).isFalse(); } } @Test void withExceptionAttribute() { - DefaultErrorAttributes errorAttributes = new DefaultErrorAttributes(true); + DefaultErrorAttributes errorAttributes = new DefaultErrorAttributes(); RuntimeException ex = new RuntimeException("Test"); this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = errorAttributes.getErrorAttributes(this.webRequest, false, true, false); + Map attributes = errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE)); assertThat(attributes.get("exception")).isEqualTo(RuntimeException.class.getName()); assertThat(attributes.get("message")).isEqualTo("Test"); } @@ -212,7 +235,8 @@ class DefaultErrorAttributesTests { void withStackTraceAttribute() { RuntimeException ex = new RuntimeException("Test"); this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, true, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.of(Include.STACK_TRACE)); assertThat(attributes.get("trace").toString()).startsWith("java.lang"); } @@ -220,14 +244,16 @@ class DefaultErrorAttributesTests { void withoutStackTraceAttribute() { RuntimeException ex = new RuntimeException("Test"); this.request.setAttribute("javax.servlet.error.exception", ex); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.containsKey("trace")).isFalse(); } @Test void path() { this.request.setAttribute("javax.servlet.error.request_uri", "path"); - Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, false, false, false); + Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, + ErrorAttributeOptions.defaults()); assertThat(attributes.get("path")).isEqualTo("path"); } diff --git a/src/checkstyle/import-control.xml b/src/checkstyle/import-control.xml index fb8206fe27..53ba5f5e99 100644 --- a/src/checkstyle/import-control.xml +++ b/src/checkstyle/import-control.xml @@ -22,12 +22,18 @@ + + + + + + @@ -99,6 +105,7 @@ + @@ -110,6 +117,9 @@ + + +