Reorder HTTP headers processing in RequestMappingHandlerAdapter

Prior to this change, the `RequestMappingHandlerAdapter` would first add
a "Cache-Control" HTTP header to the response (depending on its
`WebContentGenerator` configuration and `@SessionAttributes` on the
handler class); then, the Adapter would delegate the actual handler the
processing of the request.
This leads to issues, as the handler does not have full control to the
response and has to deal with pre-existing headers in the response. This
means that the Adapter and the handler can add incompatible
Cache-Control directives without knowing it, since one cannot see the
headers added by the other until the response is committed.

This commit switches the order of execution: first, the handler is
called (possibly adding HTTP headers), then the RMHA processes the
response and adds "Cache-Control" directives *only if there's no
Cache-Control header already defined*.

Issue: SPR-13867
cherry-picked from 8f1d06f19

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java

@@ -714,27 +714,30 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter
 	protected ModelAndView handleInternal(HttpServletRequest request,
 			HttpServletResponse response, HandlerMethod handlerMethod) throws Exception {
 
+		ModelAndView mav = null;
 		checkRequest(request);
 
-		if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) {
-			applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers);
-		}
-		else {
-			prepareResponse(response);
-		}
-
 		// Execute invokeHandlerMethod in synchronized block if required.
 		if (this.synchronizeOnSession) {
 			HttpSession session = request.getSession(false);
 			if (session != null) {
 				Object mutex = WebUtils.getSessionMutex(session);
 				synchronized (mutex) {
-					return invokeHandlerMethod(request, response, handlerMethod);
+					mav = invokeHandlerMethod(request, response, handlerMethod);
 				}
 			}
 		}
 
