root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

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
This commit is contained in:
Brian Clozel 2016-01-22 16:04:03 +01:00
parent ebccfd023a
commit 8f1d06f19c
4 changed files with 82 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -714,8 +714,22 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter
protected ModelAndView handleInternal(HttpServletRequest request,
HttpServletResponse response, HandlerMethod handlerMethod) throws Exception {
ModelAndView mav = null;
checkRequest(request);
// 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) {
mav = invokeHandlerMethod(request, response, handlerMethod);
}
}
}
mav = invokeHandlerMethod(request, response, handlerMethod);
if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) {
applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers);
}
@ -723,18 +737,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter
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);
}
}
}
return invokeHandlerMethod(request, response, handlerMethod);
return mav;
}
/**

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

@ -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_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, "");
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);
}
}

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

@ -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));

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

@ -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) {