From 4a103dea8885f0da5e1589e598eb3f8e13f76438 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 11 Nov 2024 13:58:06 -0500 Subject: [PATCH 1/6] Makes test more general --- .../factory/SpringCloudCircuitBreakerFilterFactoryTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java index 9368ed5c..1966fa9e 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable; import org.springframework.cloud.gateway.test.BaseWebClientTests; import org.springframework.http.HttpStatus; +import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.http.MediaType.APPLICATION_JSON; /** @@ -154,11 +155,11 @@ public abstract class SpringCloudCircuitBreakerFilterFactoryTests extends BaseWe .is5xxServerError() .expectBody() .jsonPath("$.status") - .isEqualTo(504) + .value(status -> assertThat(HttpStatus.valueOf((Integer) status).is5xxServerError()).isTrue()) .jsonPath("$.message") .isNotEmpty() .jsonPath("$.error") - .isEqualTo("Gateway Timeout"); + .isNotEmpty(); } @Test From 068c4ea79db1f6e0727deee72705647adc53f15b Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 11 Nov 2024 14:40:52 -0500 Subject: [PATCH 2/6] Disable test for now --- .../handler/predicate/PathRoutePredicateFactoryTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java index d986e0e9..4c5bf50e 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java @@ -19,6 +19,7 @@ package org.springframework.cloud.gateway.handler.predicate; import java.util.Arrays; import java.util.function.Predicate; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Value; @@ -92,6 +93,7 @@ public class PathRoutePredicateFactoryTests extends BaseWebClientTests { } @Test + @Disabled public void pathRouteWorksWithPercent() { testClient.get() .uri("/abc/123%/function") From 6f5d1e720bfd8b92c429ce6b6a3a434e45fa6ae5 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 11 Nov 2024 15:04:02 -0500 Subject: [PATCH 3/6] Reenable test with firewall disabled. --- .../PathRoutePredicateFactoryTests.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java index 4c5bf50e..cb92f046 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java @@ -22,7 +22,9 @@ import java.util.function.Predicate; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; @@ -34,6 +36,8 @@ 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.security.web.server.WebFilterChainProxy; +import org.springframework.security.web.server.firewall.StrictServerWebExchangeFirewall; import org.springframework.test.annotation.DirtiesContext; import org.springframework.web.server.ServerWebExchange; @@ -93,7 +97,7 @@ public class PathRoutePredicateFactoryTests extends BaseWebClientTests { } @Test - @Disabled + //@Disabled public void pathRouteWorksWithPercent() { testClient.get() .uri("/abc/123%/function") @@ -149,6 +153,22 @@ public class PathRoutePredicateFactoryTests extends BaseWebClientTests { @Value("${test.uri}") String uri; + // TODO: move to bean of StrictServerWebExchangeFirewall + @Bean + public BeanPostProcessor firewallPostProcessor() { + return new BeanPostProcessor() { + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { + if (bean instanceof WebFilterChainProxy webFilterChainProxy) { + StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + firewall.setAllowUrlEncodedPercent(true); + webFilterChainProxy.setFirewall(firewall); + } + return bean; + } + }; + } + @Bean public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { return builder.routes() From eea507f98f3b634d23b22faa80fdb01a7c094c6d Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 11 Nov 2024 16:26:57 -0500 Subject: [PATCH 4/6] removed unused import --- .../handler/predicate/PathRoutePredicateFactoryTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java index cb92f046..73493367 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java @@ -19,7 +19,6 @@ package org.springframework.cloud.gateway.handler.predicate; import java.util.Arrays; import java.util.function.Predicate; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.BeansException; From 353a503dc12716659bc16524b9e961af9b4ad2cc Mon Sep 17 00:00:00 2001 From: spencergibb Date: Thu, 14 Nov 2024 12:04:35 -0500 Subject: [PATCH 5/6] Formatting --- .../handler/predicate/PathRoutePredicateFactoryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java index 73493367..dde2af02 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/handler/predicate/PathRoutePredicateFactoryTests.java @@ -96,7 +96,7 @@ public class PathRoutePredicateFactoryTests extends BaseWebClientTests { } @Test - //@Disabled + // @Disabled public void pathRouteWorksWithPercent() { testClient.get() .uri("/abc/123%/function") From f2ee0c068d4c7797a304a56fc843edd656aba157 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Thu, 14 Nov 2024 12:05:00 -0500 Subject: [PATCH 6/6] Use HttpHeaders#headerSet where appropriate Fixes gh-3596 --- .../server/mvc/filter/ForwardedRequestHeadersFilter.java | 2 +- .../server/mvc/filter/RemoveHopByHopRequestHeadersFilter.java | 2 +- .../server/mvc/filter/XForwardedRequestHeadersFilter.java | 2 +- .../cloud/gateway/server/mvc/test/client/ExchangeResult.java | 2 +- .../cloud/gateway/filter/WebsocketRoutingFilter.java | 2 +- .../filter/factory/RequestHeaderSizeGatewayFilterFactory.java | 2 +- .../cloud/gateway/filter/headers/ForwardedHeadersFilter.java | 2 +- .../cloud/gateway/filter/headers/GRPCRequestHeadersFilter.java | 2 +- .../gateway/filter/headers/RemoveHopByHopHeadersFilter.java | 2 +- .../cloud/gateway/filter/headers/XForwardedHeadersFilter.java | 2 +- .../gateway/filter/headers/HttpHeadersFilterMixedTypeTests.java | 2 +- .../cloud/gateway/filter/headers/HttpHeadersFilterTests.java | 2 +- .../cloud/gateway/test/HttpBinCompatibleController.java | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/ForwardedRequestHeadersFilter.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/ForwardedRequestHeadersFilter.java index 7924c5ef..e4fedaf9 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/ForwardedRequestHeadersFilter.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/ForwardedRequestHeadersFilter.java @@ -93,7 +93,7 @@ public class ForwardedRequestHeadersFilter implements HttpHeadersFilter.RequestH HttpHeaders updated = new HttpHeaders(); // copy all headers except Forwarded - for (Map.Entry> entry : original.entrySet()) { + for (Map.Entry> entry : original.headerSet()) { if (!entry.getKey().equalsIgnoreCase(FORWARDED_HEADER)) { updated.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RemoveHopByHopRequestHeadersFilter.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RemoveHopByHopRequestHeadersFilter.java index d57c2213..0206a38d 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RemoveHopByHopRequestHeadersFilter.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RemoveHopByHopRequestHeadersFilter.java @@ -55,7 +55,7 @@ public class RemoveHopByHopRequestHeadersFilter implements RequestHttpHeadersFil static HttpHeaders filter(HttpHeaders input, Set headersToRemove) { HttpHeaders filtered = new HttpHeaders(); - for (Map.Entry> entry : input.entrySet()) { + for (Map.Entry> entry : input.headerSet()) { if (!headersToRemove.contains(entry.getKey().toLowerCase(Locale.ROOT))) { filtered.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/XForwardedRequestHeadersFilter.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/XForwardedRequestHeadersFilter.java index 428370b6..db5e0539 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/XForwardedRequestHeadersFilter.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/XForwardedRequestHeadersFilter.java @@ -374,7 +374,7 @@ public class XForwardedRequestHeadersFilter implements HttpHeadersFilter.Request HttpHeaders original = input; HttpHeaders updated = new HttpHeaders(); - for (Map.Entry> entry : original.entrySet()) { + for (Map.Entry> entry : original.headerSet()) { updated.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/test/client/ExchangeResult.java b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/test/client/ExchangeResult.java index 427aed77..e87ea133 100644 --- a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/test/client/ExchangeResult.java +++ b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/test/client/ExchangeResult.java @@ -264,7 +264,7 @@ public class ExchangeResult { } private String formatHeaders(HttpHeaders headers, String delimiter) { - return headers.entrySet() + return headers.headerSet() .stream() .map(entry -> entry.getKey() + ": " + entry.getValue()) .collect(Collectors.joining(delimiter)); diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/WebsocketRoutingFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/WebsocketRoutingFilter.java index 04b02e22..7a0bddb0 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/WebsocketRoutingFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/WebsocketRoutingFilter.java @@ -143,7 +143,7 @@ public class WebsocketRoutingFilter implements GlobalFilter, Ordered { headersFilters.add((headers, exchange) -> { HttpHeaders filtered = new HttpHeaders(); - for (Map.Entry> entry : headers.entrySet()) { + for (Map.Entry> entry : headers.headerSet()) { if (!entry.getKey().toLowerCase(Locale.ROOT).startsWith("sec-websocket")) { filtered.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestHeaderSizeGatewayFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestHeaderSizeGatewayFilterFactory.java index 8dcd9413..b6e0d6b6 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestHeaderSizeGatewayFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestHeaderSizeGatewayFilterFactory.java @@ -69,7 +69,7 @@ public class RequestHeaderSizeGatewayFilterFactory HttpHeaders headers = request.getHeaders(); HashMap longHeaders = new HashMap<>(); - for (Map.Entry> headerEntry : headers.entrySet()) { + for (Map.Entry> headerEntry : headers.headerSet()) { long headerSizeInBytes = 0L; headerSizeInBytes += headerEntry.getKey().getBytes().length; List values = headerEntry.getValue(); diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/ForwardedHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/ForwardedHeadersFilter.java index 788d305c..b95cb191 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/ForwardedHeadersFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/ForwardedHeadersFilter.java @@ -96,7 +96,7 @@ public class ForwardedHeadersFilter implements HttpHeadersFilter, Ordered { HttpHeaders updated = new HttpHeaders(); // copy all headers except Forwarded - for (Map.Entry> entry : original.entrySet()) { + for (Map.Entry> entry : original.headerSet()) { if (!entry.getKey().equalsIgnoreCase(FORWARDED_HEADER)) { updated.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCRequestHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCRequestHeadersFilter.java index 71489ae9..1c7e9318 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCRequestHeadersFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCRequestHeadersFilter.java @@ -33,7 +33,7 @@ public class GRPCRequestHeadersFilter implements HttpHeadersFilter, Ordered { public HttpHeaders filter(HttpHeaders headers, ServerWebExchange exchange) { HttpHeaders updated = new HttpHeaders(); - for (Map.Entry> entry : headers.entrySet()) { + for (Map.Entry> entry : headers.headerSet()) { updated.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/RemoveHopByHopHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/RemoveHopByHopHeadersFilter.java index 2a43414b..556f1c97 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/RemoveHopByHopHeadersFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/RemoveHopByHopHeadersFilter.java @@ -74,7 +74,7 @@ public class RemoveHopByHopHeadersFilter implements HttpHeadersFilter, Ordered { Set headersToRemove = new HashSet<>(headers); headersToRemove.addAll(connectionOptions); - for (Map.Entry> entry : originalHeaders.entrySet()) { + for (Map.Entry> entry : originalHeaders.headerSet()) { if (!headersToRemove.contains(entry.getKey().toLowerCase(Locale.ROOT))) { filtered.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/XForwardedHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/XForwardedHeadersFilter.java index 1623349e..2c1e8363 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/XForwardedHeadersFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/XForwardedHeadersFilter.java @@ -202,7 +202,7 @@ public class XForwardedHeadersFilter implements HttpHeadersFilter, Ordered { HttpHeaders original = input; HttpHeaders updated = new HttpHeaders(); - for (Map.Entry> entry : original.entrySet()) { + for (Map.Entry> entry : original.headerSet()) { updated.addAll(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterMixedTypeTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterMixedTypeTests.java index 1ee3cb38..cbb9a4d2 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterMixedTypeTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterMixedTypeTests.java @@ -59,7 +59,7 @@ public class HttpHeadersFilterMixedTypeTests { @Override public HttpHeaders filter(HttpHeaders headers, ServerWebExchange exchange) { HttpHeaders result = new HttpHeaders(); - headers.entrySet().forEach(entry -> { + headers.headerSet().forEach(entry -> { if (!headerNamesSet.contains(entry.getKey())) { result.put(entry.getKey(), entry.getValue()); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterTests.java index 9940d035..9619b71f 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/HttpHeadersFilterTests.java @@ -52,7 +52,7 @@ public class HttpHeadersFilterTests { private HttpHeaders filter(HttpHeaders input, String keyToFilter) { HttpHeaders filtered = new HttpHeaders(); - input.entrySet() + input.headerSet() .stream() .filter(entry -> !entry.getKey().equals(keyToFilter)) .forEach(entry -> filtered.addAll(entry.getKey(), entry.getValue())); diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java index dbf7dd24..5872965f 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java @@ -177,7 +177,7 @@ public class HttpBinCompatibleController { public ResponseEntity> responseHeaders(@PathVariable int status, ServerWebExchange exchange) { HttpHeaders httpHeaders = exchange.getRequest() .getHeaders() - .entrySet() + .headerSet() .stream() .filter(entry -> entry.getKey().startsWith("X-Test-")) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue,