Skip to content

Commit 50bcd67

Browse files
committed
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
1 parent 18e9699 commit 50bcd67

File tree

4 files changed

+77
-68
lines changed

4 files changed

+77
-68
lines changed

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -747,11 +747,13 @@ protected ModelAndView handleInternal(HttpServletRequest request,
747747
mav = invokeHandlerMethod(request, response, handlerMethod);
748748
}
749749

750-
if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) {
751-
applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers);
752-
}
753-
else {
754-
prepareResponse(response);
750+
if (!response.containsHeader(HEADER_CACHE_CONTROL)) {
751+
if (getSessionAttributesHandler(handlerMethod).hasSessionAttributes()) {
752+
applyCacheSeconds(response, this.cacheSecondsForSessionAttributeHandlers);
753+
}
754+
else {
755+
prepareResponse(response);
756+
}
755757
}
756758

757759
return mav;
@@ -895,7 +897,7 @@ private WebDataBinderFactory getDataBinderFactory(HandlerMethod handlerMethod) t
895897
}
896898
List<InvocableHandlerMethod> initBinderMethods = new ArrayList<InvocableHandlerMethod>();
897899
// Global methods first
898-
for (Entry<ControllerAdviceBean, Set<Method>> entry : this.initBinderAdviceCache .entrySet()) {
900+
for (Entry<ControllerAdviceBean, Set<Method>> entry : this.initBinderAdviceCache.entrySet()) {
899901
if (entry.getKey().isApplicableToBeanType(handlerType)) {
900902
Object bean = entry.getKey().resolveBean();
901903
for (Method method : entry.getValue()) {

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

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport {
7575

7676
private static final String HEADER_EXPIRES = "Expires";
7777

78-
private static final String HEADER_CACHE_CONTROL = "Cache-Control";
78+
protected static final String HEADER_CACHE_CONTROL = "Cache-Control";
7979

8080

8181
/** Set of supported HTTP methods */
@@ -372,16 +372,14 @@ protected final void prepareResponse(HttpServletResponse response) {
372372
* @since 4.2
373373
*/
374374
protected final void applyCacheControl(HttpServletResponse response, CacheControl cacheControl) {
375-
if (!response.containsHeader(HEADER_CACHE_CONTROL)) {
376-
String ccValue = cacheControl.getHeaderValue();
377-
if (ccValue != null) {
378-
// Set computed HTTP 1.1 Cache-Control header
379-
response.setHeader(HEADER_CACHE_CONTROL, ccValue);
380-
381-
if (response.containsHeader(HEADER_PRAGMA)) {
382-
// Reset HTTP 1.0 Pragma header if present
383-
response.setHeader(HEADER_PRAGMA, "");
384-
}
375+
String ccValue = cacheControl.getHeaderValue();
376+
if (ccValue != null) {
377+
// Set computed HTTP 1.1 Cache-Control header
378+
response.setHeader(HEADER_CACHE_CONTROL, ccValue);
379+
380+
if (response.containsHeader(HEADER_PRAGMA)) {
381+
// Reset HTTP 1.0 Pragma header if present
382+
response.setHeader(HEADER_PRAGMA, "");
385383
}
386384
}
387385
}
@@ -397,32 +395,30 @@ protected final void applyCacheControl(HttpServletResponse response, CacheContro
397395
*/
398396
@SuppressWarnings("deprecation")
399397
protected final void applyCacheSeconds(HttpServletResponse response, int cacheSeconds) {
400-
if (!response.containsHeader(HEADER_CACHE_CONTROL)) {
401-
if (this.useExpiresHeader || !this.useCacheControlHeader) {
402-
// Deprecated HTTP 1.0 cache behavior, as in previous Spring versions
403-
if (cacheSeconds > 0) {
404-
cacheForSeconds(response, cacheSeconds);
405-
}
406-
else if (cacheSeconds == 0) {
407-
preventCaching(response);
398+
if (this.useExpiresHeader || !this.useCacheControlHeader) {
399+
// Deprecated HTTP 1.0 cache behavior, as in previous Spring versions
400+
if (cacheSeconds > 0) {
401+
cacheForSeconds(response, cacheSeconds);
402+
}
403+
else if (cacheSeconds == 0) {
404+
preventCaching(response);
405+
}
406+
}
407+
else {
408+
CacheControl cControl;
409+
if (cacheSeconds > 0) {
410+
cControl = CacheControl.maxAge(cacheSeconds, TimeUnit.SECONDS);
411+
if (this.alwaysMustRevalidate) {
412+
cControl = cControl.mustRevalidate();
408413
}
409414
}
415+
else if (cacheSeconds == 0) {
416+
cControl = (this.useCacheControlNoStore ? CacheControl.noStore() : CacheControl.noCache());
417+
}
410418
else {
411-
CacheControl cControl;
412-
if (cacheSeconds > 0) {
413-
cControl = CacheControl.maxAge(cacheSeconds, TimeUnit.SECONDS);
414-
if (this.alwaysMustRevalidate) {
415-
cControl = cControl.mustRevalidate();
416-
}
417-
}
418-
else if (cacheSeconds == 0) {
419-
cControl = (this.useCacheControlNoStore ? CacheControl.noStore() : CacheControl.noCache());
420-
}
421-
else {
422-
cControl = CacheControl.empty();
423-
}
424-
applyCacheControl(response, cControl);
419+
cControl = CacheControl.empty();
425420
}
421+
applyCacheControl(response, cControl);
426422
}
427423
}
428424

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

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ public void teardown() {
148148

149149
@Test
150150
public void handle() throws Exception {
151-
Class<?>[] parameterTypes = new Class<?>[] { int.class, String.class, String.class, String.class, Map.class,
151+
Class<?>[] parameterTypes = new Class<?>[] {int.class, String.class, String.class, String.class, Map.class,
152152
Date.class, Map.class, String.class, String.class, TestBean.class, Errors.class, TestBean.class,
153153
Color.class, HttpServletRequest.class, HttpServletResponse.class, TestBean.class, TestBean.class,
154-
User.class, OtherUser.class, Model.class, UriComponentsBuilder.class };
154+
User.class, OtherUser.class, Model.class, UriComponentsBuilder.class};
155155

156156
String datePattern = "yyyy.MM.dd";
157157
String formattedDate = "2011.03.16";
@@ -188,12 +188,12 @@ public void handle() throws Exception {
188188
assertEquals("headerValue", model.get("header"));
189189
assertEquals(date, model.get("dateParam"));
190190

191-
Map<?,?> map = (Map<?,?>) model.get("headerMap");
191+
Map<?, ?> map = (Map<?, ?>) model.get("headerMap");
192192
assertEquals("headerValue", map.get("header"));
193193
assertEquals("anotherHeaderValue", map.get("anotherHeader"));
194194
assertEquals("systemHeaderValue", model.get("systemHeader"));
195195

196-
map = (Map<?,?>) model.get("paramMap");
196+
map = (Map<?, ?>) model.get("paramMap");
197197
assertEquals(formattedDate, map.get("dateParam"));
198198
assertEquals("paramByConventionValue", map.get("paramByConvention"));
199199

@@ -229,7 +229,7 @@ public void handle() throws Exception {
229229

230230
@Test
231231
public void handleRequestBody() throws Exception {
232-
Class<?>[] parameterTypes = new Class<?>[] { byte[].class };
232+
Class<?>[] parameterTypes = new Class<?>[] {byte[].class};
233233

234234
request.setMethod("POST");
235235
request.addHeader("Content-Type", "text/plain; charset=utf-8");
@@ -246,7 +246,7 @@ public void handleRequestBody() throws Exception {
246246

247247
@Test
248248
public void handleAndValidateRequestBody() throws Exception {
249-
Class<?>[] parameterTypes = new Class<?>[] { TestBean.class, Errors.class };
249+
Class<?>[] parameterTypes = new Class<?>[] {TestBean.class, Errors.class};
250250

251251
request.addHeader("Content-Type", "text/plain; charset=utf-8");
252252
request.setContent("Hello Server".getBytes("UTF-8"));
@@ -262,7 +262,7 @@ public void handleAndValidateRequestBody() throws Exception {
262262

263263
@Test
264264
public void handleHttpEntity() throws Exception {
265-
Class<?>[] parameterTypes = new Class<?>[] { HttpEntity.class };
265+
Class<?>[] parameterTypes = new Class<?>[] {HttpEntity.class};
266266

267267
request.addHeader("Content-Type", "text/plain; charset=utf-8");
268268
request.setContent("Hello Server".getBytes("UTF-8"));
@@ -282,7 +282,7 @@ public void handleHttpEntity() throws Exception {
282282
// SPR-13867
283283
@Test
284284
public void handleHttpEntityWithCacheControl() throws Exception {
285-
Class<?>[] parameterTypes = new Class<?>[] { HttpEntity.class };
285+
Class<?>[] parameterTypes = new Class<?>[] {HttpEntity.class};
286286
request.addHeader("Content-Type", "text/plain; charset=utf-8");
287287
request.setContent("Hello Server".getBytes("UTF-8"));
288288

@@ -357,27 +357,27 @@ public void model(Model model) {
357357
}
358358

359359
public String handle(
360-
@CookieValue("cookie") int cookie,
361-
@PathVariable("pathvar") String pathvar,
362-
@RequestHeader("header") String header,
363-
@RequestHeader(defaultValue="#{systemProperties.systemHeader}") String systemHeader,
364-
@RequestHeader Map<String, Object> headerMap,
365-
@RequestParam("dateParam") Date dateParam,
366-
@RequestParam Map<String, Object> paramMap,
367-
String paramByConvention,
368-
@Value("#{request.contextPath}") String value,
369-
@ModelAttribute("modelAttr") @Valid TestBean modelAttr,
370-
Errors errors,
371-
TestBean modelAttrByConvention,
372-
Color customArg,
373-
HttpServletRequest request,
374-
HttpServletResponse response,
375-
@SessionAttribute TestBean sessionAttribute,
376-
@RequestAttribute TestBean requestAttribute,
377-
User user,
378-
@ModelAttribute OtherUser otherUser,
379-
Model model,
380-
UriComponentsBuilder builder) throws Exception {
360+
@CookieValue("cookie") int cookie,
361+
@PathVariable("pathvar") String pathvar,
362+
@RequestHeader("header") String header,
363+
@RequestHeader(defaultValue = "#{systemProperties.systemHeader}") String systemHeader,
364+
@RequestHeader Map<String, Object> headerMap,
365+
@RequestParam("dateParam") Date dateParam,
366+
@RequestParam Map<String, Object> paramMap,
367+
String paramByConvention,
368+
@Value("#{request.contextPath}") String value,
369+
@ModelAttribute("modelAttr") @Valid TestBean modelAttr,
370+
Errors errors,
371+
TestBean modelAttrByConvention,
372+
Color customArg,
373+
HttpServletRequest request,
374+
HttpServletResponse response,
375+
@SessionAttribute TestBean sessionAttribute,
376+
@RequestAttribute TestBean requestAttribute,
377+
User user,
378+
@ModelAttribute OtherUser otherUser,
379+
Model model,
380+
UriComponentsBuilder builder) throws Exception {
381381

382382
model.addAttribute("cookie", cookie).addAttribute("pathvar", pathvar).addAttribute("header", header)
383383
.addAttribute("systemHeader", systemHeader).addAttribute("headerMap", headerMap)

spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,17 @@ public void writeContentInputStreamThrowingNullPointerException() throws Excepti
549549
assertEquals(0, this.response.getContentLength());
550550
}
551551

552+
// SPR-14005
553+
@Test
554+
public void doOverwriteExistingCacheControlHeaders() throws Exception {
555+
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
556+
this.response.setHeader("Cache-Control", "no-store");
557+
558+
this.handler.handleRequest(this.request, this.response);
559+
560+
assertEquals("max-age=3600", this.response.getHeader("Cache-Control"));
561+
}
562+
552563

553564
private long dateHeaderAsLong(String responseHeaderName) throws Exception {
554565
return dateFormat.parse(this.response.getHeader(responseHeaderName)).getTime();

0 commit comments

Comments
 (0)