From 1233357509a41c71bf1112c2a9f7b8f87cf2b062 Mon Sep 17 00:00:00 2001 From: Abel Salgado Romero Date: Fri, 2 Aug 2024 15:28:34 +0200 Subject: [PATCH 1/2] Remove PRAGMA HTTP header from cache This header is deprecated and is unnecessary when using 'Cache-Control'. Currently SCG always returns 'Cache-Control' already. --- .../factory/cache/ResponseCacheManager.java | 2 + ...emoveHeadersAfterCacheExchangeMutator.java | 51 ++++++++ ...esponseCacheGatewayFilterFactoryTests.java | 19 +++ .../LocalResponseCacheGlobalFilterTests.java | 20 ++++ ...veHeaderAfterCacheExchangeMutatorTest.java | 110 ++++++++++++++++++ 5 files changed, 202 insertions(+) create mode 100644 spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeadersAfterCacheExchangeMutator.java create mode 100644 spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeaderAfterCacheExchangeMutatorTest.java diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java index 7bbc4b12..e6f115f1 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/ResponseCacheManager.java @@ -34,6 +34,7 @@ import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCache import org.springframework.cloud.gateway.filter.factory.cache.LocalResponseCacheProperties.RequestOptions; import org.springframework.cloud.gateway.filter.factory.cache.keygenerator.CacheKeyGenerator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.AfterCacheExchangeMutator; +import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.RemoveHeadersAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetMaxAgeHeaderAfterCacheExchangeMutator; import org.springframework.cloud.gateway.filter.factory.cache.postprocessor.SetResponseHeadersAfterCacheExchangeMutator; @@ -80,6 +81,7 @@ public class ResponseCacheManager { this.ignoreNoCacheUpdate = isSkipNoCacheUpdateActive(requestOptions); this.afterCacheExchangeMutators = List.of(new SetResponseHeadersAfterCacheExchangeMutator(), new SetStatusCodeAfterCacheExchangeMutator(), + new RemoveHeadersAfterCacheExchangeMutator(HttpHeaders.PRAGMA), new SetMaxAgeHeaderAfterCacheExchangeMutator(configuredTimeToLive, Clock.systemDefaultZone(), ignoreNoCacheUpdate), new SetCacheDirectivesByMaxAgeAfterCacheExchangeMutator()); diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeadersAfterCacheExchangeMutator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeadersAfterCacheExchangeMutator.java new file mode 100644 index 00000000..13d9e818 --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeadersAfterCacheExchangeMutator.java @@ -0,0 +1,51 @@ +/* + * Copyright 2013-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.filter.factory.cache.postprocessor; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; +import org.springframework.web.server.ServerWebExchange; + +/** + * Removes one or more HTTP headers from response. Assumes it is run after + * {@link SetResponseHeadersAfterCacheExchangeMutator}. + * + * @author Abel Salgado Romero + */ +public class RemoveHeadersAfterCacheExchangeMutator implements AfterCacheExchangeMutator { + + private static final Log LOGGER = LogFactory.getLog(RemoveHeadersAfterCacheExchangeMutator.class); + + private final String[] httpHeader; + + public RemoveHeadersAfterCacheExchangeMutator(String... httpHeaders) { + this.httpHeader = httpHeaders; + } + + @Override + public void accept(ServerWebExchange exchange, CachedResponse cachedResponse) { + for (String header : httpHeader) { + var previousValue = exchange.getResponse().getHeaders().remove(header); + if (previousValue != null) { + LOGGER.debug("HTTP Header value found in response, removing HTTP header " + header); + } + } + } + +} diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java index 1803ae0b..b14be476 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGatewayFilterFactoryTests.java @@ -351,6 +351,25 @@ public class LocalResponseCacheGatewayFilterFactoryTests extends BaseWebClientTe .jsonPath("$.headers." + CUSTOM_HEADER, "2"); } + @Test + void shouldNotReturnPragmaHeaderInNonCachedAndCachedResponses() { + String uri = "/" + UUID.randomUUID() + "/cache/headers"; + + testClient.get() + .uri(uri) + .header("Host", "www.localresponsecache.org") + .exchange() + .expectHeader() + .doesNotExist(HttpHeaders.PRAGMA); + + testClient.get() + .uri(uri) + .header("Host", "www.localresponsecache.org") + .exchange() + .expectHeader() + .doesNotExist(HttpHeaders.PRAGMA); + } + void assertNonVaryHeaderInContent(String uri, String varyHeader, String varyHeaderValue, String nonVaryHeader, String nonVaryHeaderValue, String expectedNonVaryResponse) { testClient.get() diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGlobalFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGlobalFilterTests.java index 54ebd154..28d63d0b 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGlobalFilterTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/LocalResponseCacheGlobalFilterTests.java @@ -30,6 +30,7 @@ import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder; import org.springframework.cloud.gateway.test.BaseWebClientTests; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; +import org.springframework.http.HttpHeaders; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ActiveProfiles; @@ -123,6 +124,25 @@ public class LocalResponseCacheGlobalFilterTests { .isEqualTo("1"); } + @Test + void shouldNotReturnPragmaHeaderInNonCachedAndCachedResponses() { + String uri = "/" + UUID.randomUUID() + "/global-cache/headers"; + + testClient.get() + .uri(uri) + .header("Host", "www.localresponsecache.org") + .exchange() + .expectHeader() + .doesNotExist(HttpHeaders.PRAGMA); + + testClient.get() + .uri(uri) + .header("Host", "www.localresponsecache.org") + .exchange() + .expectHeader() + .doesNotExist(HttpHeaders.PRAGMA); + } + @EnableAutoConfiguration @SpringBootConfiguration @Import(DefaultTestConfig.class) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeaderAfterCacheExchangeMutatorTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeaderAfterCacheExchangeMutatorTest.java new file mode 100644 index 00000000..5217a779 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/cache/postprocessor/RemoveHeaderAfterCacheExchangeMutatorTest.java @@ -0,0 +1,110 @@ +/* + * Copyright 2013-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.filter.factory.cache.postprocessor; + +import java.time.Duration; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.gateway.filter.factory.cache.CachedResponse; +import org.springframework.http.CacheControl; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.http.server.reactive.MockServerHttpResponse; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.http.HttpHeaders.CACHE_CONTROL; +import static org.springframework.http.HttpHeaders.CONTENT_TYPE; +import static org.springframework.http.HttpHeaders.EXPIRES; +import static org.springframework.http.HttpHeaders.PRAGMA; + +/** + * @author Abel Salgado Romero + */ +class RemoveHeaderAfterCacheExchangeMutatorTest { + + private static final String HTTP_HEADER_TO_REMOVE = "X-To-Remove"; + + @Test + void onlyHeaderToRemoveFromResponseIsRemoved() { + final ServerWebExchange inputExchange = setupExchange(Map.of(HTTP_HEADER_TO_REMOVE, "A-Value")); + + final var mutator = new RemoveHeadersAfterCacheExchangeMutator(HTTP_HEADER_TO_REMOVE); + CachedResponse cachedResponse = new CachedResponse.Builder(HttpStatus.OK).build(); + + mutator.accept(inputExchange, cachedResponse); + + assertThat(inputExchange.getResponse().getHeaders()).doesNotContainKey(HTTP_HEADER_TO_REMOVE) + .containsEntry(CACHE_CONTROL, List.of("max-age=60")) + .containsEntry(CONTENT_TYPE, List.of("application/octet-stream")) + .hasSize(2); + } + + @Test + void multipleHeadersToRemoveFromResponseAreRemoved() { + final Map headers = Map.of(HTTP_HEADER_TO_REMOVE, "A-Value", PRAGMA, "void", EXPIRES, "0"); + final ServerWebExchange inputExchange = setupExchange(headers); + + final var mutator = new RemoveHeadersAfterCacheExchangeMutator(HTTP_HEADER_TO_REMOVE, PRAGMA, EXPIRES); + CachedResponse cachedResponse = new CachedResponse.Builder(HttpStatus.OK).build(); + + mutator.accept(inputExchange, cachedResponse); + + assertThat(inputExchange.getResponse().getHeaders()).doesNotContainKey(HTTP_HEADER_TO_REMOVE) + .doesNotContainKey(PRAGMA) + .doesNotContainKey(EXPIRES) + .containsEntry(CACHE_CONTROL, List.of("max-age=60")) + .containsEntry(CONTENT_TYPE, List.of("application/octet-stream")) + .hasSize(2); + } + + @Test + void headersAreNotModifiedIfHeaderToRemoveIsEmpty() { + final ServerWebExchange inputExchange = setupExchange(Map.of()); + + final var mutator = new RemoveHeadersAfterCacheExchangeMutator(HTTP_HEADER_TO_REMOVE); + CachedResponse cachedResponse = new CachedResponse.Builder(HttpStatus.OK).build(); + + mutator.accept(inputExchange, cachedResponse); + + assertThat(inputExchange.getResponse().getHeaders()).containsEntry(CACHE_CONTROL, List.of("max-age=60")) + .containsEntry(CONTENT_TYPE, List.of("application/octet-stream")) + .hasSize(2); + } + + private ServerWebExchange setupExchange(Map headersToAdd) { + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setCacheControl(CacheControl.maxAge(Duration.ofSeconds(60))); + responseHeaders.setContentType(MediaType.APPLICATION_OCTET_STREAM); + headersToAdd.forEach((k, v) -> responseHeaders.set(k, v)); + + MockServerHttpRequest httpRequest = MockServerHttpRequest.get("https://this").build(); + MockServerWebExchange inputExchange = MockServerWebExchange.from(httpRequest); + MockServerHttpResponse httpResponse = inputExchange.getResponse(); + httpResponse.setStatusCode(HttpStatus.OK); + httpResponse.getHeaders().putAll(responseHeaders); + + return inputExchange; + } + +} From f586c961c0128c8dc03cbeb3909d0b8e950d838c Mon Sep 17 00:00:00 2001 From: spencergibb Date: Wed, 4 Sep 2024 16:06:37 -0400 Subject: [PATCH 2/2] Removes commons lang3 --- .../PathRoutePredicatePathContainerAttrBenchMarkTests.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicatePathContainerAttrBenchMarkTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicatePathContainerAttrBenchMarkTests.java index e7f47220..41055e5c 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicatePathContainerAttrBenchMarkTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicatePathContainerAttrBenchMarkTests.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Random; import java.util.function.Predicate; -import org.apache.commons.lang3.RandomStringUtils; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -47,8 +46,10 @@ public class PathRoutePredicatePathContainerAttrBenchMarkTests { static { predicates = new LinkedList<>(); - PATH_PATTERN_PREFIX = String.format("/%s/%s/", RandomStringUtils.random(20, true, false), - RandomStringUtils.random(10, true, false)); + Random random = new Random(); + String path1 = String.format("%1$" + 20 + "s", random.nextInt()).replace(' ', '0'); + String path2 = String.format("%1$" + 10 + "s", random.nextInt()).replace(' ', '0'); + PATH_PATTERN_PREFIX = String.format("/%s/%s/", path1, path2); for (int i = 0; i < ROUTES_NUM; i++) { PathRoutePredicateFactory.Config config = new PathRoutePredicateFactory.Config() .setPatterns(Collections.singletonList(PATH_PATTERN_PREFIX + i))