Align BasicErrorController’s HTML response status with non-HTML status

Previously, BasicErrorController would return the response status
set in the javax.servlet.error.status_code request attribute when
serving JSON but would also return a 200 OK response when serving
HTML. This didn’t cause much trouble when a person was browsing, but
proved problematic for machine clients that request text/html and care
about the response status. For example, the success handler would be
driven for an XHR request even though the response was really an error.

This commit updates BasicErrorController to set the response status for
text/html responses to match the status that it would use in an
application/json response.

Closes spring-projectsgh-4694

Author: wilkinsona

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

@@ -19,6 +19,7 @@
 import java.util.Map;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory;
@@ -61,7 +62,9 @@ public String getErrorPath() {
 	}
 
 	@RequestMapping(value = "${error.path:/error}", produces = "text/html")
-	public ModelAndView errorHtml(HttpServletRequest request) {
+	public ModelAndView errorHtml(HttpServletRequest request,
+			HttpServletResponse response) {
+		response.setStatus(getStatus(request).value());
 		return new ModelAndView("error", getErrorAttributes(request, false));
 	}
 

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerDirectMockMvcTests.java

@@ -85,7 +85,7 @@ public void errorPageAvailableWithParentContext() throws Exception {
 						.run("--server.port=0"));
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("status=999"));
 	}
@@ -96,7 +96,7 @@ public void errorPageAvailableWithMvcIncluded() throws Exception {
 				WebMvcIncludedConfiguration.class).run("--server.port=0"));
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("status=999"));
 	}
@@ -116,7 +116,7 @@ public void errorControllerWithAop() throws Exception {
 				WithAopConfiguration.class).run("--server.port=0"));
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("status=999"));
 	}

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerMockMvcTests.java

@@ -121,7 +121,7 @@ public void testBindingExceptionForMachineClient() throws Exception {
 	public void testDirectAccessForBrowserClient() throws Exception {
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("ERROR_BEAN"));
 	}

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DefaultErrorViewIntegrationTests.java

@@ -69,7 +69,7 @@ public void setup() {
 	public void testErrorForBrowserClient() throws Exception {
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("<html>"));
 		assertTrue("Wrong content: " + content, content.contains("999"));
@@ -83,7 +83,7 @@ public void testErrorWithEscape() throws Exception {
 								new RuntimeException(
 										"<script>alert('Hello World')</script>"))
 						.accept(MediaType.TEXT_HTML))
-				.andExpect(status().isOk()).andReturn();
+				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
 		assertTrue("Wrong content: " + content, content.contains("&lt;script&gt;"));
 		assertTrue("Wrong content: " + content, content.contains("Hello World"));

spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/sample/actuator/ui/SampleActuatorUiApplicationTests.java

@@ -89,7 +89,7 @@ public void testError() throws Exception {
 		ResponseEntity<String> entity = new TestRestTemplate().exchange(
 				"http://localhost:" + this.port + "/error", HttpMethod.GET,
 				new HttpEntity<Void>(headers), String.class);
-		assertEquals(HttpStatus.OK, entity.getStatusCode());
+		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode());
 		assertTrue("Wrong body:\n" + entity.getBody(),
 				entity.getBody().contains("<html>"));
 		assertTrue("Wrong body:\n" + entity.getBody(),