From 0ddf816016762fcd14b61ee7632bb15630cd50b1 Mon Sep 17 00:00:00 2001 From: Gaemi Date: Thu, 19 Sep 2019 06:30:20 +0900 Subject: [PATCH 01/10] Modify newContentType to be reflected in downstream requests in ModifyRequestBodyGatewayFilterFactory Fixes gh-1305 Fixes gh-1306 --- ...ModifyRequestBodyGatewayFilterFactory.java | 2 +- ...yRequestBodyGatewayFilterFactoryTests.java | 83 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java index a819a5de..9286949c 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java @@ -117,7 +117,7 @@ public class ModifyRequestBodyGatewayFilterFactory extends public HttpHeaders getHeaders() { long contentLength = headers.getContentLength(); HttpHeaders httpHeaders = new HttpHeaders(); - httpHeaders.putAll(super.getHeaders()); + httpHeaders.putAll(headers); if (contentLength > 0) { httpHeaders.setContentLength(contentLength); } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java new file mode 100644 index 00000000..aac58675 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java @@ -0,0 +1,83 @@ +/* + * Copyright 2013-2019 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.rewrite; + +import org.junit.Test; +import org.junit.runner.RunWith; +import reactor.core.publisher.Mono; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.gateway.route.RouteLocator; +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.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.reactive.function.BodyInserters; + +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Junghoon Song + */ +@RunWith(SpringRunner.class) +@SpringBootTest(webEnvironment = RANDOM_PORT) +@DirtiesContext +public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTests { + + @Test + public void modifyRequestBody() { + testClient.post().uri("/post").header("Host", "www.modifyrequestbody.org") + .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_XML_VALUE) + .body(BodyInserters.fromValue("request")) + .exchange() + .expectStatus().isEqualTo(HttpStatus.OK) + .expectBody() + .jsonPath("headers.Content-Type").isEqualTo(MediaType.APPLICATION_JSON_VALUE) + .jsonPath("data").isEqualTo("modifyrequest"); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + public static class TestConfig { + + @Value("${test.uri}") + String uri; + + @Bean + public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { + return builder.routes() + .route("test_modify_request_body", + r -> r.order(-1).host("**.modifyrequestbody.org") + .filters(f -> f.modifyRequestBody(String.class, String.class, + MediaType.APPLICATION_JSON_VALUE, (serverWebExchange, aVoid) -> { + return Mono.just("modifyrequest"); + }) + ) + .uri(uri)) + .build(); + } + } +} From 6553bce9f621d78c3d2a84d718a0949ff2b07c80 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 27 Feb 2020 18:39:45 -0500 Subject: [PATCH 02/10] Fixes test to call proper method. --- .../cloud/gateway/webflux/ProductionConfigurationTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java b/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java index b9cfe0e5..dd0e568d 100644 --- a/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java +++ b/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java @@ -350,7 +350,7 @@ public class ProductionConfigurationTests { ProxyExchange> proxy) throws Exception { body.put("id", id); return proxy.uri(home.toString() + "/bars").body(Arrays.asList(body)) - .post(this::first); + .forward(this::first); } } From fd1763d5b9f2b2f744d87f4ed0a41d7c7678a896 Mon Sep 17 00:00:00 2001 From: echooymxq Date: Wed, 15 Jan 2020 13:14:55 +0800 Subject: [PATCH 03/10] Supports timeouts from properties rather than yaml. Fixes gh-1522 --- .../gateway/filter/NettyRoutingFilter.java | 29 ++++++- .../NettyRoutingFilterCompatibleTests.java | 82 +++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java index c8ffc72a..6cf87f57 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java @@ -252,17 +252,38 @@ public class NettyRoutingFilter implements GlobalFilter, Ordered { * @return */ protected HttpClient getHttpClient(Route route, ServerWebExchange exchange) { - Integer connectTimeout = (Integer) route.getMetadata().get(CONNECT_TIMEOUT_ATTR); - if (connectTimeout != null) { + Object connectTimeoutAttr = route.getMetadata().get(CONNECT_TIMEOUT_ATTR); + if (connectTimeoutAttr != null) { + Integer connectTimeout = getInteger(connectTimeoutAttr); return this.httpClient.tcpConfiguration((tcpClient) -> tcpClient .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, connectTimeout)); } return httpClient; } + static Integer getInteger(Object connectTimeoutAttr) { + Integer connectTimeout; + if (connectTimeoutAttr instanceof Integer) { + connectTimeout = (Integer) connectTimeoutAttr; + } + else { + connectTimeout = Integer.parseInt(connectTimeoutAttr.toString()); + } + return connectTimeout; + } + private Duration getResponseTimeout(Route route) { - Number responseTimeout = (Number) route.getMetadata().get(RESPONSE_TIMEOUT_ATTR); - return responseTimeout != null ? Duration.ofMillis(responseTimeout.longValue()) + Object responseTimeoutAttr = route.getMetadata().get(RESPONSE_TIMEOUT_ATTR); + Long responseTimeout = null; + if (responseTimeoutAttr != null) { + if (responseTimeoutAttr instanceof Number) { + responseTimeout = ((Number) responseTimeoutAttr).longValue(); + } + else { + responseTimeout = Long.valueOf(responseTimeoutAttr.toString()); + } + } + return responseTimeout != null ? Duration.ofMillis(responseTimeout) : properties.getResponseTimeout(); } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java new file mode 100644 index 00000000..4291fb4a --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java @@ -0,0 +1,82 @@ +/* + * Copyright 2013-2019 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; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.gateway.route.Route; +import org.springframework.cloud.gateway.test.BaseWebClientTests; +import org.springframework.context.annotation.Import; +import org.springframework.http.HttpStatus; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.server.ServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * This test just avoid class cast exception with YAML or Properties parsing. + * + * {@link NettyRoutingFilter#getHttpClient(Route, ServerWebExchange)} + * {@link NettyRoutingFilter#getResponseTimeout(Route)} + * + * @author echooymxq + **/ +@RunWith(SpringRunner.class) +@SpringBootTest(properties = { "spring.cloud.gateway.routes[0].id=route_connect_timeout", + "spring.cloud.gateway.routes[0].uri=http://localhost:32167", + "spring.cloud.gateway.routes[0].predicates[0].name=Path", + "spring.cloud.gateway.routes[0].predicates[0].args[pattern]=/connect/delay/{timeout}", + "spring.cloud.gateway.routes[0].metadata[connect-timeout]=5", + "spring.cloud.gateway.routes[1].id=route_response_timeout", + "spring.cloud.gateway.routes[1].uri=lb://testservice", + "spring.cloud.gateway.routes[1].predicates[0].name=Path", + "spring.cloud.gateway.routes[1].predicates[0].args[pattern]=/route/delay/{timeout}", + "spring.cloud.gateway.routes[1].filters[0]=StripPrefix=1", + "spring.cloud.gateway.routes[1].metadata.response-timeout=1000" }, + webEnvironment = RANDOM_PORT) +@DirtiesContext +public class NettyRoutingFilterCompatibleTests extends BaseWebClientTests { + + @Test + public void shouldApplyConnectTimeoutPerRoute() { + assertThat(NettyRoutingFilter.getInteger("5")).isEqualTo(5); + assertThat(NettyRoutingFilter.getInteger(5)).isEqualTo(5); + } + + @Test + public void shouldApplyResponseTimeoutPerRoute() { + testClient.get().uri("/route/delay/2").exchange().expectStatus() + .isEqualTo(HttpStatus.GATEWAY_TIMEOUT).expectBody().jsonPath("$.status") + .isEqualTo(String.valueOf(HttpStatus.GATEWAY_TIMEOUT.value())) + .jsonPath("$.message") + .isEqualTo("Response took longer than timeout: PT1S"); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + public static class TestConfig { + + } + +} From 88be470c9bd6fa28e5f4f281108911a53b36e074 Mon Sep 17 00:00:00 2001 From: "owen.q" Date: Fri, 20 Dec 2019 16:47:36 +0900 Subject: [PATCH 04/10] Add error handling to invalid RouteDefinition parse - invalid definitions are logged with warn level fixes gh-1376 fixes gh-1496 --- .../route/RouteDefinitionRouteLocator.java | 6 ++- .../RouteDefinitionRouteLocatorTests.java | 53 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java index acd3f1e6..9870b257 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java @@ -144,7 +144,11 @@ public class RouteDefinitionRouteLocator @Override public Flux getRoutes() { return this.routeDefinitionLocator.getRouteDefinitions().map(this::convertToRoute) - // TODO: error handling + .onErrorContinue((error, obj) -> { + if (logger.isWarnEnabled()) { + logger.warn("RouteDefinition id " + ((RouteDefinition) obj).getId() + " will be ignored. Definition has invalid configs, " + error.getMessage()); + } + }) .map(route -> { if (logger.isDebugEnabled()) { logger.debug("RouteDefinition matched: " + route.getId()); diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java index 53da140d..f21a0a83 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java @@ -81,6 +81,59 @@ public class RouteDefinitionRouteLocatorTests { .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); } + @Test + public void contextLoadsWithErrorRecovery() { + List predicates = Arrays + .asList(new HostRoutePredicateFactory()); + List gatewayFilterFactories = Arrays.asList( + new RemoveResponseHeaderGatewayFilterFactory(), + new AddResponseHeaderGatewayFilterFactory(), + new TestOrderedGatewayFilterFactory()); + GatewayProperties gatewayProperties = new GatewayProperties(); + gatewayProperties.setRoutes(containsInvalidRoutes()); + + RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( + new PropertiesRouteDefinitionLocator(gatewayProperties), predicates, + gatewayFilterFactories, gatewayProperties, + new DefaultConversionService()); + + List routes = routeDefinitionRouteLocator.getRoutes().collectList() + .block(); + List filters = routes.get(0).getFilters(); + assertThat(filters).hasSize(3); + assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + assertThat(getFilterClassName(filters.get(2))) + .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + } + + private List containsInvalidRoutes() { + return Arrays.asList( + new RouteDefinition() { + { + setId("foo"); + setUri(URI.create("https://foo.example.com")); + setPredicates( + Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + setFilters(Arrays.asList( + new FilterDefinition("RemoveResponseHeader=Server"), + new FilterDefinition("TestOrdered="), + new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); + } + }, + + new RouteDefinition() { + { + setId("exceptionRaised"); + setUri(URI.create("https://foo.example.com")); + setPredicates( + Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + setFilters(Arrays.asList(new FilterDefinition("Generate exception"))); + } + } + ); + } + private String getFilterClassName(GatewayFilter target) { if (target instanceof OrderedGatewayFilter) { return getFilterClassName(((OrderedGatewayFilter) target).getDelegate()); From 75a34df7e2bdcaac4cac052e74642ed1b365665f Mon Sep 17 00:00:00 2001 From: MattyA Date: Mon, 17 Feb 2020 11:01:59 +0000 Subject: [PATCH 05/10] Moves routeId generation schedule to more specific location. This allows only the routeId generation to be on an elastic scheduler while everything else remains on the main thread. That way if there are errors loading routes, this will halt execution. Fixes gh-1574 Fixes gh-1575 --- .../CompositeRouteDefinitionLocator.java | 23 +++--- .../RouteDefinitionRouteLocatorTests.java | 81 ++++++++++++++++--- .../RouteConstructionIntegrationTests.java | 79 ++++++++++++++++++ .../application-verification-route.yml | 11 +++ 4 files changed, 172 insertions(+), 22 deletions(-) create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java create mode 100644 spring-cloud-gateway-core/src/test/resources/application-verification-route.yml 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 5195b407..11df375c 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 @@ -50,18 +50,21 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator { @Override public Flux 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); - } + .flatMap(routeDefinition -> { + if (routeDefinition.getId() == null) { + return randomId().map(id -> { + routeDefinition.setId(id); + if (log.isDebugEnabled()) { + log.debug("Id set on route definition: " + routeDefinition); } return routeDefinition; - })); + }); + } + return Mono.just(routeDefinition); + }); } + protected Mono randomId() { + return Mono.fromSupplier(idGenerator::toString).publishOn(Schedulers.boundedElastic()); + } } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java index f21a0a83..c2f6919d 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java @@ -21,6 +21,8 @@ import java.util.Arrays; import java.util.List; import org.junit.Test; +import reactor.core.publisher.Flux; +import reactor.test.StepVerifier; import org.springframework.cloud.gateway.config.GatewayProperties; import org.springframework.cloud.gateway.config.PropertiesRouteDefinitionLocator; @@ -34,7 +36,7 @@ import org.springframework.cloud.gateway.filter.factory.RemoveResponseHeaderGate import org.springframework.cloud.gateway.handler.predicate.HostRoutePredicateFactory; import org.springframework.cloud.gateway.handler.predicate.PredicateDefinition; import org.springframework.cloud.gateway.handler.predicate.RoutePredicateFactory; -import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.cloud.gateway.support.ConfigurationService; import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -66,19 +68,74 @@ public class RouteDefinitionRouteLocatorTests { } })); + PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties); + @SuppressWarnings("deprecation") RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( - new PropertiesRouteDefinitionLocator(gatewayProperties), predicates, - gatewayFilterFactories, gatewayProperties, - new DefaultConversionService()); + new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), + predicates, gatewayFilterFactories, gatewayProperties, + new ConfigurationService()); - List routes = routeDefinitionRouteLocator.getRoutes().collectList() - .block(); - List filters = routes.get(0).getFilters(); - assertThat(filters).hasSize(3); - assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); - assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); - assertThat(getFilterClassName(filters.get(2))) - .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + StepVerifier.create(routeDefinitionRouteLocator.getRoutes()) + .assertNext(route -> { + List filters = route.getFilters(); + assertThat(filters).hasSize(3); + assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + assertThat(getFilterClassName(filters.get(2))) + .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + }) + .expectComplete() + .verify(); + } + + @Test + public void contextLoadsWithErrorRecovery() { + List predicates = Arrays + .asList(new HostRoutePredicateFactory()); + List gatewayFilterFactories = Arrays.asList( + new RemoveResponseHeaderGatewayFilterFactory(), + new AddResponseHeaderGatewayFilterFactory(), + new TestOrderedGatewayFilterFactory()); + GatewayProperties gatewayProperties = new GatewayProperties(); + gatewayProperties.setRoutes(containsInvalidRoutes()); + + PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties); + @SuppressWarnings("deprecation") + RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( + new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), + predicates, gatewayFilterFactories, gatewayProperties, + new ConfigurationService()); + + StepVerifier.create(routeDefinitionRouteLocator.getRoutes()) + .assertNext(route -> { + List filters = route.getFilters(); + assertThat(filters).hasSize(3); + assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + assertThat(getFilterClassName(filters.get(2))) + .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + }) + .expectComplete() + .verify(); + } + + private List containsInvalidRoutes() { + RouteDefinition foo = new RouteDefinition(); + foo.setId("foo"); + foo.setUri(URI.create("https://foo.example.com")); + foo.setPredicates( + Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + foo.setFilters(Arrays.asList( + new FilterDefinition("RemoveResponseHeader=Server"), + new FilterDefinition("TestOrdered="), + new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); + RouteDefinition bad = new RouteDefinition(); + bad.setId("exceptionRaised"); + bad.setUri(URI.create("https://foo.example.com")); + bad.setPredicates( + Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + bad.setFilters(Arrays.asList(new FilterDefinition("Generate exception"))); + return Arrays.asList(foo, bad); } @Test diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java new file mode 100644 index 00000000..b6b65192 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java @@ -0,0 +1,79 @@ +/* + * 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.test; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.cloud.gateway.filter.GatewayFilter; +import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory; +import org.springframework.context.annotation.Bean; + +public class RouteConstructionIntegrationTests { + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void routesWithVerificationShouldFail() { + exception.expect(Throwable.class); + new SpringApplicationBuilder(TestConfig.class) + .profiles("verification-route") + .run(); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + public static class TestConfig { + + @Bean + public TestFilterGatewayFilterFactory testFilterGatewayFilterFactory() { + return new TestFilterGatewayFilterFactory(); + } + + } + + public static class TestFilterGatewayFilterFactory + extends AbstractGatewayFilterFactory { + + public TestFilterGatewayFilterFactory() { + super(Config.class); + } + + @Override + public GatewayFilter apply(Config config) { + throw new AssertionError("Stop right now!"); + } + + public static class Config { + + private String arg1; + + public String getArg1() { + return arg1; + } + + public void setArg1(String arg1) { + this.arg1 = arg1; + } + } + } +} diff --git a/spring-cloud-gateway-core/src/test/resources/application-verification-route.yml b/spring-cloud-gateway-core/src/test/resources/application-verification-route.yml new file mode 100644 index 00000000..c0f92d49 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/resources/application-verification-route.yml @@ -0,0 +1,11 @@ +spring: + cloud: + gateway: + routes: + - uri: https://example.com + predicates: + - Path=/verification/** + filters: + - name: TestFilter + args: + arg1: world \ No newline at end of file From 05ba82855e8f6cf20a6eda35a6ad83cce302af57 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 27 Feb 2020 23:48:08 -0500 Subject: [PATCH 06/10] Makes failing on route definition errors opt-out. Since this is a behavior change, the new behavior needs to be opt in. See gh-1376 --- .../gateway/config/GatewayProperties.java | 23 +++- .../CompositeRouteDefinitionLocator.java | 7 +- .../route/RouteDefinitionRouteLocator.java | 35 +++--- .../RouteDefinitionRouteLocatorTests.java | 109 +++++------------- .../RouteConstructionIntegrationTests.java | 6 +- 5 files changed, 75 insertions(+), 105 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayProperties.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayProperties.java index 6db4bac2..c67fc577 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayProperties.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayProperties.java @@ -29,6 +29,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.cloud.gateway.filter.FilterDefinition; import org.springframework.cloud.gateway.route.RouteDefinition; +import org.springframework.core.style.ToStringCreator; import org.springframework.http.MediaType; import org.springframework.validation.annotation.Validated; @@ -56,6 +57,12 @@ public class GatewayProperties { private List streamingMediaTypes = Arrays .asList(MediaType.TEXT_EVENT_STREAM, MediaType.APPLICATION_STREAM_JSON); + /** + * Option to fail on route definition errors, defaults to true. Otherwise, a warning + * is logged. + */ + private boolean failOnRouteDefinitionError = true; + public List getRoutes() { return routes; } @@ -83,10 +90,22 @@ public class GatewayProperties { this.streamingMediaTypes = streamingMediaTypes; } + public boolean isFailOnRouteDefinitionError() { + return failOnRouteDefinitionError; + } + + public void setFailOnRouteDefinitionError(boolean failOnRouteDefinitionError) { + this.failOnRouteDefinitionError = failOnRouteDefinitionError; + } + @Override public String toString() { - return "GatewayProperties{" + "routes=" + routes + ", defaultFilters=" - + defaultFilters + ", streamingMediaTypes=" + streamingMediaTypes + '}'; + return new ToStringCreator(this).append("routes", routes) + .append("defaultFilters", defaultFilters) + .append("streamingMediaTypes", streamingMediaTypes) + .append("failOnRouteDefinitionError", failOnRouteDefinitionError) + .toString(); + } } 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 11df375c..4cafd929 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 @@ -55,7 +55,8 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator { return randomId().map(id -> { routeDefinition.setId(id); if (log.isDebugEnabled()) { - log.debug("Id set on route definition: " + routeDefinition); + log.debug( + "Id set on route definition: " + routeDefinition); } return routeDefinition; }); @@ -65,6 +66,8 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator { } protected Mono randomId() { - return Mono.fromSupplier(idGenerator::toString).publishOn(Schedulers.boundedElastic()); + return Mono.fromSupplier(idGenerator::toString) + .publishOn(Schedulers.boundedElastic()); } + } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java index 9870b257..f7ab0183 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java @@ -143,23 +143,26 @@ public class RouteDefinitionRouteLocator @Override public Flux getRoutes() { - return this.routeDefinitionLocator.getRouteDefinitions().map(this::convertToRoute) - .onErrorContinue((error, obj) -> { - if (logger.isWarnEnabled()) { - logger.warn("RouteDefinition id " + ((RouteDefinition) obj).getId() + " will be ignored. Definition has invalid configs, " + error.getMessage()); - } - }) - .map(route -> { - if (logger.isDebugEnabled()) { - logger.debug("RouteDefinition matched: " + route.getId()); - } - return route; - }); + Flux routes = this.routeDefinitionLocator.getRouteDefinitions() + .map(this::convertToRoute); - /* - * TODO: trace logging if (logger.isTraceEnabled()) { - * logger.trace("RouteDefinition did not match: " + routeDefinition.getId()); } - */ + if (!gatewayProperties.isFailOnRouteDefinitionError()) { + // instead of letting error bubble up, continue + routes = routes.onErrorContinue((error, obj) -> { + if (logger.isWarnEnabled()) { + logger.warn("RouteDefinition id " + ((RouteDefinition) obj).getId() + + " will be ignored. Definition has invalid configs, " + + error.getMessage()); + } + }); + } + + return routes.map(route -> { + if (logger.isDebugEnabled()) { + logger.debug("RouteDefinition matched: " + route.getId()); + } + return route; + }); } private Route convertToRoute(RouteDefinition routeDefinition) { diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java index c2f6919d..504128cb 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java @@ -68,24 +68,23 @@ public class RouteDefinitionRouteLocatorTests { } })); - PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties); + PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator( + gatewayProperties); @SuppressWarnings("deprecation") RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), predicates, gatewayFilterFactories, gatewayProperties, new ConfigurationService()); - StepVerifier.create(routeDefinitionRouteLocator.getRoutes()) - .assertNext(route -> { - List filters = route.getFilters(); - assertThat(filters).hasSize(3); - assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); - assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); - assertThat(getFilterClassName(filters.get(2))) - .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); - }) - .expectComplete() - .verify(); + StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> { + List filters = route.getFilters(); + assertThat(filters).hasSize(3); + assertThat(getFilterClassName(filters.get(0))) + .contains("RemoveResponseHeader"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + assertThat(getFilterClassName(filters.get(2))) + .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + }).expectComplete().verify(); } @Test @@ -98,99 +97,43 @@ public class RouteDefinitionRouteLocatorTests { new TestOrderedGatewayFilterFactory()); GatewayProperties gatewayProperties = new GatewayProperties(); gatewayProperties.setRoutes(containsInvalidRoutes()); + gatewayProperties.setFailOnRouteDefinitionError(false); - PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties); + PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator( + gatewayProperties); @SuppressWarnings("deprecation") RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), predicates, gatewayFilterFactories, gatewayProperties, new ConfigurationService()); - StepVerifier.create(routeDefinitionRouteLocator.getRoutes()) - .assertNext(route -> { - List filters = route.getFilters(); - assertThat(filters).hasSize(3); - assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); - assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); - assertThat(getFilterClassName(filters.get(2))) - .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); - }) - .expectComplete() - .verify(); + StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> { + List filters = route.getFilters(); + assertThat(filters).hasSize(3); + assertThat(getFilterClassName(filters.get(0))) + .contains("RemoveResponseHeader"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + assertThat(getFilterClassName(filters.get(2))) + .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); + }).expectComplete().verify(); } private List containsInvalidRoutes() { RouteDefinition foo = new RouteDefinition(); foo.setId("foo"); foo.setUri(URI.create("https://foo.example.com")); - foo.setPredicates( - Arrays.asList(new PredicateDefinition("Host=*.example.com"))); - foo.setFilters(Arrays.asList( - new FilterDefinition("RemoveResponseHeader=Server"), + foo.setPredicates(Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + foo.setFilters(Arrays.asList(new FilterDefinition("RemoveResponseHeader=Server"), new FilterDefinition("TestOrdered="), new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); RouteDefinition bad = new RouteDefinition(); bad.setId("exceptionRaised"); bad.setUri(URI.create("https://foo.example.com")); - bad.setPredicates( - Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + bad.setPredicates(Arrays.asList(new PredicateDefinition("Host=*.example.com"))); bad.setFilters(Arrays.asList(new FilterDefinition("Generate exception"))); return Arrays.asList(foo, bad); } - @Test - public void contextLoadsWithErrorRecovery() { - List predicates = Arrays - .asList(new HostRoutePredicateFactory()); - List gatewayFilterFactories = Arrays.asList( - new RemoveResponseHeaderGatewayFilterFactory(), - new AddResponseHeaderGatewayFilterFactory(), - new TestOrderedGatewayFilterFactory()); - GatewayProperties gatewayProperties = new GatewayProperties(); - gatewayProperties.setRoutes(containsInvalidRoutes()); - - RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( - new PropertiesRouteDefinitionLocator(gatewayProperties), predicates, - gatewayFilterFactories, gatewayProperties, - new DefaultConversionService()); - - List routes = routeDefinitionRouteLocator.getRoutes().collectList() - .block(); - List filters = routes.get(0).getFilters(); - assertThat(filters).hasSize(3); - assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader"); - assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); - assertThat(getFilterClassName(filters.get(2))) - .contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway"); - } - - private List containsInvalidRoutes() { - return Arrays.asList( - new RouteDefinition() { - { - setId("foo"); - setUri(URI.create("https://foo.example.com")); - setPredicates( - Arrays.asList(new PredicateDefinition("Host=*.example.com"))); - setFilters(Arrays.asList( - new FilterDefinition("RemoveResponseHeader=Server"), - new FilterDefinition("TestOrdered="), - new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); - } - }, - - new RouteDefinition() { - { - setId("exceptionRaised"); - setUri(URI.create("https://foo.example.com")); - setPredicates( - Arrays.asList(new PredicateDefinition("Host=*.example.com"))); - setFilters(Arrays.asList(new FilterDefinition("Generate exception"))); - } - } - ); - } - private String getFilterClassName(GatewayFilter target) { if (target instanceof OrderedGatewayFilter) { return getFilterClassName(((OrderedGatewayFilter) target).getDelegate()); diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java index b6b65192..ffa9fe08 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/RouteConstructionIntegrationTests.java @@ -35,8 +35,7 @@ public class RouteConstructionIntegrationTests { @Test public void routesWithVerificationShouldFail() { exception.expect(Throwable.class); - new SpringApplicationBuilder(TestConfig.class) - .profiles("verification-route") + new SpringApplicationBuilder(TestConfig.class).profiles("verification-route") .run(); } @@ -74,6 +73,9 @@ public class RouteConstructionIntegrationTests { public void setArg1(String arg1) { this.arg1 = arg1; } + } + } + } From 2e890864bd3c562a37be75e387346340a9399d9b Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 27 Feb 2020 23:50:09 -0500 Subject: [PATCH 07/10] formatting --- ...yRequestBodyGatewayFilterFactoryTests.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java index aac58675..c9331df8 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java @@ -50,12 +50,10 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes public void modifyRequestBody() { testClient.post().uri("/post").header("Host", "www.modifyrequestbody.org") .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_XML_VALUE) - .body(BodyInserters.fromValue("request")) - .exchange() - .expectStatus().isEqualTo(HttpStatus.OK) - .expectBody() - .jsonPath("headers.Content-Type").isEqualTo(MediaType.APPLICATION_JSON_VALUE) - .jsonPath("data").isEqualTo("modifyrequest"); + .body(BodyInserters.fromValue("request")).exchange().expectStatus() + .isEqualTo(HttpStatus.OK).expectBody().jsonPath("headers.Content-Type") + .isEqualTo(MediaType.APPLICATION_JSON_VALUE).jsonPath("data") + .isEqualTo("modifyrequest"); } @EnableAutoConfiguration @@ -68,16 +66,17 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes @Bean public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { - return builder.routes() - .route("test_modify_request_body", - r -> r.order(-1).host("**.modifyrequestbody.org") - .filters(f -> f.modifyRequestBody(String.class, String.class, - MediaType.APPLICATION_JSON_VALUE, (serverWebExchange, aVoid) -> { - return Mono.just("modifyrequest"); - }) - ) - .uri(uri)) + return builder.routes().route("test_modify_request_body", + r -> r.order(-1).host("**.modifyrequestbody.org") + .filters(f -> f.modifyRequestBody(String.class, String.class, + MediaType.APPLICATION_JSON_VALUE, + (serverWebExchange, aVoid) -> { + return Mono.just("modifyrequest"); + })) + .uri(uri)) .build(); } + } + } From 7eaecedd5e21ded45806d288cc74300460c02b2c Mon Sep 17 00:00:00 2001 From: Stefan_Stus Date: Wed, 31 Jul 2019 22:42:43 +0300 Subject: [PATCH 08/10] Calls RewriteFunction even if body is empty. fixes gh-1219 fixes gh-1220 --- ...odifyResponseBodyGatewayFilterFactory.java | 6 +- .../route/builder/GatewayFilterSpec.java | 24 +++-- .../route/builder/GatewayFilterSpecTests.java | 98 +++++++++++++++++++ .../test/HttpBinCompatibleController.java | 6 ++ .../sample/GatewaySampleApplication.java | 25 +++++ .../sample/GatewaySampleApplicationTests.java | 23 +++++ 6 files changed, 167 insertions(+), 15 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyResponseBodyGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyResponseBodyGatewayFilterFactory.java index 6fb6ef05..0e4d7fda 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyResponseBodyGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyResponseBodyGatewayFilterFactory.java @@ -203,8 +203,10 @@ public class ModifyResponseBodyGatewayFilterFactory extends // TODO: flux or mono Mono modifiedBody = clientResponse.bodyToMono(inClass) - .flatMap(originalBody -> config.rewriteFunction - .apply(exchange, originalBody)); + .flatMap(originalBody -> config.getRewriteFunction() + .apply(exchange, originalBody)) + .switchIfEmpty(Mono.defer(() -> (Mono) config + .getRewriteFunction().apply(exchange, null))); BodyInserter bodyInserter = BodyInserters.fromPublisher(modifiedBody, outClass); diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java index df8aca99..9871d5cc 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java @@ -246,8 +246,7 @@ public class GatewayFilterSpec extends UriSpec { } /** - * A filter that can be used to modify the request body. This filter is BETA and may - * be subject to change in a future release. + * A filter that can be used to modify the request body. * @param inClass the class to convert the incoming request body to * @param outClass the class the Gateway will add to the request before it is routed * @param rewriteFunction the {@link RewriteFunction} that transforms the request body @@ -263,8 +262,7 @@ public class GatewayFilterSpec extends UriSpec { } /** - * A filter that can be used to modify the request body. This filter is BETA and may - * be subject to change in a future release. + * A filter that can be used to modify the request body. * @param inClass the class to convert the incoming request body to * @param outClass the class the Gateway will add to the request before it is routed * @param newContentType the new Content-Type header to be sent @@ -281,9 +279,10 @@ public class GatewayFilterSpec extends UriSpec { } /** - * A filter that can be used to modify the request body. This filter is BETA and may - * be subject to change in a future release. + * A filter that can be used to modify the request body. * @param configConsumer request spec for response modification + * @param the original request body class + * @param the new request body class * @return a {@link GatewayFilterSpec} that can be used to apply additional filters *
 	 * {@code
@@ -304,8 +303,7 @@ public class GatewayFilterSpec extends UriSpec {
 	}
 
 	/**
-	 * A filter that can be used to modify the response body This filter is BETA and may
-	 * be subject to change in a future release.
+	 * A filter that can be used to modify the response body.
 	 * @param inClass the class to conver the response body to
 	 * @param outClass the class the Gateway will add to the response before it is
 	 * returned to the client
@@ -322,8 +320,7 @@ public class GatewayFilterSpec extends UriSpec {
 	}
 
 	/**
-	 * A filter that can be used to modify the response body This filter is BETA and may
-	 * be subject to change in a future release.
+	 * A filter that can be used to modify the response body.
 	 * @param inClass the class to conver the response body to
 	 * @param outClass the class the Gateway will add to the response before it is
 	 * returned to the client
@@ -344,9 +341,10 @@ public class GatewayFilterSpec extends UriSpec {
 	}
 
 	/**
-	 * A filter that can be used to modify the response body using custom spec. This
-	 * filter is BETA and may be subject to change in a future release.
+	 * A filter that can be used to modify the response body using custom spec.
 	 * @param configConsumer response spec for response modification
+	 * @param  the original response body class
+	 * @param  the new response body class
 	 * @return a {@link GatewayFilterSpec} that can be used to apply additional filters
 	 * 
 	 * {@code
@@ -501,7 +499,7 @@ public class GatewayFilterSpec extends UriSpec {
 	}
 
 	/**
-	 * A filter which rewrites the request path before it is routed by the Gateway
+	 * A filter which rewrites the request path before it is routed by the Gateway.
 	 * @param regex a Java regular expression to match the path against
 	 * @param replacement the replacement for the path
 	 * @return a {@link GatewayFilterSpec} that can be used to apply additional filters
diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java
index c9e1fe44..871d7834 100644
--- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java
+++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java
@@ -21,14 +21,18 @@ import reactor.core.publisher.Mono;
 
 import org.springframework.cloud.gateway.filter.GatewayFilter;
 import org.springframework.cloud.gateway.filter.GatewayFilterChain;
+import org.springframework.cloud.gateway.filter.NettyWriteResponseFilter;
 import org.springframework.cloud.gateway.filter.OrderedGatewayFilter;
+import org.springframework.cloud.gateway.filter.factory.rewrite.ModifyResponseBodyGatewayFilterFactory;
 import org.springframework.cloud.gateway.route.Route;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.core.Ordered;
+import org.springframework.http.MediaType;
 import org.springframework.web.server.ServerWebExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class GatewayFilterSpecTests {
 
@@ -81,6 +85,100 @@ public class GatewayFilterSpecTests {
 		assertFilter(route.getFilters().get(1), MyOrderedFilter.class, 1000);
 	}
 
+	@Test
+	public void shouldSetModifyBodyResponseFilterWithRewriteFunction() {
+		ConfigurableApplicationContext context = mock(
+				ConfigurableApplicationContext.class);
+		Route.AsyncBuilder routeBuilder = Route.async().id("123").uri("abc:123")
+				.predicate(exchange -> true);
+
+		when(context.getBean(ModifyResponseBodyGatewayFilterFactory.class))
+				.thenReturn(new ModifyResponseBodyGatewayFilterFactory());
+
+		RouteLocatorBuilder.Builder routes = new RouteLocatorBuilder(context).routes();
+		GatewayFilterSpec spec = new GatewayFilterSpec(routeBuilder, routes);
+		spec.modifyResponseBody(String.class, String.class,
+				(exchange, s) -> Mono.just(s));
+
+		Route route = routeBuilder.build();
+		assertThat(route.getFilters()).hasSize(1);
+
+		assertFilter(route.getFilters().get(0),
+				ModifyResponseBodyGatewayFilterFactory.ModifyResponseGatewayFilter.class,
+				NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER - 1);
+	}
+
+	@Test
+	public void shouldSetModifyBodyResponseFilterWithRewriteFunctionAndEmptyBodySupplier() {
+		ConfigurableApplicationContext context = mock(
+				ConfigurableApplicationContext.class);
+		Route.AsyncBuilder routeBuilder = Route.async().id("123").uri("abc:123")
+				.predicate(exchange -> true);
+
+		when(context.getBean(ModifyResponseBodyGatewayFilterFactory.class))
+				.thenReturn(new ModifyResponseBodyGatewayFilterFactory());
+
+		RouteLocatorBuilder.Builder routes = new RouteLocatorBuilder(context).routes();
+		GatewayFilterSpec spec = new GatewayFilterSpec(routeBuilder, routes);
+		spec.modifyResponseBody(String.class, String.class,
+				(exchange, s) -> Mono.just(s == null ? "emptybody" : s));
+
+		Route route = routeBuilder.build();
+		assertThat(route.getFilters()).hasSize(1);
+
+		assertFilter(route.getFilters().get(0),
+				ModifyResponseBodyGatewayFilterFactory.ModifyResponseGatewayFilter.class,
+				NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER - 1);
+	}
+
+	@Test
+	public void shouldSetModifyBodyResponseFilterWithRewriteFunctionAndNewContentType() {
+		ConfigurableApplicationContext context = mock(
+				ConfigurableApplicationContext.class);
+		Route.AsyncBuilder routeBuilder = Route.async().id("123").uri("abc:123")
+				.predicate(exchange -> true);
+
+		when(context.getBean(ModifyResponseBodyGatewayFilterFactory.class))
+				.thenReturn(new ModifyResponseBodyGatewayFilterFactory());
+
+		RouteLocatorBuilder.Builder routes = new RouteLocatorBuilder(context).routes();
+		GatewayFilterSpec spec = new GatewayFilterSpec(routeBuilder, routes);
+		spec.modifyResponseBody(String.class, String.class,
+				MediaType.APPLICATION_JSON_VALUE, (exchange, s) -> Mono.just(s));
+
+		Route route = routeBuilder.build();
+		assertThat(route.getFilters()).hasSize(1);
+
+		assertFilter(route.getFilters().get(0),
+				ModifyResponseBodyGatewayFilterFactory.ModifyResponseGatewayFilter.class,
+				NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER - 1);
+	}
+
+	@Test
+	public void shouldSetModifyBodyResponseFilterWithConfigConsumer() {
+		ConfigurableApplicationContext context = mock(
+				ConfigurableApplicationContext.class);
+		Route.AsyncBuilder routeBuilder = Route.async().id("123").uri("abc:123")
+				.predicate(exchange -> true);
+
+		when(context.getBean(ModifyResponseBodyGatewayFilterFactory.class))
+				.thenReturn(new ModifyResponseBodyGatewayFilterFactory());
+
+		RouteLocatorBuilder.Builder routes = new RouteLocatorBuilder(context).routes();
+		GatewayFilterSpec spec = new GatewayFilterSpec(routeBuilder, routes);
+		spec.modifyResponseBody(
+				(smth) -> new ModifyResponseBodyGatewayFilterFactory.Config()
+						.setRewriteFunction(String.class, String.class,
+								(exchange, s) -> Mono.just(s)));
+
+		Route route = routeBuilder.build();
+		assertThat(route.getFilters()).hasSize(1);
+
+		assertFilter(route.getFilters().get(0),
+				ModifyResponseBodyGatewayFilterFactory.ModifyResponseGatewayFilter.class,
+				NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER - 1);
+	}
+
 	protected static class MyOrderedFilter implements GatewayFilter, Ordered {
 
 		@Override
diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java
index fa2438bd..530c64e0 100644
--- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java
+++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/HttpBinCompatibleController.java
@@ -147,6 +147,12 @@ public class HttpBinCompatibleController {
 		return ResponseEntity.status(status).body("Failed with " + status);
 	}
 
+	@RequestMapping(path = "/post/empty", method = RequestMethod.POST,
+			produces = MediaType.APPLICATION_JSON_VALUE)
+	public Mono emptyResponse() {
+		return Mono.empty();
+	}
+
 	public Map getHeaders(ServerWebExchange exchange) {
 		return exchange.getRequest().getHeaders().toSingleValueMap();
 	}
diff --git a/spring-cloud-gateway-sample/src/main/java/org/springframework/cloud/gateway/sample/GatewaySampleApplication.java b/spring-cloud-gateway-sample/src/main/java/org/springframework/cloud/gateway/sample/GatewaySampleApplication.java
index 6e2bcf4b..cb0b194d 100644
--- a/spring-cloud-gateway-sample/src/main/java/org/springframework/cloud/gateway/sample/GatewaySampleApplication.java
+++ b/spring-cloud-gateway-sample/src/main/java/org/springframework/cloud/gateway/sample/GatewaySampleApplication.java
@@ -99,6 +99,31 @@ public class GatewaySampleApplication {
 									})
 					).uri(uri)
 				)
+				.route("rewrite_empty_response", r -> r.host("*.rewriteemptyresponse.org")
+					.filters(f -> f.prefixPath("/httpbin")
+							.addResponseHeader("X-TestHeader", "rewrite_empty_response")
+							.modifyResponseBody(String.class, String.class,
+									(exchange, s) -> {
+										if (s == null) {
+											return Mono.just("emptybody");
+										}
+										return Mono.just(s.toUpperCase());
+									})
+
+					).uri(uri)
+				)
+				.route("rewrite_response_fail_supplier", r -> r.host("*.rewriteresponsewithfailsupplier.org")
+					.filters(f -> f.prefixPath("/httpbin")
+							.addResponseHeader("X-TestHeader", "rewrite_response_fail_supplier")
+							.modifyResponseBody(String.class, String.class,
+									(exchange, s) -> {
+										if (s == null) {
+											return Mono.error(new IllegalArgumentException("this should not happen"));
+										}
+										return Mono.just(s.toUpperCase());
+									})
+					).uri(uri)
+				)
 				.route("rewrite_response_obj", r -> r.host("*.rewriteresponseobj.org")
 					.filters(f -> f.prefixPath("/httpbin")
 							.addResponseHeader("X-TestHeader", "rewrite_response_obj")
diff --git a/spring-cloud-gateway-sample/src/test/java/org/springframework/cloud/gateway/sample/GatewaySampleApplicationTests.java b/spring-cloud-gateway-sample/src/test/java/org/springframework/cloud/gateway/sample/GatewaySampleApplicationTests.java
index 1b0127b8..61099b56 100644
--- a/spring-cloud-gateway-sample/src/test/java/org/springframework/cloud/gateway/sample/GatewaySampleApplicationTests.java
+++ b/spring-cloud-gateway-sample/src/test/java/org/springframework/cloud/gateway/sample/GatewaySampleApplicationTests.java
@@ -130,6 +130,29 @@ public class GatewaySampleApplicationTests {
 						.containsEntry("DATA", "HELLO"));
 	}
 
+	@Test
+	@SuppressWarnings("unchecked")
+	public void rewriteResponseEmptyBodyToStringWorks() {
+		webClient.post().uri("/post/empty").header("Host", "www.rewriteemptyresponse.org")
+				.exchange().expectStatus().isOk().expectHeader()
+				.valueEquals("X-TestHeader", "rewrite_empty_response")
+				.expectBody(String.class)
+				.consumeWith(result -> assertThat(result.getResponseBody())
+						.isEqualTo("emptybody"));
+	}
+
+	@Test
+	@SuppressWarnings("unchecked")
+	public void emptyBodySupplierNotCalledWhenBodyPresent() {
+		webClient.post().uri("/post")
+				.header("Host", "www.rewriteresponsewithfailsupplier.org")
+				.bodyValue("hello").exchange().expectStatus().isOk().expectHeader()
+				.valueEquals("X-TestHeader", "rewrite_response_fail_supplier")
+				.expectBody(Map.class)
+				.consumeWith(result -> assertThat(result.getResponseBody())
+						.containsEntry("DATA", "HELLO"));
+	}
+
 	@Test
 	@SuppressWarnings("unchecked")
 	public void rewriteResponeBodyObjectWorks() {

From 28ae74b58b41e0f21cd01f2f24b66a024ddee593 Mon Sep 17 00:00:00 2001
From: Gaemi 
Date: Thu, 19 Sep 2019 22:37:43 +0900
Subject: [PATCH 09/10] Allows RewriteFunction to be applied when no request
 body.

Fixes gh-1309
---
 ...ModifyRequestBodyGatewayFilterFactory.java |  6 +++--
 ...yRequestBodyGatewayFilterFactoryTests.java | 22 +++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java
index 9286949c..128887b3 100644
--- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java
+++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactory.java
@@ -71,8 +71,10 @@ public class ModifyRequestBodyGatewayFilterFactory extends
 
 				// TODO: flux or mono
 				Mono modifiedBody = serverRequest.bodyToMono(inClass)
-						// .log("modify_request_mono", Level.INFO)
-						.flatMap(o -> config.rewriteFunction.apply(exchange, o));
+						.flatMap(originalBody -> config.getRewriteFunction()
+								.apply(exchange, originalBody))
+						.switchIfEmpty(Mono.defer(() -> (Mono) config.getRewriteFunction()
+								.apply(exchange, null)));
 
 				BodyInserter bodyInserter = BodyInserters.fromPublisher(modifiedBody,
 						config.getOutClass());
diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
index c9331df8..f52a7fad 100644
--- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
+++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
@@ -56,6 +56,17 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes
 				.isEqualTo("modifyrequest");
 	}
 
+	@Test
+	public void upstreamRequestBodyIsEmpty() {
+		testClient.post().uri("/post").header("Host", "www.modifyrequestbodyempty.org")
+				.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
+				.exchange().expectStatus().isEqualTo(HttpStatus.OK).expectBody()
+				.jsonPath("headers.Content-Type")
+				.isEqualTo(MediaType.APPLICATION_JSON_VALUE).jsonPath("data")
+				.isEqualTo("modifyrequest");
+	}
+
+
 	@EnableAutoConfiguration
 	@SpringBootConfiguration
 	@Import(DefaultTestConfig.class)
@@ -74,6 +85,17 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes
 										return Mono.just("modifyrequest");
 									}))
 							.uri(uri))
+					.route("test_modify_request_body_empty",
+							r -> r.order(-1).host("**.modifyrequestbodyempty.org")
+									.filters(f -> f.modifyRequestBody(String.class, String.class,
+											MediaType.APPLICATION_JSON_VALUE,
+											(serverWebExchange, body) -> {
+												if (body == null) {
+													return Mono.just("modifyrequest");
+												}
+												return Mono.just(body.toUpperCase());
+											}))
+									.uri(uri))
 					.build();
 		}
 

From 138ea51ec2238134ffc8f20e695e1b2a32ae1ec9 Mon Sep 17 00:00:00 2001
From: Spencer Gibb 
Date: Fri, 28 Feb 2020 01:35:00 -0500
Subject: [PATCH 10/10] formatting

---
 ...yRequestBodyGatewayFilterFactoryTests.java | 23 +++++++++----------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
index f52a7fad..230dc692 100644
--- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
+++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryTests.java
@@ -66,7 +66,6 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes
 				.isEqualTo("modifyrequest");
 	}
 
-
 	@EnableAutoConfiguration
 	@SpringBootConfiguration
 	@Import(DefaultTestConfig.class)
@@ -85,17 +84,17 @@ public class ModifyRequestBodyGatewayFilterFactoryTests extends BaseWebClientTes
 										return Mono.just("modifyrequest");
 									}))
 							.uri(uri))
-					.route("test_modify_request_body_empty",
-							r -> r.order(-1).host("**.modifyrequestbodyempty.org")
-									.filters(f -> f.modifyRequestBody(String.class, String.class,
-											MediaType.APPLICATION_JSON_VALUE,
-											(serverWebExchange, body) -> {
-												if (body == null) {
-													return Mono.just("modifyrequest");
-												}
-												return Mono.just(body.toUpperCase());
-											}))
-									.uri(uri))
+					.route("test_modify_request_body_empty", r -> r.order(-1)
+							.host("**.modifyrequestbodyempty.org")
+							.filters(f -> f.modifyRequestBody(String.class, String.class,
+									MediaType.APPLICATION_JSON_VALUE,
+									(serverWebExchange, body) -> {
+										if (body == null) {
+											return Mono.just("modifyrequest");
+										}
+										return Mono.just(body.toUpperCase());
+									}))
+							.uri(uri))
 					.build();
 		}