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

Author: wilkinsona

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/error/Error.java

@@ -21,9 +21,12 @@
 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 String getDefaultMessage() {
 	 * Return the original cause of the error.
 	 * @return the error cause
 	 */
+	@JsonIgnore
 	public MessageSourceResolvable getCause() {
 		return this.cause;
 	}
@@ -91,10 +95,13 @@ public String toString() {
 	}
 
 	/**
-	 * 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<Error> wrap(List<? extends MessageSourceResolvable> errors) {
 		if (CollectionUtils.isEmpty(errors)) {
 			return Collections.emptyList();
@@ -106,4 +113,27 @@ public static List<Error> wrap(List<? extends MessageSourceResolvable> errors) {
 		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<MessageSourceResolvable> wrapIfNecessary(List<? extends MessageSourceResolvable> errors) {
+		if (CollectionUtils.isEmpty(errors)) {
+			return Collections.emptyList();
+		}
+		List<MessageSourceResolvable> 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);
+	}
+
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java

@@ -47,8 +47,10 @@
  * <li>error - The error reason</li>
  * <li>exception - The class name of the root exception (if configured)</li>
  * <li>message - The exception message (if configured)</li>
- * <li>errors - Any validation errors wrapped in {@link Error}, derived from a
- * {@link BindingResult} or {@link MethodValidationResult} exception (if configured)</li>
+ * <li>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}</li>
  * <li>trace - The exception stack trace (if configured)</li>
  * <li>path - The URL path when the exception was raised</li>
  * <li>requestId - Unique ID associated with the current request</li>
@@ -114,18 +116,18 @@ private void handleException(Map<String, Object> errorAttributes, Throwable erro
 		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 {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java

@@ -51,8 +51,10 @@
  * <li>error - The error reason</li>
  * <li>exception - The class name of the root exception (if configured)</li>
  * <li>message - The exception message (if configured)</li>
- * <li>errors - Any validation errors wrapped in {@link Error}, derived from a
- * {@link BindingResult} or {@link MethodValidationResult} exception (if configured)</li>
+ * <li>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}</li>
  * <li>trace - The exception stack trace (if configured)</li>
  * <li>path - The URL path when the exception was raised</li>
  * </ul>
@@ -154,14 +156,14 @@ private void addErrorMessage(Map<String, Object> errorAttributes, WebRequest web
 	private void addMessageAndErrorsFromBindingResult(Map<String, Object> 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<String, Object> 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<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {

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<Error> 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<MessageSourceResolvable> 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";
+		}
+
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java

@@ -274,7 +274,7 @@ void extractBindingResultErrors() throws Exception {
 					+ "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 @@ void extractBindingResultErrorsThatCausedAResponseStatusException() throws Excep
 				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 @@ void extractMethodValidationResultErrors() throws Exception {
 			.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 @@ void extractParameterValidationResultErrors() throws Exception {
 			.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

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java

@@ -241,7 +241,8 @@ private void testErrors(List<? extends MessageSourceResolvable> errors, String e
 			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");