Prevent response updates if @ResponseStatus has reason

rstoyanchev · rstoyanchev · commit d52fc3bd2fb9 · 2012-05-11T17:41:14.000-04:00
When @ResponseStatus has a reason and servletResponse.sendError() is
called, the response is committed and should no longer be written to.
After this change, the ServletInvocableHandlerMethod will mark the
response fully handled and will ignore any non-null return values.

Issue: SPR-9159
diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -46,7 +46,8 @@
 
 	/**
 	 * The reason to be used for the response. <p>If this element is not set, it will default to the standard status
-	 * message for the status code.
+	 * message for the status code. Note that due to the use of {@code HttpServletResponse.sendError(int, String)},
+	 * the response will be considered complete and should not be written to any further.
 	 *
 	 * @see javax.servlet.http.HttpServletResponse#sendError(int, String)
 	 */
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java
@@ -104,6 +104,10 @@ public final void invokeAndHandle(ServletWebRequest webRequest,
 				return;
 			}
 		}
+		else if (StringUtils.hasText(this.responseReason)) {
+			mavContainer.setRequestHandled(true);
+			return;
+		}
 
 		mavContainer.setRequestHandled(false);
 
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java
@@ -72,19 +72,17 @@ public void setUp() throws Exception {
 	}
 
 	@Test
-	public void nullReturnValueResponseStatus() throws Exception {
+	public void invokeAndHandle_VoidWithResponseStatus() throws Exception {
 		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatus");
 		handlerMethod.invokeAndHandle(webRequest, mavContainer);
 
 		assertTrue("Null return value + @ResponseStatus should result in 'request handled'",
 				mavContainer.isRequestHandled());
-
 		assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus());
-		assertEquals("400 Bad Request", response.getErrorMessage());
 	}
 
 	@Test
-	public void nullReturnValueHttpServletResponseArg() throws Exception {
+	public void invokeAndHandle_VoidWithHttpServletResponseArgument() throws Exception {
 		argumentResolvers.addResolver(new ServletResponseMethodArgumentResolver());
 
 		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("httpServletResponse", HttpServletResponse.class);
@@ -95,7 +93,7 @@ public void nullReturnValueHttpServletResponseArg() throws Exception {
 	}
 
 	@Test
-	public void nullReturnValueRequestNotModified() throws Exception {
+	public void invokeAndHandle_VoidRequestNotModified() throws Exception {
 		webRequest.getNativeRequest(MockHttpServletRequest.class).addHeader("If-Modified-Since", 10 * 1000 * 1000);
 		int lastModifiedTimestamp = 1000 * 1000;
 		webRequest.checkNotModified(lastModifiedTimestamp);
@@ -107,8 +105,20 @@ public void nullReturnValueRequestNotModified() throws Exception {
 				mavContainer.isRequestHandled());
 	}
 
+	// SPR-9159
+
+	@Test
+	public void invokeAndHandle_NotVoidWithResponseStatusAndReason() throws Exception {
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatusWithReason");
+		handlerMethod.invokeAndHandle(webRequest, mavContainer);
+
+		assertTrue("When a phrase is used, the response should not be used any more", mavContainer.isRequestHandled());
+		assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus());
+		assertEquals("400 Bad Request", response.getErrorMessage());
+	}
+
 	@Test(expected=HttpMessageNotWritableException.class)
-	public void exceptionWhileHandlingReturnValue() throws Exception {
+	public void invokeAndHandle_Exception() throws Exception {
 		returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler());
 
 		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("handle");
@@ -117,7 +127,7 @@ public void exceptionWhileHandlingReturnValue() throws Exception {
 	}
 
 	@Test
-	public void dynamicReturnValue() throws Exception {
+	public void invokeAndHandle_DynamicReturnValue() throws Exception {
 		argumentResolvers.addResolver(new RequestParamMethodArgumentResolver(null, false));
 		returnValueHandlers.addHandler(new ViewMethodReturnValueHandler());
 		returnValueHandlers.addHandler(new ViewNameMethodReturnValueHandler());
@@ -153,10 +163,15 @@ public String handle() {
 			return "view";
 		}
 
-		@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request")
+		@ResponseStatus(value = HttpStatus.BAD_REQUEST)
 		public void responseStatus() {
 		}
 
+		@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request")
+		public String responseStatusWithReason() {
+			return "foo";
+		}
+
 		public void httpServletResponse(HttpServletResponse response) {
 		}
 
diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt
@@ -14,12 +14,13 @@ Changes in version 3.2 M1
 * fix case-sensitivity issue with some containers on access to 'Content-Disposition' header
 * add Servlet 3.0 based async support
 * fix issue with encoded params in UriComponentsBuilder
-* add Jackson 2 HttpMessageConverter and View types
+* add HttpMessageConverter and View types compatible with Jackson 2.0
 * add pretty print option to Jackson HttpMessageConverter and View types
 * fix issue with resolving Errors controller method argument
 * detect controller methods via InitializingBean in RequestMappingHandlerMapping
 * translate IOException from Jackson to HttpMessageNotReadableException
 * fix content negotiation issue when sorting selected media types by quality value
+* Prevent further writing to the response when @ResponseStatus contains a reason
 
 
 Changes in version 3.1.1 (2012-02-16)

Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,10 @@ public final void invokeAndHandle(ServletWebRequest webRequest,`
`104`	`104`	`return;`
`105`	`105`	`}`
`106`	`106`	`}`
	`107`	`+ else if (StringUtils.hasText(this.responseReason)) {`
	`108`	`+ mavContainer.setRequestHandled(true);`
	`109`	`+ return;`
	`110`	`+ }`
`107`	`111`
`108`	`112`	`mavContainer.setRequestHandled(false);`
`109`	`113`