From 32170e5847a73ba13dd57a4ca7713096a5db3d1b Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 28 Apr 2026 09:28:00 +0200 Subject: [PATCH] Avoid cache collisions in CachingResourceResolver Prior to this commit, there could be cache collisions in the `CachingResourceResolver` because the cache key generation was incomplete. This commit expands the cache key generation to avoid such cases. Fixes gh-36713 --- .../resource/CachingResourceResolver.java | 23 +++++- .../CachingResourceResolverTests.java | 73 ++++++++++++------- .../resource/CachingResourceResolver.java | 23 +++++- .../CachingResourceResolverTests.java | 71 +++++++++++------- 4 files changed, 130 insertions(+), 60 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java index b7d205bc55c..3acbbb441fb 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java @@ -107,7 +107,7 @@ public class CachingResourceResolver extends AbstractResourceResolver { protected Mono resolveResourceInternal(@Nullable ServerWebExchange exchange, String requestPath, List locations, ResourceResolverChain chain) { - String key = computeKey(exchange, requestPath); + String key = computeKey(exchange, requestPath, locations); Resource cachedResource = this.cache.get(key, Resource.class); if (cachedResource != null) { @@ -120,14 +120,31 @@ public class CachingResourceResolver extends AbstractResourceResolver { .doOnNext(resource -> this.cache.put(key, resource)); } + /** + * Compute the caching key for the given exchange and resource request path. + * @deprecated since 6.2.19 in favor of {@link #computeKey(ServerWebExchange, String, List)}. + */ + @Deprecated(since = "6.2.19", forRemoval = true) protected String computeKey(@Nullable ServerWebExchange exchange, String requestPath) { + return computeKey(exchange, requestPath, Collections.emptyList()); + } + + /** + * Compute the caching key for the given exchange and resource request path in the configured locations. + */ + protected String computeKey(@Nullable ServerWebExchange exchange, String requestPath, List locations) { + StringBuilder builder = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX); + if (!locations.isEmpty()) { + builder.append(Integer.toHexString(locations.hashCode())).append(":"); + } + builder.append(requestPath); if (exchange != null) { String codingKey = getContentCodingKey(exchange); if (StringUtils.hasText(codingKey)) { - return RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath + "+encoding=" + codingKey; + builder.append("+encoding=").append(codingKey); } } - return RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath; + return builder.toString(); } private @Nullable String getContentCodingKey(ServerWebExchange exchange) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CachingResourceResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CachingResourceResolverTests.java index bbf06665b89..7dc0a1a1059 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CachingResourceResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CachingResourceResolverTests.java @@ -16,6 +16,7 @@ package org.springframework.web.reactive.resource; +import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -39,6 +40,7 @@ import static org.springframework.web.testfixture.http.server.reactive.MockServe * Tests for {@link CachingResourceResolver}. * * @author Rossen Stoyanchev + * @author Brian Clozel */ @ExtendWith(GzipSupport.class) class CachingResourceResolverTests { @@ -48,6 +50,8 @@ class CachingResourceResolverTests { private Cache cache; + private CachingResourceResolver cachingResolver; + private ResourceResolverChain chain; private List locations; @@ -59,7 +63,9 @@ class CachingResourceResolverTests { this.cache = new ConcurrentMapCache("resourceCache"); List resolvers = new ArrayList<>(); - resolvers.add(new CachingResourceResolver(this.cache)); + this.cachingResolver = new CachingResourceResolver(this.cache); + resolvers.add(this.cachingResolver); + resolvers.add(new EncodedResourceResolver()); resolvers.add(new PathResourceResolver()); this.chain = new DefaultResourceResolverChain(resolvers); @@ -81,7 +87,7 @@ class CachingResourceResolverTests { @Test void resolveResourceInternalFromCache() { Resource expected = mock(); - this.cache.put(resourceKey("bar.css"), expected); + this.cache.put(this.cachingResolver.computeKey(null, "bar.css", this.locations), expected); MockServerWebExchange exchange = MockServerWebExchange.from(get("")); Resource actual = this.chain.resolveResource(exchange, "bar.css", this.locations).block(TIMEOUT); @@ -118,7 +124,7 @@ class CachingResourceResolverTests { } @Test - void resolveResourceAcceptEncodingInCacheKey(GzippedFiles gzippedFiles) { + void resolveResourceAcceptEncodingInCacheKey(GzippedFiles gzippedFiles) throws IOException { String file = "bar.css"; gzippedFiles.create(file); @@ -126,59 +132,70 @@ class CachingResourceResolverTests { // 1. Resolve plain resource MockServerWebExchange exchange = MockServerWebExchange.from(get(file)); - Resource expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); - - String cacheKey = resourceKey(file); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); + Resource actual = getFromResourceCache(exchange, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); // 2. Resolve with Accept-Encoding exchange = MockServerWebExchange.from(get(file) .header("Accept-Encoding", "gzip ; a=b , deflate , br ; c=d ")); - expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); + this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); - cacheKey = resourceKey(file + "+encoding=br,gzip"); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + actual = getFromResourceCache(exchange, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css.gz"); // 3. Resolve with Accept-Encoding but no matching codings exchange = MockServerWebExchange.from(get(file).header("Accept-Encoding", "deflate")); - expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); + this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); - cacheKey = resourceKey(file); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + actual = getFromResourceCache(exchange, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); } @Test - void resolveResourceNoAcceptEncoding() { + void resolveResourceNoAcceptEncoding() throws IOException { String file = "bar.css"; MockServerWebExchange exchange = MockServerWebExchange.from(get(file)); - Resource expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); + this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT); - String cacheKey = resourceKey(file); - Object actual = this.cache.get(cacheKey).get(); - - assertThat(actual).isEqualTo(expected); + Resource actual = getFromResourceCache(exchange, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); } @Test void resolveResourceMatchingEncoding() { Resource resource = mock(); Resource gzipped = mock(); - this.cache.put(resourceKey("bar.css"), resource); - this.cache.put(resourceKey("bar.css+encoding=gzip"), gzipped); - String file = "bar.css"; - MockServerWebExchange exchange = MockServerWebExchange.from(get(file)); - assertThat(this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT)).isSameAs(resource); + MockServerWebExchange exchange = MockServerWebExchange.from(get("bar.css")); + this.cache.put(this.cachingResolver.computeKey(exchange, "bar.css", this.locations), resource); - exchange = MockServerWebExchange.from(get(file).header("Accept-Encoding", "gzip")); - assertThat(this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT)).isSameAs(gzipped); + MockServerWebExchange gzipExchange = MockServerWebExchange.from(get("bar.css").header("Accept-Encoding", "gzip")); + this.cache.put(this.cachingResolver.computeKey(gzipExchange, "bar.css", this.locations), gzipped); + + assertThat(this.chain.resolveResource(exchange, "bar.css", this.locations).block(TIMEOUT)).isSameAs(resource); + assertThat(this.chain.resolveResource(gzipExchange, "bar.css", this.locations).block(TIMEOUT)).isSameAs(gzipped); } - private static String resourceKey(String key) { - return CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + key; + @Test + void shareCacheBetweenResourceLocations() { + MockServerWebExchange exchange = MockServerWebExchange.from(get("bar.css")); + + List firstLocations = List.of(new ClassPathResource("testalternatepath/", getClass())); + Resource firstResource = this.chain.resolveResource(exchange, "bar.css", firstLocations).block(TIMEOUT); + + List secondLocations = List.of(new ClassPathResource("test/", getClass())); + Resource secondResource = this.chain.resolveResource(exchange, "bar.css", secondLocations).block(TIMEOUT); + + assertThat(firstResource).isNotSameAs(secondResource); + } + + private Resource getFromResourceCache(MockServerWebExchange exchange, String file) { + String cacheKey = this.cachingResolver.computeKey(exchange, file, this.locations); + return this.cache.get(cacheKey, Resource.class); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java index f8575376d76..07648397f53 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java @@ -107,7 +107,7 @@ public class CachingResourceResolver extends AbstractResourceResolver { protected @Nullable Resource resolveResourceInternal(@Nullable HttpServletRequest request, String requestPath, List locations, ResourceResolverChain chain) { - String key = computeKey(request, requestPath); + String key = computeKey(request, requestPath, locations); Resource resource = this.cache.get(key, Resource.class); if (resource != null) { @@ -125,14 +125,31 @@ public class CachingResourceResolver extends AbstractResourceResolver { return resource; } + /** + * Compute the caching key for the given request and resource request path. + * @deprecated since 6.2.19 in favor of {@link #computeKey(HttpServletRequest, String, List)}. + */ + @Deprecated(since = "6.2.19", forRemoval = true) protected String computeKey(@Nullable HttpServletRequest request, String requestPath) { + return computeKey(request, requestPath, Collections.emptyList()); + } + + /** + * Compute the caching key for the given request and resource request path in the configured locations. + */ + protected String computeKey(@Nullable HttpServletRequest request, String requestPath, List locations) { + StringBuilder builder = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX); + if (!locations.isEmpty()) { + builder.append(Integer.toHexString(locations.hashCode())).append(":"); + } + builder.append(requestPath); if (request != null) { String codingKey = getContentCodingKey(request); if (StringUtils.hasText(codingKey)) { - return RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath + "+encoding=" + codingKey; + builder.append("+encoding=").append(codingKey); } } - return RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath; + return builder.toString(); } private @Nullable String getContentCodingKey(HttpServletRequest request) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java index 5a312d6967c..8be1e03fe58 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java @@ -16,6 +16,7 @@ package org.springframework.web.servlet.resource; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -37,12 +38,15 @@ import static org.mockito.Mockito.mock; * Tests for {@link CachingResourceResolver}. * * @author Rossen Stoyanchev + * @author Brian Clozel */ @ExtendWith(GzipSupport.class) class CachingResourceResolverTests { private Cache cache; + private CachingResourceResolver cachingResolver; + private ResourceResolverChain chain; private List locations; @@ -54,7 +58,9 @@ class CachingResourceResolverTests { this.cache = new ConcurrentMapCache("resourceCache"); List resolvers = new ArrayList<>(); - resolvers.add(new CachingResourceResolver(this.cache)); + this.cachingResolver = new CachingResourceResolver(this.cache); + resolvers.add(this.cachingResolver); + resolvers.add(new EncodedResourceResolver()); resolvers.add(new PathResourceResolver()); this.chain = new DefaultResourceResolverChain(resolvers); @@ -75,7 +81,7 @@ class CachingResourceResolverTests { @Test void resolveResourceInternalFromCache() { Resource expected = mock(); - this.cache.put(resourceKey("bar.css"), expected); + this.cache.put(this.cachingResolver.computeKey(null, "bar.css", this.locations), expected); Resource actual = this.chain.resolveResource(null, "bar.css", this.locations); assertThat(actual).isSameAs(expected); @@ -109,7 +115,7 @@ class CachingResourceResolverTests { } @Test - void resolveResourceAcceptEncodingInCacheKey(GzippedFiles gzippedFiles) { + void resolveResourceAcceptEncodingInCacheKey(GzippedFiles gzippedFiles) throws IOException { String file = "bar.css"; gzippedFiles.create(file); @@ -117,59 +123,72 @@ class CachingResourceResolverTests { // 1. Resolve plain resource MockHttpServletRequest request = new MockHttpServletRequest("GET", file); - Resource expected = this.chain.resolveResource(request, file, this.locations); + this.chain.resolveResource(request, file, this.locations); - String cacheKey = resourceKey(file); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + Resource actual = getFromResourceCache(request, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); // 2. Resolve with Accept-Encoding request = new MockHttpServletRequest("GET", file); request.addHeader("Accept-Encoding", "gzip ; a=b , deflate , br ; c=d "); - expected = this.chain.resolveResource(request, file, this.locations); + this.chain.resolveResource(request, file, this.locations); - cacheKey = resourceKey(file + "+encoding=br,gzip"); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + actual = getFromResourceCache(request, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css.gz"); // 3. Resolve with Accept-Encoding but no matching codings request = new MockHttpServletRequest("GET", file); request.addHeader("Accept-Encoding", "deflate"); - expected = this.chain.resolveResource(request, file, this.locations); + this.chain.resolveResource(request, file, this.locations); - cacheKey = resourceKey(file); - assertThat(this.cache.get(cacheKey).get()).isSameAs(expected); + actual = getFromResourceCache(request, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); } @Test - void resolveResourceNoAcceptEncoding() { + void resolveResourceNoAcceptEncoding() throws IOException { String file = "bar.css"; MockHttpServletRequest request = new MockHttpServletRequest("GET", file); - Resource expected = this.chain.resolveResource(request, file, this.locations); + this.chain.resolveResource(request, file, this.locations); - String cacheKey = resourceKey(file); - Object actual = this.cache.get(cacheKey).get(); - - assertThat(actual).isEqualTo(expected); + Resource actual = getFromResourceCache(request, file); + assertThat(actual.getFile().getName()).isEqualTo("bar.css"); } @Test void resolveResourceMatchingEncoding() { Resource resource = mock(); Resource gzipped = mock(); - this.cache.put(resourceKey("bar.css"), resource); - this.cache.put(resourceKey("bar.css+encoding=gzip"), gzipped); MockHttpServletRequest request = new MockHttpServletRequest("GET", "bar.css"); - assertThat(this.chain.resolveResource(request, "bar.css", this.locations)).isSameAs(resource); + this.cache.put(this.cachingResolver.computeKey(request, "bar.css", this.locations), resource); - request = new MockHttpServletRequest("GET", "bar.css"); - request.addHeader("Accept-Encoding", "gzip"); - assertThat(this.chain.resolveResource(request, "bar.css", this.locations)).isSameAs(gzipped); + MockHttpServletRequest gzipRequest = new MockHttpServletRequest("GET", "bar.css"); + gzipRequest.addHeader("Accept-Encoding", "gzip"); + this.cache.put(this.cachingResolver.computeKey(gzipRequest, "bar.css", this.locations), gzipped); + + assertThat(this.chain.resolveResource(request, "bar.css", this.locations)).isSameAs(resource); + assertThat(this.chain.resolveResource(gzipRequest, "bar.css", this.locations)).isSameAs(gzipped); } - private static String resourceKey(String key) { - return CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + key; + @Test + void shareCacheBetweenResourceLocations() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "bar.css"); + + List firstLocations = List.of(new ClassPathResource("testalternatepath/", getClass())); + Resource firstResource = this.chain.resolveResource(request, "bar.css", firstLocations); + + List secondLocations = List.of(new ClassPathResource("test/", getClass())); + Resource secondResource = this.chain.resolveResource(request, "bar.css", secondLocations); + + assertThat(firstResource).isNotSameAs(secondResource); + } + + private Resource getFromResourceCache(MockHttpServletRequest request, String file) { + String cacheKey = this.cachingResolver.computeKey(request, file, this.locations); + return this.cache.get(cacheKey, Resource.class); } }