-		return invokeHandlerMethod(request, response, handlerMethod);
+		mav = invokeHandlerMethod(request, response, handlerMethod);
+
+		if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) {
+			applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers);
+		}
+		else {
+			prepareResponse(response);
+		}
+
+		return mav;
 	}
 
 	/**

spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -20,6 +20,7 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -329,14 +330,16 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport {
 	 * @since 4.2
 	 */
 	protected final void applyCacheControl(HttpServletResponse response, CacheControl cacheControl) {
-		String ccValue = cacheControl.getHeaderValue();
-		if (ccValue != null) {
-			// Set computed HTTP 1.1 Cache-Control header
-			response.setHeader(HEADER_CACHE_CONTROL, ccValue);
-
-			if (response.containsHeader(HEADER_PRAGMA)) {
-				// Reset HTTP 1.0 Pragma header if present
-				response.setHeader(HEADER_PRAGMA, "");
+		if (!response.containsHeader(HEADER_CACHE_CONTROL)) {
+			String ccValue = cacheControl.getHeaderValue();
+			if (ccValue != null) {
+				// Set computed HTTP 1.1 Cache-Control header
+				response.setHeader(HEADER_CACHE_CONTROL, ccValue);
+
+				if (response.containsHeader(HEADER_PRAGMA)) {
+					// Reset HTTP 1.0 Pragma header if present
+					response.setHeader(HEADER_PRAGMA, "");
+				}
 			}
 		}
 	}
@@ -352,30 +355,32 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport {
 	 */
 	@SuppressWarnings("deprecation")
 	protected final void applyCacheSeconds(HttpServletResponse response, int cacheSeconds) {
-		if (this.useExpiresHeader || !this.useCacheControlHeader) {
-			// Deprecated HTTP 1.0 cache behavior, as in previous Spring versions
-			if (cacheSeconds > 0) {
-				cacheForSeconds(response, cacheSeconds);
-			}
-			else if (cacheSeconds == 0) {
-				preventCaching(response);
-			}
-		}
-		else {
-			CacheControl cControl;
-			if (cacheSeconds > 0) {
-				cControl = CacheControl.maxAge(cacheSeconds, TimeUnit.SECONDS);
-				if (this.alwaysMustRevalidate) {
-					cControl = cControl.mustRevalidate();
+		if (!response.containsHeader(HEADER_CACHE_CONTROL)) {
+			if (this.useExpiresHeader || !this.useCacheControlHeader) {
+				// Deprecated HTTP 1.0 cache behavior, as in previous Spring versions
+				if (cacheSeconds > 0) {
+					cacheForSeconds(response, cacheSeconds);
+				}
+				else if (cacheSeconds == 0) {
+					preventCaching(response);
 				}
-			}
-			else if (cacheSeconds == 0) {
-				cControl = (this.useCacheControlNoStore ? CacheControl.noStore() : CacheControl.noCache());
 			}
 			else {
-				cControl = CacheControl.empty();
+				CacheControl cControl;
+				if (cacheSeconds > 0) {
+					cControl = CacheControl.maxAge(cacheSeconds, TimeUnit.SECONDS);
+					if (this.alwaysMustRevalidate) {
+						cControl = cControl.mustRevalidate();
+					}
+				}
+				else if (cacheSeconds == 0) {
+					cControl = (this.useCacheControlNoStore ? CacheControl.noStore() : CacheControl.noCache());
+				}
+				else {
+					cControl = CacheControl.empty();
+				}
+				applyCacheControl(response, cControl);
 			}
-			applyCacheControl(response, cControl);
 		}
 	}
 

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/ControllerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -49,7 +49,7 @@ public class ControllerTests {
 		ParameterizableViewController pvc = new ParameterizableViewController();
 		pvc.setViewName(viewName);
 		// We don't care about the params.
-		ModelAndView mv = pvc.handleRequest(new MockHttpServletRequest("GET", "foo.html"), null);
+		ModelAndView mv = pvc.handleRequest(new MockHttpServletRequest("GET", "foo.html"), new MockHttpServletResponse());
 		assertTrue("model has no data", mv.getModel().size() == 0);
 		assertTrue("model has correct viewname", mv.getViewName().equals(viewName));
 		assertTrue("getViewName matches", pvc.getViewName().equals(viewName));

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -28,11 +28,14 @@ import java.util.GregorianCalendar;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.validation.Valid;
 
+import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -40,8 +43,8 @@ import org.junit.Test;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.beans.propertyeditors.CustomDateEditor;
 import org.springframework.core.MethodParameter;
+import org.springframework.http.CacheControl;
 import org.springframework.http.HttpEntity;
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.mock.web.test.MockHttpServletRequest;
@@ -85,6 +88,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -262,6 +266,24 @@ public class RequestMappingHandlerAdapterIntegrationTests {
 		assertEquals(HttpStatus.ACCEPTED.value(), response.getStatus());
 		assertEquals("Handled requestBody=[Hello Server]", new String(response.getContentAsByteArray(), "UTF-8"));
 		assertEquals("headerValue", response.getHeader("header"));
+		// set because of @SesstionAttributes
+		assertEquals("no-store", response.getHeader("Cache-Control"));
+	}
+
+	// SPR-13867
+	@Test
+	public void handleHttpEntityWithCacheControl() throws Exception {
+		Class<?>[] parameterTypes = new Class<?>[] { HttpEntity.class };
+		request.addHeader("Content-Type", "text/plain; charset=utf-8");
+		request.setContent("Hello Server".getBytes("UTF-8"));
+
+		HandlerMethod handlerMethod = handlerMethod("handleHttpEntityWithCacheControl", parameterTypes);
+		ModelAndView mav = handlerAdapter.handle(request, response, handlerMethod);
+
+		assertNull(mav);
+		assertEquals(HttpStatus.OK.value(), response.getStatus());
+		assertEquals("Handled requestBody=[Hello Server]", new String(response.getContentAsByteArray(), "UTF-8"));
+		assertThat(response.getHeaderValues("Cache-Control"), Matchers.contains("max-age=3600"));
 	}
 
 	@Test
@@ -373,10 +395,15 @@ public class RequestMappingHandlerAdapterIntegrationTests {
 		}
 
 		public ResponseEntity<String> handleHttpEntity(HttpEntity<byte[]> httpEntity) throws Exception {
-			HttpHeaders responseHeaders = new HttpHeaders();
-			responseHeaders.set("header", "headerValue");
 			String responseBody = "Handled requestBody=[" + new String(httpEntity.getBody(), "UTF-8") + "]";
-			return new ResponseEntity<String>(responseBody, responseHeaders, HttpStatus.ACCEPTED);
+			return ResponseEntity.accepted()
+					.header("header", "headerValue")
+					.body(responseBody);
+		}
+
+		public ResponseEntity<String> handleHttpEntityWithCacheControl(HttpEntity<byte[]> httpEntity) throws Exception {
+			String responseBody = "Handled requestBody=[" + new String(httpEntity.getBody(), "UTF-8") + "]";
+			return ResponseEntity.ok().cacheControl(CacheControl.maxAge(1, TimeUnit.HOURS)).body(responseBody);
 		}
 
 		public void handleRequestPart(@RequestPart String requestPart, Model model) {