Make BasicErrorController respect problem details

Fix GH-34871

Author: quaff

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

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.autoconfigure.web.servlet.error;
 
+import java.net.URI;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -24,12 +25,14 @@
 import jakarta.servlet.http.HttpServletResponse;
 
 import org.springframework.boot.autoconfigure.web.ErrorProperties;
+import org.springframework.boot.autoconfigure.web.servlet.WebMvcProperties;
 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;
 import org.springframework.http.MediaType;
+import org.springframework.http.ProblemDetail;
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.Assert;
@@ -49,6 +52,7 @@
  * @author Michael Stummvoll
  * @author Stephane Nicoll
  * @author Scott Frederick
+ * @author Yanming Zhou
  * @since 1.0.0
  * @see ErrorAttributes
  * @see ErrorProperties
@@ -59,26 +63,33 @@ public class BasicErrorController extends AbstractErrorController {
 
 	private final ErrorProperties errorProperties;
 
+	private final WebMvcProperties webMvcProperties;
+
 	/**
 	 * Create a new {@link BasicErrorController} instance.
 	 * @param errorAttributes the error attributes
 	 * @param errorProperties configuration properties
+	 * @param webMvcProperties webMvc properties
 	 */
-	public BasicErrorController(ErrorAttributes errorAttributes, ErrorProperties errorProperties) {
-		this(errorAttributes, errorProperties, Collections.emptyList());
+	public BasicErrorController(ErrorAttributes errorAttributes, ErrorProperties errorProperties,
+			WebMvcProperties webMvcProperties) {
+		this(errorAttributes, errorProperties, webMvcProperties, Collections.emptyList());
 	}
 
 	/**
 	 * Create a new {@link BasicErrorController} instance.
 	 * @param errorAttributes the error attributes
 	 * @param errorProperties configuration properties
+	 * @param webMvcProperties webMvc properties
 	 * @param errorViewResolvers error view resolvers
 	 */
 	public BasicErrorController(ErrorAttributes errorAttributes, ErrorProperties errorProperties,
-			List<ErrorViewResolver> errorViewResolvers) {
+			WebMvcProperties webMvcProperties, List<ErrorViewResolver> errorViewResolvers) {
 		super(errorAttributes, errorViewResolvers);
 		Assert.notNull(errorProperties, "ErrorProperties must not be null");
 		this.errorProperties = errorProperties;
+		Assert.notNull(webMvcProperties, "WebMvcProperties must not be null");
+		this.webMvcProperties = webMvcProperties;
 	}
 
 	@RequestMapping(produces = MediaType.TEXT_HTML_VALUE)
@@ -92,13 +103,23 @@ public ModelAndView errorHtml(HttpServletRequest request, HttpServletResponse re
 	}
 
 	@RequestMapping
-	public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
+	public ResponseEntity<?> error(HttpServletRequest request) {
 		HttpStatus status = getStatus(request);
 		if (status == HttpStatus.NO_CONTENT) {
 			return new ResponseEntity<>(status);
 		}
 		Map<String, Object> body = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.ALL));
-		return new ResponseEntity<>(body, status);
+		if (this.webMvcProperties.getProblemdetails().isEnabled()) {
+			String detail = (body.get("message") != null) ? body.get("message").toString() : "";
+			ProblemDetail pd = ProblemDetail.forStatusAndDetail(status, detail);
+			if (body.get("path") != null) {
+				pd.setInstance(URI.create(body.get("path").toString()));
+			}
+			return ResponseEntity.status(status).body(pd);
+		}
+		else {
+			return new ResponseEntity<>(body, status);
+		}
 	}
 
 	@ExceptionHandler(HttpMediaTypeNotAcceptableException.class)

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

@@ -80,6 +80,7 @@
  * @author Stephane Nicoll
  * @author Brian Clozel
  * @author Scott Frederick
+ * @author Yanming Zhou
  * @since 1.0.0
  */
 // Load before the main WebMvcAutoConfiguration so that the error View is available
@@ -103,9 +104,9 @@ public DefaultErrorAttributes errorAttributes() {
 
 	@Bean
 	@ConditionalOnMissingBean(value = ErrorController.class, search = SearchStrategy.CURRENT)
-	public BasicErrorController basicErrorController(ErrorAttributes errorAttributes,
+	public BasicErrorController basicErrorController(ErrorAttributes errorAttributes, WebMvcProperties webMvcProperties,
 			ObjectProvider<ErrorViewResolver> errorViewResolvers) {
-		return new BasicErrorController(errorAttributes, this.serverProperties.getError(),
+		return new BasicErrorController(errorAttributes, this.serverProperties.getError(), webMvcProperties,
 				errorViewResolvers.orderedStream().toList());
 	}
 

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

@@ -0,0 +1,92 @@
+/*
+ * Copyright 2012-2023 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.autoconfigure.web.servlet.error;
+
+import java.io.IOException;
+import java.net.URI;
+
+import jakarta.servlet.http.HttpServletResponse;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
+import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
+import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration;
+import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration;
+import org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration;
+import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ProblemDetail;
+import org.springframework.http.RequestEntity;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link BasicErrorController} producing
+ * {@link org.springframework.http.ProblemDetail}
+ *
+ * @author Yanming Zhou
+ */
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+		properties = { "server.error.include-message=always", "spring.mvc.problemdetails.enabled=true" })
+class BasicErrorControllerProblemDetailTests {
+
+	@Autowired
+	private TestRestTemplate testRestTemplate;
+
+	@Test
+	void sendErrorShouldProduceProblemDetails() {
+		String path = "/conflict";
+		ResponseEntity<ProblemDetail> resp = this.testRestTemplate.exchange(
+				RequestEntity.method(HttpMethod.GET, path).header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE).build(),
+				ProblemDetail.class);
+		assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.CONFLICT);
+		assertThat(resp.getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_PROBLEM_JSON);
+		ProblemDetail expected = ProblemDetail.forStatusAndDetail(HttpStatus.CONFLICT, "Conflicts arise");
+		expected.setTitle(HttpStatus.CONFLICT.getReasonPhrase());
+		expected.setInstance(URI.create(path));
+		assertThat(resp.getBody()).isEqualTo(expected);
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@ImportAutoConfiguration({ ServletWebServerFactoryAutoConfiguration.class, DispatcherServletAutoConfiguration.class,
+			WebMvcAutoConfiguration.class, HttpMessageConvertersAutoConfiguration.class,
+			ErrorMvcAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class })
+	static class TestConfiguration {
+
+		@RestController
+		public static class TestController {
+
+			@RequestMapping("/conflict")
+			void home(HttpServletResponse response) throws IOException {
+				response.sendError(HttpServletResponse.SC_CONFLICT, "Conflicts arise");
+			}
+
+		}
+
+	}
+
+}