From efca113f07f5149b18c6eaf2cc183c410cfed0e0 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 4 Jul 2025 16:05:36 +0100 Subject: [PATCH] Do not wrap known-safe MessageSourceResolvables In Spring Boot 3.5, support was added for including the errors from a MethodValidationResult in the standard error response. It was requested in gh-42013 and implemented in gh-43330. With hindsight, the implementation went too far as it changed how all errors are serialized to JSON. This commit reworks the wrapping so that ObjectErrors are not wrapped, restoring Spring Boot 3.4's behavior. This is considered safe as the contents of an ObjectError are fairly limited and should serialize to JSON without problems. Other MessageSourceResolvable implementations are still wrapped as there are no limits on their contents and we cannot predict how suitable they are for JSON serialization. Closes gh-46260 --- .../springframework/boot/web/error/Error.java | 32 +++++++- .../error/DefaultErrorAttributes.java | 12 +-- .../servlet/error/DefaultErrorAttributes.java | 10 ++- .../boot/web/error/ErrorTests.java | 77 +++++++++++++++++++ .../error/DefaultErrorAttributesTests.java | 8 +- .../error/DefaultErrorAttributesTests.java | 3 +- 6 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/error/ErrorTests.java 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");