From ce541bebcf61ae324f7c022f4f9ca0f07164341b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 10 Dec 2015 13:40:16 +0000 Subject: [PATCH] =?UTF-8?q?Align=20BasicErrorController=E2=80=99s=20HTML?= =?UTF-8?q?=20response=20status=20with=20non-HTML=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 gh-4694 --- .../boot/autoconfigure/web/BasicErrorController.java | 5 ++++- .../web/BasicErrorControllerDirectMockMvcTests.java | 6 +++--- .../autoconfigure/web/BasicErrorControllerMockMvcTests.java | 2 +- .../autoconfigure/web/DefaultErrorViewIntegrationTests.java | 4 ++-- .../actuator/ui/SampleActuatorUiApplicationTests.java | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java index e6fb0913983..fd2267fb718 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/BasicErrorController.java @@ -19,6 +19,7 @@ package org.springframework.boot.autoconfigure.web; 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 class BasicErrorController implements ErrorController { } @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)); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerDirectMockMvcTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerDirectMockMvcTests.java index a4fd5323ea7..cb0782b940c 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerDirectMockMvcTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerDirectMockMvcTests.java @@ -85,7 +85,7 @@ public class BasicErrorControllerDirectMockMvcTests { .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 class BasicErrorControllerDirectMockMvcTests { 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 class BasicErrorControllerDirectMockMvcTests { 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")); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerMockMvcTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerMockMvcTests.java index e37e602d615..0d3f385b9b6 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerMockMvcTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/BasicErrorControllerMockMvcTests.java @@ -121,7 +121,7 @@ public class BasicErrorControllerMockMvcTests { 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")); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DefaultErrorViewIntegrationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DefaultErrorViewIntegrationTests.java index 13b822202b2..980978f882a 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DefaultErrorViewIntegrationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DefaultErrorViewIntegrationTests.java @@ -69,7 +69,7 @@ public class DefaultErrorViewIntegrationTests { 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("")); assertTrue("Wrong content: " + content, content.contains("999")); @@ -83,7 +83,7 @@ public class DefaultErrorViewIntegrationTests { new RuntimeException( "")) .accept(MediaType.TEXT_HTML)) - .andExpect(status().isOk()).andReturn(); + .andExpect(status().is5xxServerError()).andReturn(); String content = response.getResponse().getContentAsString(); assertTrue("Wrong content: " + content, content.contains("<script>")); assertTrue("Wrong content: " + content, content.contains("Hello World")); diff --git a/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/sample/actuator/ui/SampleActuatorUiApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/sample/actuator/ui/SampleActuatorUiApplicationTests.java index 46de5efe171..0188d132f3f 100644 --- a/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/sample/actuator/ui/SampleActuatorUiApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/sample/actuator/ui/SampleActuatorUiApplicationTests.java @@ -89,7 +89,7 @@ public class SampleActuatorUiApplicationTests { ResponseEntity entity = new TestRestTemplate().exchange( "http://localhost:" + this.port + "/error", HttpMethod.GET, new HttpEntity(headers), String.class); - assertEquals(HttpStatus.OK, entity.getStatusCode()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode()); assertTrue("Wrong body:\n" + entity.getBody(), entity.getBody().contains("")); assertTrue("Wrong body:\n" + entity.getBody(),