Skip to content

Commit 89694b0

Browse files
bclozelbclozel
committed
Fix CachingResourceResolver key generation
When used in combination with GzipResourceResolver, the CachingResourceResolver does not properly cache results, since it only takes the request path as a input for cache key generation. Here's an example of that behavior: 1. an HTTP client requests a resource with `Accept-Encoding: gzip`, so the GzipResourceResolver can resolve a gzipped resource. 2. the configured CachingResourceResolver caches that resource. 3. another HTTP client requests the same resource, but it does not support gzip encoding; the previously cached gzipped resource is still returned. This commit uses the presence/absence of gzip encoding support as an input in cache keys generation. Issue: SPR-12982
1 parent a8f7539 commit 89694b0

File tree

3 files changed

+91
-5
lines changed

3 files changed

+91
-5
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,6 +30,7 @@
3030
* delegates to the resolver chain and saves the result in the cache.
3131
*
3232
* @author Rossen Stoyanchev
33+
* @author Brian Clozel
3334
* @since 4.1
3435
*/
3536
public class CachingResourceResolver extends AbstractResourceResolver {
@@ -61,7 +62,7 @@ public Cache getCache() {
6162
protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
6263
List<? extends Resource> locations, ResourceResolverChain chain) {
6364

64-
String key = RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath;
65+
String key = computeKey(request, requestPath);
6566
Resource resource = this.cache.get(key, Resource.class);
6667

6768
if (resource != null) {
@@ -82,6 +83,18 @@ protected Resource resolveResourceInternal(HttpServletRequest request, String re
8283
return resource;
8384
}
8485

86+
protected String computeKey(HttpServletRequest request, String requestPath) {
87+
StringBuilder key = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX);
88+
key.append(requestPath);
89+
if(request != null) {
90+
String encoding = request.getHeader("Accept-Encoding");
91+
if(encoding != null && encoding.contains("gzip")) {
92+
key.append("+encoding=gzip");
93+
}
94+
}
95+
return key.toString();
96+
}
97+
8598
@Override
8699
protected String resolveUrlPathInternal(String resourceUrlPath,
87100
List<? extends Resource> locations, ResourceResolverChain chain) {

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@
2727
import org.springframework.cache.concurrent.ConcurrentMapCache;
2828
import org.springframework.core.io.ClassPathResource;
2929
import org.springframework.core.io.Resource;
30+
import org.springframework.mock.web.test.MockHttpServletRequest;
3031

3132
import static org.junit.Assert.*;
3233

@@ -108,4 +109,42 @@ public void resolverUrlPathNoMatch() {
108109
assertNull(this.chain.resolveUrlPath("invalid.css", this.locations));
109110
}
110111

112+
@Test
113+
public void resolveResourceAcceptEncodingInCacheKey() {
114+
String file = "bar.css";
115+
116+
MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
117+
request.addHeader("Accept-Encoding", "gzip");
118+
Resource expected = this.chain.resolveResource(request, file, this.locations);
119+
String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file + "+encoding=gzip";
120+
121+
assertEquals(expected, this.cache.get(cacheKey).get());
122+
}
123+
124+
@Test
125+
public void resolveResourceNoAcceptEncodingInCacheKey() {
126+
String file = "bar.css";
127+
128+
MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
129+
Resource expected = this.chain.resolveResource(request, file, this.locations);
130+
String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file;
131+
132+
assertEquals(expected, this.cache.get(cacheKey).get());
133+
}
134+
135+
@Test
136+
public void resolveResourceMatchingEncoding() {
137+
Resource resource = Mockito.mock(Resource.class);
138+
Resource gzResource = Mockito.mock(Resource.class);
139+
this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", resource);
140+
this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css+encoding=gzip", gzResource);
141+
142+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "bar.css");
143+
assertSame(resource, this.chain.resolveResource(request,"bar.css", this.locations));
144+
145+
request = new MockHttpServletRequest("GET", "bar.css");
146+
request.addHeader("Accept-Encoding", "gzip");
147+
assertSame(gzResource, this.chain.resolveResource(request, "bar.css", this.locations));
148+
}
149+
111150
}

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,6 +28,8 @@
2828
import org.junit.BeforeClass;
2929
import org.junit.Test;
3030

31+
import org.springframework.cache.Cache;
32+
import org.springframework.cache.concurrent.ConcurrentMapCache;
3133
import org.springframework.core.io.ClassPathResource;
3234
import org.springframework.core.io.FileSystemResource;
3335
import org.springframework.core.io.Resource;
@@ -38,6 +40,8 @@
3840

3941

4042
/**
43+
* Unit tests for
44+
* {@link org.springframework.web.servlet.resource.GzipResourceResolver}.
4145
*
4246
* @author Jeremy Grelle
4347
*/
@@ -47,6 +51,8 @@ public class GzipResourceResolverTests {
4751

4852
private List<Resource> locations;
4953

54+
private Cache cache;
55+
5056
@BeforeClass
5157
public static void createGzippedResources() throws IOException {
5258
Resource location = new ClassPathResource("test/", GzipResourceResolverTests.class);
@@ -71,12 +77,15 @@ public static void createGzippedResources() throws IOException {
7177

7278
@Before
7379
public void setUp() {
80+
this.cache = new ConcurrentMapCache("resourceCache");
81+
7482
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
7583
versionStrategyMap.put("/**", new ContentVersionStrategy());
7684
VersionResourceResolver versionResolver = new VersionResourceResolver();
7785
versionResolver.setStrategyMap(versionStrategyMap);
7886

7987
List<ResourceResolver> resolvers = new ArrayList<ResourceResolver>();
88+
resolvers.add(new CachingResourceResolver(this.cache));
8089
resolvers.add(new GzipResourceResolver());
8190
resolvers.add(versionResolver);
8291
resolvers.add(new PathResourceResolver());
@@ -96,7 +105,7 @@ public void resolveGzippedFile() throws IOException {
96105
Resource resolved = resolver.resolveResource(request, file, locations);
97106

98107
assertEquals(resource.getDescription(), resolved.getDescription());
99-
assertEquals(new ClassPathResource("test/"+file).getFilename(), resolved.getFilename());
108+
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
100109
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
101110
resolved instanceof EncodedResource);
102111
}
@@ -115,4 +124,29 @@ public void resolveFingerprintedGzippedFile() throws IOException {
115124
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
116125
resolved instanceof EncodedResource);
117126
}
127+
128+
@Test
129+
public void resolveFromCacheWithEncodingVariants() throws IOException {
130+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/js/foo.js");
131+
request.addHeader("Accept-Encoding", "gzip");
132+
String file = "js/foo.js";
133+
String gzFile = file+".gz";
134+
Resource resource = new ClassPathResource("test/"+file, getClass());
135+
Resource gzResource = new ClassPathResource("test/"+gzFile, getClass());
136+
137+
// resolved resource is now cached in CachingResourceResolver
138+
Resource resolved = resolver.resolveResource(request, file, locations);
139+
140+
assertEquals(gzResource.getDescription(), resolved.getDescription());
141+
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
142+
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
143+
resolved instanceof EncodedResource);
144+
145+
request = new MockHttpServletRequest("GET", "/js/foo.js");
146+
resolved = resolver.resolveResource(request, file, locations);
147+
assertEquals(resource.getDescription(), resolved.getDescription());
148+
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
149+
assertFalse("Expected " + resolved + " to *not* be of type " + EncodedResource.class,
150+
resolved instanceof EncodedResource);
151+
}
118152
}

0 commit comments

Comments
 (0)