Improve support for caching encoded resources

The key in CachingResourceResolver now includes the "Accept-Encoding"
request header cleaned to exclude "*", "identity", and parameters, and
also sorted alphabetically.

For encoded resources the response now includes a response header with
"Vary: Accept-Encoding".

Issue: SPR-16381

spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -16,7 +16,9 @@
 
 package org.springframework.web.reactive.resource;
 
+import java.util.Arrays;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import reactor.core.publisher.Mono;
 
@@ -25,6 +27,7 @@ import org.springframework.cache.CacheManager;
 import org.springframework.core.io.Resource;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
@@ -94,14 +97,31 @@ public class CachingResourceResolver extends AbstractResourceResolver {
 		StringBuilder key = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX);
 		key.append(requestPath);
 		if (exchange != null) {
-			String encoding = exchange.getRequest().getHeaders().getFirst("Accept-Encoding");
-			if (encoding != null && encoding.contains("gzip")) {
-				key.append("+encoding=gzip");
+			String codingKey = getContentCodingKey(exchange);
+			if (codingKey != null) {
+				key.append("+encoding=").append(codingKey);
 			}
 		}
 		return key.toString();
 	}
 
+	@Nullable
+	private static String getContentCodingKey(ServerWebExchange exchange) {
+		String header = exchange.getRequest().getHeaders().getFirst("Accept-Encoding");
+		if (!StringUtils.hasText(header)) {
+			return null;
+		}
+		return Arrays.stream(StringUtils.tokenizeToStringArray(header, ","))
+				.map(token -> {
+					int index = token.indexOf(';');
+					return (index >= 0 ? token.substring(0, index) : token).trim().toLowerCase();
+				})
+				.filter(coding -> !coding.equals("*"))
+				.filter(coding -> !coding.equals("identity"))
+				.sorted()
+				.collect(Collectors.joining(","));
+	}
+
 	@Override
 	protected Mono<String> resolveUrlPathInternal(String resourceUrlPath,
 			List<? extends Resource> locations, ResourceResolverChain chain) {

spring-webflux/src/main/java/org/springframework/web/reactive/resource/EncodedResourceResolver.java

@@ -268,6 +268,7 @@ public class EncodedResourceResolver extends AbstractResourceResolver {
 				headers = new HttpHeaders();
 			}
 			headers.add(HttpHeaders.CONTENT_ENCODING, this.coding);
+			headers.add(HttpHeaders.VARY, HttpHeaders.ACCEPT_ENCODING);
 			return headers;
 		}
 	}

spring-webflux/src/main/java/org/springframework/web/reactive/resource/GzipResourceResolver.java

@@ -166,6 +166,7 @@ public class GzipResourceResolver extends AbstractResourceResolver {
 				headers = new HttpHeaders();
 			}
 			headers.add(HttpHeaders.CONTENT_ENCODING, "gzip");
+			headers.add(HttpHeaders.VARY, HttpHeaders.ACCEPT_ENCODING);
 			return headers;
 		}
 	}

spring-webflux/src/test/java/org/springframework/web/reactive/resource/CachingResourceResolverTests.java

@@ -28,10 +28,10 @@ import org.springframework.cache.Cache;
 import org.springframework.cache.concurrent.ConcurrentMapCache;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.Resource;
-import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 
 import static org.junit.Assert.*;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
 
 /**
  * Unit tests for {@link CachingResourceResolver}.
@@ -68,7 +68,7 @@ public class CachingResourceResolverTests {
 	@Test
 	public void resolveResourceInternal() {
 		Resource expected = new ClassPathResource("test/bar.css", getClass());
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(""));
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(""));
 		Resource actual = this.chain.resolveResource(exchange, "bar.css", this.locations).block(TIMEOUT);
 
 		assertNotSame(expected, actual);
@@ -78,9 +78,9 @@ public class CachingResourceResolverTests {
 	@Test
 	public void resolveResourceInternalFromCache() {
 		Resource expected = Mockito.mock(Resource.class);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", expected);
+		this.cache.put(getCacheKey("bar.css"), expected);
 
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(""));
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(""));
 		Resource actual = this.chain.resolveResource(exchange, "bar.css", this.locations).block(TIMEOUT);
 
 		assertSame(expected, actual);
@@ -88,7 +88,7 @@ public class CachingResourceResolverTests {
 
 	@Test
 	public void resolveResourceInternalNoMatch() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(""));
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(""));
 		assertNull(this.chain.resolveResource(exchange, "invalid.css", this.locations).block(TIMEOUT));
 	}
 
@@ -117,11 +117,11 @@ public class CachingResourceResolverTests {
 	@Test
 	public void resolveResourceAcceptEncodingInCacheKey() {
 		String file = "bar.css";
-		MockServerHttpRequest request = MockServerHttpRequest.get(file).header("Accept-Encoding", "gzip").build();
-		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(file)
+				.header("Accept-Encoding", "gzip ; a=b  , deflate ,  brotli  ; c=d "));
 		Resource expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT);
 
-		String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file + "+encoding=gzip";
+		String cacheKey = getCacheKey(file + "+encoding=brotli,deflate,gzip");
 		Object actual = this.cache.get(cacheKey).get();
 
 		assertSame(expected, actual);
@@ -130,10 +130,10 @@ public class CachingResourceResolverTests {
 	@Test
 	public void resolveResourceNoAcceptEncoding() {
 		String file = "bar.css";
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(file));
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(file));
 		Resource expected = this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT);
 
-		String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file;
+		String cacheKey = getCacheKey(file);
 		Object actual = this.cache.get(cacheKey).get();
 
 		assertEquals(expected, actual);
@@ -143,15 +143,19 @@ public class CachingResourceResolverTests {
 	public void resolveResourceMatchingEncoding() {
 		Resource resource = Mockito.mock(Resource.class);
 		Resource gzipped = Mockito.mock(Resource.class);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", resource);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css+encoding=gzip", gzipped);
+		this.cache.put(getCacheKey("bar.css"), resource);
+		this.cache.put(getCacheKey("bar.css+encoding=gzip"), gzipped);
 
 		String file = "bar.css";
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(file));
+		MockServerWebExchange exchange = MockServerWebExchange.from(get(file));
 		assertSame(resource, this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT));
 
-		exchange = MockServerWebExchange.from(MockServerHttpRequest.get(file).header("Accept-Encoding", "gzip"));
+		exchange = MockServerWebExchange.from(get(file).header("Accept-Encoding", "gzip"));
 		assertSame(gzipped, this.chain.resolveResource(exchange, file, this.locations).block(TIMEOUT));
 	}
 
+	private static String getCacheKey(String key) {
+		return CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + key;
+	}
+
 }

spring-webflux/src/test/java/org/springframework/web/reactive/resource/EncodedResourceResolverTests.java

@@ -37,6 +37,7 @@ import org.springframework.cache.concurrent.ConcurrentMapCache;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
+import org.springframework.http.HttpHeaders;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.util.FileCopyUtils;
@@ -109,7 +110,11 @@ public class EncodedResourceResolverTests {
 
 		assertEquals(getResource(file + ".gz").getDescription(), actual.getDescription());
 		assertEquals(getResource(file).getFilename(), actual.getFilename());
+
 		assertTrue(actual instanceof HttpResource);
+		HttpHeaders headers = ((HttpResource) actual).getResponseHeaders();
+		assertEquals("gzip", headers.getFirst(HttpHeaders.CONTENT_ENCODING));
+		assertEquals("Accept-Encoding", headers.getFirst(HttpHeaders.VARY));
 	}
 
 	@Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -16,14 +16,18 @@
 
 package org.springframework.web.servlet.resource;
 
+import java.util.Arrays;
 import java.util.List;
+import java.util.stream.Collectors;
 import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.cache.Cache;
 import org.springframework.cache.CacheManager;
 import org.springframework.core.io.Resource;
+import org.springframework.http.HttpHeaders;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * A {@link org.springframework.web.servlet.resource.ResourceResolver} that
@@ -95,14 +99,31 @@ public class CachingResourceResolver extends AbstractResourceResolver {
 		StringBuilder key = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX);
 		key.append(requestPath);
 		if (request != null) {
-			String encoding = request.getHeader("Accept-Encoding");
-			if (encoding != null && encoding.contains("gzip")) {
-				key.append("+encoding=gzip");
+			String codingKey = getContentCodingKey(request);
+			if (codingKey != null) {
+				key.append("+encoding=").append(codingKey);
 			}
 		}
 		return key.toString();
 	}
 
+	@Nullable
+	private static String getContentCodingKey(HttpServletRequest request) {
+		String header = request.getHeader(HttpHeaders.ACCEPT_ENCODING);
+		if (!StringUtils.hasText(header)) {
+			return null;
+		}
+		return Arrays.stream(StringUtils.tokenizeToStringArray(header, ","))
+				.map(token -> {
+					int index = token.indexOf(';');
+					return (index >= 0 ? token.substring(0, index) : token).trim().toLowerCase();
+				})
+				.filter(coding -> !coding.equals("*"))
+				.filter(coding -> !coding.equals("identity"))
+				.sorted()
+				.collect(Collectors.joining(","));
+	}
+
 	@Override
 	protected String resolveUrlPathInternal(String resourceUrlPath,
 			List<? extends Resource> locations, ResourceResolverChain chain) {

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

@@ -263,6 +263,7 @@ public class EncodedResourceResolver extends AbstractResourceResolver {
 				headers = new HttpHeaders();
 			}
 			headers.add(HttpHeaders.CONTENT_ENCODING, this.coding);
+			headers.add(HttpHeaders.VARY, HttpHeaders.ACCEPT_ENCODING);
 			return headers;
 		}
 	}

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

@@ -167,6 +167,7 @@ public class GzipResourceResolver extends AbstractResourceResolver {
 				headers = new HttpHeaders();
 			}
 			headers.add(HttpHeaders.CONTENT_ENCODING, "gzip");
+			headers.add(HttpHeaders.VARY, HttpHeaders.ACCEPT_ENCODING);
 			return headers;
 		}
 	}

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

@@ -73,7 +73,7 @@ public class CachingResourceResolverTests {
 	@Test
 	public void resolveResourceInternalFromCache() {
 		Resource expected = Mockito.mock(Resource.class);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", expected);
+		this.cache.put(getCacheKey("bar.css"), expected);
 		Resource actual = this.chain.resolveResource(null, "bar.css", this.locations);
 
 		assertSame(expected, actual);
@@ -110,10 +110,10 @@ public class CachingResourceResolverTests {
 	public void resolveResourceAcceptEncodingInCacheKey() {
 		String file = "bar.css";
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
-		request.addHeader("Accept-Encoding", "gzip");
+		request.addHeader("Accept-Encoding", "gzip ; a=b  , deflate ,  brotli  ; c=d ");
 		Resource expected = this.chain.resolveResource(request, file, this.locations);
 
-		String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file + "+encoding=gzip";
+		String cacheKey = getCacheKey(file + "+encoding=brotli,deflate,gzip");
 		Object actual = this.cache.get(cacheKey).get();
 
 		assertSame(expected, actual);
@@ -125,7 +125,7 @@ public class CachingResourceResolverTests {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
 		Resource expected = this.chain.resolveResource(request, file, this.locations);
 
-		String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file;
+		String cacheKey = getCacheKey(file);
 		Object actual = this.cache.get(cacheKey).get();
 
 		assertEquals(expected, actual);
@@ -135,8 +135,8 @@ public class CachingResourceResolverTests {
 	public void resolveResourceMatchingEncoding() {
 		Resource resource = Mockito.mock(Resource.class);
 		Resource gzipped = Mockito.mock(Resource.class);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", resource);
-		this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css+encoding=gzip", gzipped);
+		this.cache.put(getCacheKey("bar.css"), resource);
+		this.cache.put(getCacheKey("bar.css+encoding=gzip"), gzipped);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "bar.css");
 		assertSame(resource, this.chain.resolveResource(request,"bar.css", this.locations));
@@ -146,4 +146,8 @@ public class CachingResourceResolverTests {
 		assertSame(gzipped, this.chain.resolveResource(request, "bar.css", this.locations));
 	}
 
+	private static String getCacheKey(String key) {
+		return CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + key;
+	}
+
 }

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

@@ -36,6 +36,7 @@ import org.springframework.cache.concurrent.ConcurrentMapCache;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
+import org.springframework.http.HttpHeaders;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.util.FileCopyUtils;
 
@@ -105,7 +106,11 @@ public class EncodedResourceResolverTests {
 
 		assertEquals(getResource(file + ".gz").getDescription(), actual.getDescription());
 		assertEquals(getResource(file).getFilename(), actual.getFilename());
+
 		assertTrue(actual instanceof HttpResource);
+		HttpHeaders headers = ((HttpResource) actual).getResponseHeaders();
+		assertEquals("gzip", headers.getFirst(HttpHeaders.CONTENT_ENCODING));
+		assertEquals("Accept-Encoding", headers.getFirst(HttpHeaders.VARY));
 	}
 
 	@Test