From 50bcd67fb639dfb10443c316c1377550f74f8cba Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 2 Mar 2016 16:53:24 +0100 Subject: [PATCH] Relax SPR-13867 changes for ResourceHttpRequestHandler Prior to this change, SPR-13867 made sure that any class extending WebContentGenerator would not overwrite existing HTTP "Cache-Control" response headers - set by a filter, a Controller handler, etc. This caused issues with resource handling, since specifying a cache configuration there would not overwrite default headers set by filters, for example by Spring Security. This commit restricts the previous changes to the RequestMappingHandlerAdapter, in order to avoid overwriting header set by a filter or a Controller handler in those cases. Issue: SPR-14005 --- .../RequestMappingHandlerAdapter.java | 14 +++-- .../servlet/support/WebContentGenerator.java | 60 +++++++++---------- ...MappingHandlerAdapterIntegrationTests.java | 58 +++++++++--------- .../ResourceHttpRequestHandlerTests.java | 11 ++++ 4 files changed, 76 insertions(+), 67 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 259bcc89e3..298be712d3 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -747,11 +747,13 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter mav = invokeHandlerMethod(request, response, handlerMethod); } - if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) { - applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers); - } - else { - prepareResponse(response); + if (!response.containsHeader(HEADER_CACHE_CONTROL)) { + if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) { + applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers); + } + else { + prepareResponse(response); + } } return mav; @@ -895,7 +897,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter } List initBinderMethods = new ArrayList(); // Global methods first - for (Entry> entry : this.initBinderAdviceCache .entrySet()) { + for (Entry> entry : this.initBinderAdviceCache.entrySet()) { if (entry.getKey().isApplicableToBeanType(handlerType)) { Object bean = entry.getKey().resolveBean(); for (Method method : entry.getValue()) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java index 3f578053c1..130467b6e4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java @@ -75,7 +75,7 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { private static final String HEADER_EXPIRES = "Expires"; - private static final String HEADER_CACHE_CONTROL = "Cache-Control"; + protected static final String HEADER_CACHE_CONTROL = "Cache-Control"; /** Set of supported HTTP methods */ @@ -372,16 +372,14 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { * @since 4.2 */ protected final void applyCacheControl(HttpServletResponse response, CacheControl cacheControl) { - 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); + 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, ""); } } } @@ -397,32 +395,30 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { */ @SuppressWarnings("deprecation") protected final void applyCacheSeconds(HttpServletResponse response, int cacheSeconds) { - 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); + 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(); } } + else if (cacheSeconds == 0) { + cControl = (this.useCacheControlNoStore ? CacheControl.noStore() : CacheControl.noCache()); + } else { - 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); + cControl = CacheControl.empty(); } + applyCacheControl(response, cControl); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java index b9f2e8489e..7b67044dd2 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java @@ -148,10 +148,10 @@ public class RequestMappingHandlerAdapterIntegrationTests { @Test public void handle() throws Exception { - Class[] parameterTypes = new Class[] { int.class, String.class, String.class, String.class, Map.class, + Class[] parameterTypes = new Class[] {int.class, String.class, String.class, String.class, Map.class, Date.class, Map.class, String.class, String.class, TestBean.class, Errors.class, TestBean.class, Color.class, HttpServletRequest.class, HttpServletResponse.class, TestBean.class, TestBean.class, - User.class, OtherUser.class, Model.class, UriComponentsBuilder.class }; + User.class, OtherUser.class, Model.class, UriComponentsBuilder.class}; String datePattern = "yyyy.MM.dd"; String formattedDate = "2011.03.16"; @@ -188,12 +188,12 @@ public class RequestMappingHandlerAdapterIntegrationTests { assertEquals("headerValue", model.get("header")); assertEquals(date, model.get("dateParam")); - Map map = (Map) model.get("headerMap"); + Map map = (Map) model.get("headerMap"); assertEquals("headerValue", map.get("header")); assertEquals("anotherHeaderValue", map.get("anotherHeader")); assertEquals("systemHeaderValue", model.get("systemHeader")); - map = (Map) model.get("paramMap"); + map = (Map) model.get("paramMap"); assertEquals(formattedDate, map.get("dateParam")); assertEquals("paramByConventionValue", map.get("paramByConvention")); @@ -229,7 +229,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { @Test public void handleRequestBody() throws Exception { - Class[] parameterTypes = new Class[] { byte[].class }; + Class[] parameterTypes = new Class[] {byte[].class}; request.setMethod("POST"); request.addHeader("Content-Type", "text/plain; charset=utf-8"); @@ -246,7 +246,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { @Test public void handleAndValidateRequestBody() throws Exception { - Class[] parameterTypes = new Class[] { TestBean.class, Errors.class }; + Class[] parameterTypes = new Class[] {TestBean.class, Errors.class}; request.addHeader("Content-Type", "text/plain; charset=utf-8"); request.setContent("Hello Server".getBytes("UTF-8")); @@ -262,7 +262,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { @Test public void handleHttpEntity() throws Exception { - Class[] parameterTypes = new Class[] { HttpEntity.class }; + Class[] parameterTypes = new Class[] {HttpEntity.class}; request.addHeader("Content-Type", "text/plain; charset=utf-8"); request.setContent("Hello Server".getBytes("UTF-8")); @@ -282,7 +282,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { // SPR-13867 @Test public void handleHttpEntityWithCacheControl() throws Exception { - Class[] parameterTypes = new Class[] { HttpEntity.class }; + Class[] parameterTypes = new Class[] {HttpEntity.class}; request.addHeader("Content-Type", "text/plain; charset=utf-8"); request.setContent("Hello Server".getBytes("UTF-8")); @@ -357,27 +357,27 @@ public class RequestMappingHandlerAdapterIntegrationTests { } public String handle( - @CookieValue("cookie") int cookie, - @PathVariable("pathvar") String pathvar, - @RequestHeader("header") String header, - @RequestHeader(defaultValue="#{systemProperties.systemHeader}") String systemHeader, - @RequestHeader Map headerMap, - @RequestParam("dateParam") Date dateParam, - @RequestParam Map paramMap, - String paramByConvention, - @Value("#{request.contextPath}") String value, - @ModelAttribute("modelAttr") @Valid TestBean modelAttr, - Errors errors, - TestBean modelAttrByConvention, - Color customArg, - HttpServletRequest request, - HttpServletResponse response, - @SessionAttribute TestBean sessionAttribute, - @RequestAttribute TestBean requestAttribute, - User user, - @ModelAttribute OtherUser otherUser, - Model model, - UriComponentsBuilder builder) throws Exception { + @CookieValue("cookie") int cookie, + @PathVariable("pathvar") String pathvar, + @RequestHeader("header") String header, + @RequestHeader(defaultValue = "#{systemProperties.systemHeader}") String systemHeader, + @RequestHeader Map headerMap, + @RequestParam("dateParam") Date dateParam, + @RequestParam Map paramMap, + String paramByConvention, + @Value("#{request.contextPath}") String value, + @ModelAttribute("modelAttr") @Valid TestBean modelAttr, + Errors errors, + TestBean modelAttrByConvention, + Color customArg, + HttpServletRequest request, + HttpServletResponse response, + @SessionAttribute TestBean sessionAttribute, + @RequestAttribute TestBean requestAttribute, + User user, + @ModelAttribute OtherUser otherUser, + Model model, + UriComponentsBuilder builder) throws Exception { model.addAttribute("cookie", cookie).addAttribute("pathvar", pathvar).addAttribute("header", header) .addAttribute("systemHeader", systemHeader).addAttribute("headerMap", headerMap) diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index d826972fe0..276966fe4e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -549,6 +549,17 @@ public class ResourceHttpRequestHandlerTests { assertEquals(0, this.response.getContentLength()); } + // SPR-14005 + @Test + public void doOverwriteExistingCacheControlHeaders() throws Exception { + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); + this.response.setHeader("Cache-Control", "no-store"); + + this.handler.handleRequest(this.request, this.response); + + assertEquals("max-age=3600", this.response.getHeader("Cache-Control")); + } + private long dateHeaderAsLong(String responseHeaderName) throws Exception { return dateFormat.parse(this.response.getHeader(responseHeaderName)).getTime();