diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/Error.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/Error.java index d074afb4bb6..2eeb961bf3f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/Error.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/Error.java @@ -21,9 +21,12 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import com.fasterxml.jackson.annotation.JsonIgnore; + import org.springframework.context.MessageSourceResolvable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; +import org.springframework.validation.ObjectError; /** * A wrapper class for {@link MessageSourceResolvable} errors that is safe for JSON @@ -65,6 +68,7 @@ public final class Error implements MessageSourceResolvable { * Return the original cause of the error. * @return the error cause */ + @JsonIgnore public MessageSourceResolvable getCause() { return this.cause; } @@ -91,10 +95,13 @@ public final class Error implements MessageSourceResolvable { } /** - * Wrap the given errors. + * Wrap the given errors such that they are suitable for serialization to JSON. * @param errors the errors to wrap * @return a new Error list + * @deprecated since 3.5.4 for removal in 4.0.0 in favor of + * {@link #wrapIfNecessary(List)} */ + @Deprecated(since = "3.5.4", forRemoval = true) public static List wrap(List errors) { if (CollectionUtils.isEmpty(errors)) { return Collections.emptyList(); @@ -106,4 +113,27 @@ public final class Error implements MessageSourceResolvable { return List.copyOf(result); } + /** + * Wrap the given errors, if necessary, such that they are suitable for serialization + * to JSON. {@link MessageSourceResolvable} implementations that are known to be + * suitable are not wrapped. + * @param errors the errors to wrap + * @return a new Error list + * @since 3.5.4 + */ + public static List wrapIfNecessary(List errors) { + if (CollectionUtils.isEmpty(errors)) { + return Collections.emptyList(); + } + List result = new ArrayList<>(errors.size()); + for (MessageSourceResolvable error : errors) { + result.add(requiresWrapping(error) ? new Error(error) : error); + } + return List.copyOf(result); + } + + private static boolean requiresWrapping(MessageSourceResolvable error) { + return !(error instanceof ObjectError); + } + } 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 535bb6cd121..505c5dc2fdd 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 @@ -47,8 +47,10 @@ import org.springframework.web.server.ServerWebExchange; *
  • error - The error reason
  • *
  • exception - The class name of the root exception (if configured)
  • *
  • message - The exception message (if configured)
  • - *
  • errors - Any validation errors wrapped in {@link Error}, derived from a - * {@link BindingResult} or {@link MethodValidationResult} exception (if configured)
  • + *
  • errors - Any validation errors derived from a {@link BindingResult} or + * {@link MethodValidationResult} exception (if configured). To ensure safe serialization + * to JSON, errors are {@link Error#wrapIfNecessary(java.util.List) wrapped if + * necessary}
  • *
  • trace - The exception stack trace (if configured)
  • *
  • path - The URL path when the exception was raised
  • *
  • requestId - Unique ID associated with the current request
  • @@ -114,18 +116,18 @@ public class DefaultErrorAttributes implements ErrorAttributes { if (error instanceof BindingResult bindingResult) { exception = error; errorAttributes.put("message", error.getMessage()); - errorAttributes.put("errors", Error.wrap(bindingResult.getAllErrors())); + errorAttributes.put("errors", Error.wrapIfNecessary(bindingResult.getAllErrors())); } else if (error instanceof MethodValidationResult methodValidationResult) { exception = error; errorAttributes.put("message", getErrorMessage(methodValidationResult)); - errorAttributes.put("errors", Error.wrap(methodValidationResult.getAllErrors())); + errorAttributes.put("errors", Error.wrapIfNecessary(methodValidationResult.getAllErrors())); } else if (error instanceof ResponseStatusException responseStatusException) { exception = (responseStatusException.getCause() != null) ? responseStatusException.getCause() : error; errorAttributes.put("message", responseStatusException.getReason()); if (exception instanceof BindingResult bindingResult) { - errorAttributes.put("errors", Error.wrap(bindingResult.getAllErrors())); + errorAttributes.put("errors", Error.wrapIfNecessary(bindingResult.getAllErrors())); } } else { 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 0caa59cf26d..b66902c7722 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 @@ -51,8 +51,10 @@ import org.springframework.web.servlet.ModelAndView; *
  • error - The error reason
  • *
  • exception - The class name of the root exception (if configured)
  • *
  • message - The exception message (if configured)
  • - *
  • errors - Any validation errors wrapped in {@link Error}, derived from a - * {@link BindingResult} or {@link MethodValidationResult} exception (if configured)
  • + *
  • errors - Any validation errors derived from a {@link BindingResult} or + * {@link MethodValidationResult} exception (if configured). To ensure safe serialization + * to JSON, errors are {@link Error#wrapIfNecessary(java.util.List) wrapped if + * necessary}
  • *
  • trace - The exception stack trace (if configured)
  • *
  • path - The URL path when the exception was raised
  • * @@ -154,14 +156,14 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException private void addMessageAndErrorsFromBindingResult(Map errorAttributes, BindingResult result) { errorAttributes.put("message", "Validation failed for object='%s'. Error count: %s" .formatted(result.getObjectName(), result.getAllErrors().size())); - errorAttributes.put("errors", Error.wrap(result.getAllErrors())); + errorAttributes.put("errors", Error.wrapIfNecessary(result.getAllErrors())); } private void addMessageAndErrorsFromMethodValidationResult(Map errorAttributes, MethodValidationResult result) { errorAttributes.put("message", "Validation failed for method='%s'. Error count: %s" .formatted(result.getMethod(), result.getAllErrors().size())); - errorAttributes.put("errors", Error.wrap(result.getAllErrors())); + errorAttributes.put("errors", Error.wrapIfNecessary(result.getAllErrors())); } private void addExceptionErrorMessage(Map errorAttributes, WebRequest webRequest, Throwable error) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/error/ErrorTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/error/ErrorTests.java new file mode 100644 index 00000000000..5185002ffd6 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/error/ErrorTests.java @@ -0,0 +1,77 @@ +/* + * Copyright 2012-present 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.List; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; + +import org.springframework.context.MessageSourceResolvable; +import org.springframework.context.support.DefaultMessageSourceResolvable; +import org.springframework.validation.FieldError; +import org.springframework.validation.ObjectError; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link Error}. + * + * @author Andy Wilkinson + */ +class ErrorTests { + + @Test + @SuppressWarnings({ "rawtypes", "removal" }) + @Deprecated(since = "3.5.4", forRemoval = true) + void wrapWrapsAllErrors() { + List wrapped = Error.wrap(List.of(new ObjectError("name", "message"), + new FieldError("name", "field", "message"), new CustomMessageSourceResolvable("code"))); + assertThat(wrapped).extracting((error) -> (Class) error.getClass()) + .containsExactly(Error.class, Error.class, Error.class); + } + + @Test + @SuppressWarnings("rawtypes") + void wrapIfNecessaryDoesNotWrapFieldErrorOrObjectError() { + List wrapped = Error.wrapIfNecessary(List.of(new ObjectError("name", "message"), + new FieldError("name", "field", "message"), new CustomMessageSourceResolvable("code"))); + assertThat(wrapped).extracting((error) -> (Class) error.getClass()) + .containsExactly(ObjectError.class, FieldError.class, Error.class); + } + + @Test + void errorCauseDoesNotAppearInJson() throws JsonProcessingException { + String json = new ObjectMapper() + .writeValueAsString(Error.wrapIfNecessary(List.of(new CustomMessageSourceResolvable("code")))); + assertThat(json).doesNotContain("some detail"); + } + + public static class CustomMessageSourceResolvable extends DefaultMessageSourceResolvable { + + CustomMessageSourceResolvable(String code) { + super(code); + } + + public String getDetail() { + return "some detail"; + } + + } + +} 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 52f2b7e6a97..62f07f59e66 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 @@ -274,7 +274,7 @@ class DefaultErrorAttributesTests { + "int org.springframework.boot.web.reactive.error.DefaultErrorAttributesTests" + ".method(java.lang.String), with 1 error(s)"); assertThat(attributes).containsEntry("errors", - org.springframework.boot.web.error.Error.wrap(bindingResult.getAllErrors())); + org.springframework.boot.web.error.Error.wrapIfNecessary(bindingResult.getAllErrors())); } @Test @@ -290,7 +290,7 @@ class DefaultErrorAttributesTests { ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS)); assertThat(attributes.get("message")).isEqualTo("Invalid"); assertThat(attributes).containsEntry("errors", - org.springframework.boot.web.error.Error.wrap(bindingResult.getAllErrors())); + org.springframework.boot.web.error.Error.wrapIfNecessary(bindingResult.getAllErrors())); } @Test @@ -312,7 +312,7 @@ class DefaultErrorAttributesTests { .isEqualTo( "Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1"); assertThat(attributes).containsEntry("errors", - org.springframework.boot.web.error.Error.wrap(methodValidationResult.getAllErrors())); + org.springframework.boot.web.error.Error.wrapIfNecessary(methodValidationResult.getAllErrors())); } @Test @@ -349,7 +349,7 @@ class DefaultErrorAttributesTests { .isEqualTo( "Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1"); assertThat(attributes).containsEntry("errors", - org.springframework.boot.web.error.Error.wrap(methodValidationResult.getAllErrors())); + org.springframework.boot.web.error.Error.wrapIfNecessary(methodValidationResult.getAllErrors())); } @Test 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 f25b2473a12..9d69f2a3081 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 @@ -241,7 +241,8 @@ class DefaultErrorAttributesTests { assertThat(attributes).doesNotContainKey("message"); } if (options.isIncluded(Include.BINDING_ERRORS)) { - assertThat(attributes).containsEntry("errors", org.springframework.boot.web.error.Error.wrap(errors)); + assertThat(attributes).containsEntry("errors", + org.springframework.boot.web.error.Error.wrapIfNecessary(errors)); } else { assertThat(attributes).doesNotContainKey("errors");