From 1e263bbc860a3c5a32b4b6abd540116a9496d3a6 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 12 Dec 2019 15:59:46 -0500 Subject: [PATCH 1/2] Revert "Moves random uuid back to constructor, but only if config is set." This reverts commit 0997dc6f --- .../main/asciidoc/spring-cloud-gateway.adoc | 2 -- .../CompositeRouteDefinitionLocator.java | 32 ++++++++++++++++++- .../cloud/gateway/route/RouteDefinition.java | 9 ------ .../route/RouteDefinitionDefaultIdTests.java | 12 ------- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-gateway.adoc b/docs/src/main/asciidoc/spring-cloud-gateway.adoc index 7507e468..ef59a964 100644 --- a/docs/src/main/asciidoc/spring-cloud-gateway.adoc +++ b/docs/src/main/asciidoc/spring-cloud-gateway.adoc @@ -51,8 +51,6 @@ NOTE: URIs defined in routes without a port will get a default port set to 80 an Spring Cloud Gateway matches routes as part of the Spring WebFlux `HandlerMapping` infrastructure. Spring Cloud Gateway includes many built-in Route Predicate Factories. All of these predicates match on different attributes of the HTTP request. Multiple Route Predicate Factories can be combined and are combined via logical `and`. -WARNING: Previously, Spring Cloud Gateway generated a default Route ID by creating a random UUID. Generating a random UUID is a blocking operation and can lead to instability. Default id generation has been turned off by default. To re-enable this set an environment variable `SPRING_CLOUD_GATEWAY_ROUTE_GENERATE_ID` or system property `spring.cloud.gateway.route.generate-id` to `true`. - === After Route Predicate Factory The After Route Predicate Factory takes one parameter, a datetime. This predicate matches requests that happen after the current datetime. diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java index fbef41b6..a1017b02 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CompositeRouteDefinitionLocator.java @@ -16,22 +16,52 @@ package org.springframework.cloud.gateway.route; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +import org.springframework.util.AlternativeJdkIdGenerator; +import org.springframework.util.IdGenerator; /** * @author Spencer Gibb */ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator { + private static final Log log = LogFactory + .getLog(CompositeRouteDefinitionLocator.class); + private final Flux delegates; + private final IdGenerator idGenerator; + public CompositeRouteDefinitionLocator(Flux delegates) { + this(delegates, new AlternativeJdkIdGenerator()); + } + + public CompositeRouteDefinitionLocator(Flux delegates, + IdGenerator idGenerator) { this.delegates = delegates; + this.idGenerator = idGenerator; } @Override public Flux getRouteDefinitions() { - return this.delegates.flatMap(RouteDefinitionLocator::getRouteDefinitions); + return this.delegates.flatMap(RouteDefinitionLocator::getRouteDefinitions) + .flatMap(routeDefinition -> Mono.justOrEmpty(routeDefinition.getId()) + .defaultIfEmpty(idGenerator.generateId().toString()) + .publishOn(Schedulers.elastic()).map(id -> { + if (routeDefinition.getId() == null) { + routeDefinition.setId(id); + if (log.isDebugEnabled()) { + log.debug( + "Id set on route definition: " + routeDefinition); + } + } + return routeDefinition; + })); } } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinition.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinition.java index d35a1665..ba7b699c 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinition.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinition.java @@ -20,7 +20,6 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.UUID; import javax.validation.Valid; import javax.validation.ValidationException; @@ -39,7 +38,6 @@ import static org.springframework.util.StringUtils.tokenizeToStringArray; @Validated public class RouteDefinition { - @NotEmpty private String id; @NotEmpty @@ -55,13 +53,6 @@ public class RouteDefinition { private int order = 0; public RouteDefinition() { - String envGenerateId = System.getenv("SPRING_CLOUD_GATEWAY_ROUTE_GENERATE_ID"); - String propGenerateId = System - .getProperty("spring.cloud.gateway.route.generate-id", "false"); - if ("true".equalsIgnoreCase(envGenerateId) - || "true".equalsIgnoreCase(propGenerateId)) { - id = UUID.randomUUID().toString(); - } } public RouteDefinition(String text) { diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionDefaultIdTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionDefaultIdTests.java index b68b1ffc..8e28bad5 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionDefaultIdTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionDefaultIdTests.java @@ -16,8 +16,6 @@ package org.springframework.cloud.gateway.route; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,16 +43,6 @@ public class RouteDefinitionDefaultIdTests { @Autowired private GatewayProperties properties; - @BeforeClass - public static void before() { - System.setProperty("spring.cloud.gateway.route.generate-id", "true"); - } - - @AfterClass - public static void after() { - System.clearProperty("spring.cloud.gateway.route.generate-id"); - } - @Test public void testDefaultIdWorks() { assertThat(properties.getRoutes()).hasSize(1); From b517f7d8005c23be3de7e00980a082364d1b0b0f Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 12 Dec 2019 16:00:14 -0500 Subject: [PATCH 2/2] Adds recursive filter check --- .../cloud/gateway/test/BaseWebClientTests.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/BaseWebClientTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/BaseWebClientTests.java index ff944508..d9f0904a 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/BaseWebClientTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/BaseWebClientTests.java @@ -23,6 +23,7 @@ import com.netflix.loadbalancer.ServerList; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Before; +import reactor.core.publisher.Mono; import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -90,6 +91,16 @@ public class BaseWebClientTests { return new HttpBinCompatibleController(); } + @Bean + public GlobalFilter recursiveHttpbinFilter() { + return (exchange, chain) -> { + if (exchange.getRequest().getPath().toString().contains("httpbin/httpbin")) { + return Mono.error(new IllegalStateException("recursive call to /httpbin")); + } + return chain.filter(exchange); + }; + } + @Bean @Order(500) public GlobalFilter modifyResponseFilter() {