From b883aad1f1541b07971f3f2e5da7ed9a9db8baf6 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 11 May 2020 21:25:54 +0200 Subject: [PATCH] Use weak ETags in VersionResourceResolver Prior to this commit, the `VersionResourceResolver` implementations would write a strong ETag HTTP response header with the resolved version of the resource (the actual value depending on the chosen strategy). This approach doesn't work well when combined with HTTP compression. Web servers disable HTTP response copression in the presence of strong ETags since mutating the response body would break the contract. This commit changes this semantic and ensures that weak ETags are used instead. Closes gh-24898 --- .../web/reactive/resource/VersionResourceResolver.java | 4 ++-- .../web/reactive/resource/ResourceWebHandlerTests.java | 2 +- .../web/reactive/resource/VersionResourceResolverTests.java | 4 ++-- .../web/servlet/resource/VersionResourceResolver.java | 4 ++-- .../web/servlet/resource/ResourceHttpRequestHandlerTests.java | 2 +- .../web/servlet/resource/VersionResourceResolverTests.java | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java index 92ed2c4467d..ff712f7068b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -317,7 +317,7 @@ public class VersionResourceResolver extends AbstractResourceResolver { public HttpHeaders getResponseHeaders() { HttpHeaders headers = (this.original instanceof HttpResource ? ((HttpResource) this.original).getResponseHeaders() : new HttpHeaders()); - headers.setETag("\"" + this.version + "\""); + headers.setETag("W/\"" + this.version + "\""); return headers; } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index 7bdbeff9452..ac96e74ae3d 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -165,7 +165,7 @@ public class ResourceWebHandlerTests { setPathWithinHandlerMapping(exchange, "versionString/foo.css"); this.handler.handle(exchange).block(TIMEOUT); - assertThat(exchange.getResponse().getHeaders().getETag()).isEqualTo("\"versionString\""); + assertThat(exchange.getResponse().getHeaders().getETag()).isEqualTo("W/\"versionString\""); assertThat(exchange.getResponse().getHeaders().getFirst("Accept-Ranges")).isEqualTo("bytes"); assertThat(exchange.getResponse().getHeaders().get("Accept-Ranges").size()).isEqualTo(1); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/VersionResourceResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/VersionResourceResolverTests.java index 4467fa42fd0..e92a0261ed0 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/VersionResourceResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/VersionResourceResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -175,7 +175,7 @@ public class VersionResourceResolverTests { assertThat(actual.getFilename()).isEqualTo(expected.getFilename()); verify(this.versionStrategy, times(1)).getResourceVersion(expected); assertThat(actual).isInstanceOf(HttpResource.class); - assertThat(((HttpResource) actual).getResponseHeaders().getETag()).isEqualTo(("\"" + version + "\"")); + assertThat(((HttpResource) actual).getResponseHeaders().getETag()).isEqualTo(("W/\"" + version + "\"")); } @Test diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java index 1e089ca01d5..d2041e5b656 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -313,7 +313,7 @@ public class VersionResourceResolver extends AbstractResourceResolver { public HttpHeaders getResponseHeaders() { HttpHeaders headers = (this.original instanceof HttpResource ? ((HttpResource) this.original).getResponseHeaders() : new HttpHeaders()); - headers.setETag("\"" + this.version + "\""); + headers.setETag("W/\"" + this.version + "\""); return headers; } } 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 9af18827f9e..cf00f1bf424 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 @@ -152,7 +152,7 @@ public class ResourceHttpRequestHandlerTests { this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "versionString/foo.css"); this.handler.handleRequest(this.request, this.response); - assertThat(this.response.getHeader("ETag")).isEqualTo("\"versionString\""); + assertThat(this.response.getHeader("ETag")).isEqualTo("W/\"versionString\""); assertThat(this.response.getHeader("Accept-Ranges")).isEqualTo("bytes"); assertThat(this.response.getHeaders("Accept-Ranges").size()).isEqualTo(1); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/VersionResourceResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/VersionResourceResolverTests.java index 472b5b77b8e..dde2a6f8e3c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/VersionResourceResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/VersionResourceResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -153,7 +153,7 @@ public class VersionResourceResolverTests { assertThat(actual.getFilename()).isEqualTo(expected.getFilename()); verify(this.versionStrategy, times(1)).getResourceVersion(expected); assertThat(actual).isInstanceOf(HttpResource.class); - assertThat(((HttpResource)actual).getResponseHeaders().getETag()).isEqualTo("\"" + version + "\""); + assertThat(((HttpResource)actual).getResponseHeaders().getETag()).isEqualTo("W/\"" + version + "\""); } @Test