From a72bea7213686280a4e7bac946d72aa6bf7a457b Mon Sep 17 00:00:00 2001 From: Catalin Sanda Date: Mon, 22 Feb 2021 00:12:53 +0200 Subject: [PATCH] Reset the exchange on the fallback path in SpringCloudCircuitBreakerFilterFactory Fix #1957 --- ...pringCloudCircuitBreakerFilterFactory.java | 7 ++--- ...BreakerResilience4JFilterFactoryTests.java | 10 +++++++ .../SpringCloudCircuitBreakerTestConfig.java | 29 +++++++++++++++++++ .../test/HttpBinCompatibleController.java | 13 +++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java index 952001ad..6823e89f 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java @@ -45,7 +45,6 @@ import static org.springframework.cloud.gateway.support.GatewayToStringStyler.fi import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CIRCUITBREAKER_EXECUTION_EXCEPTION_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.containsEncodedParts; -import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.removeAlreadyRouted; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.reset; /** @@ -117,10 +116,8 @@ public abstract class SpringCloudCircuitBreakerFilterFactory exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, requestUrl); addExceptionDetails(t, exchange); - // Before we continue on remove the already routed attribute since the - // fallback may go back through the route handler if the fallback - // is to another route in the Gateway - removeAlreadyRouted(exchange); + // Reset the exchange + reset(exchange); ServerHttpRequest request = exchange.getRequest().mutate().uri(requestUrl).build(); return getDispatcherHandler().handle(exchange.mutate().request(request).build()); diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerResilience4JFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerResilience4JFilterFactoryTests.java index 9b0d015a..bb201e41 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerResilience4JFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerResilience4JFilterFactoryTests.java @@ -31,10 +31,12 @@ import org.springframework.cloud.gateway.test.BaseWebClientTests; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.BodyInserters; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.core.StringContains.containsString; @@ -85,6 +87,14 @@ public class SpringCloudCircuitBreakerResilience4JFilterFactoryTests assertThat(filter.toString()).contains("myname").contains("forward:/myfallback"); } + @Test + public void testHeadersAreClearedOnFallback() { + testClient.post().uri("/responseheaders/502").body(BodyInserters.fromFormData("name-1", "value-1")) + .header("Host", "www.circuitbreakerresetexchange.org").header("X-Test-Header-1", "value1") + .accept(MediaType.APPLICATION_JSON).exchange().expectStatus().isOk().expectHeader() + .doesNotExist("X-Test-Header-1").expectHeader().valueEquals("X-Test-Header-1-fallback", "value1"); + } + @EnableAutoConfiguration @SpringBootConfiguration @Import(BaseWebClientTests.DefaultTestConfig.class) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java index ee43bc93..ac65ee68 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java @@ -16,8 +16,11 @@ package org.springframework.cloud.gateway.filter.factory; +import java.util.AbstractMap; import java.util.Collections; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import reactor.core.publisher.Mono; @@ -31,6 +34,9 @@ import org.springframework.cloud.gateway.test.BaseWebClientTests; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient; 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.ResponseEntity; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @@ -76,6 +82,19 @@ public class SpringCloudCircuitBreakerTestConfig { return Collections.singletonMap("from", "statusCodeFallbackController"); } + @RequestMapping("/resetExchangeFallbackController") + public ResponseEntity> resetExchangeFallbackController(ServerWebExchange exchange) { + return ResponseEntity.status(HttpStatus.OK) + .headers((HttpHeaders) exchange.getRequest().getHeaders().entrySet().stream() + .filter(entry -> entry.getKey().startsWith("X-Test-")) + .map(entry -> new AbstractMap.SimpleEntry<>(entry.getKey() + "-fallback", entry.getValue())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, + (list1, list2) -> Stream.concat(list1.stream(), list2.stream()) + .collect(Collectors.toList()), + HttpHeaders::new))) + .body(Collections.singletonMap("from", "resetExchangeFallbackController")); + } + @Bean public RouteLocator circuitBreakerRouteLocator(RouteLocatorBuilder builder) { return builder.routes() @@ -100,6 +119,16 @@ public class SpringCloudCircuitBreakerTestConfig { .filters(f -> f.prefixPath("/httpbin") .circuitBreaker(config -> config.setName("stalling-command"))) .uri(uri)) + .route("circuitbreaker_fallback_test_reset_exchange", + r -> r.host("**.circuitbreakerresetexchange.org") + .filters(f -> f.circuitBreaker(config -> config.setName("fallbackcmd") + .setFallbackUri("forward:/resetExchangeFallbackController")) + .filter((exchange, chain) -> chain.filter(exchange) + .then(Mono.defer(() -> + !exchange.getResponse().isCommitted() ? + Mono.error(new Exception("Some Random Exception")) : + Mono.empty())))) + .uri(uri)) .build(); } 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 ffb3150f..f53ed868 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 @@ -24,6 +24,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.zip.GZIPOutputStream; import com.fasterxml.jackson.databind.ObjectMapper; @@ -150,6 +152,17 @@ public class HttpBinCompatibleController { return ResponseEntity.status(status).body("Failed with " + status); } + @RequestMapping(value = "/responseheaders/{status}", method = { RequestMethod.GET, RequestMethod.POST }) + public ResponseEntity> responseHeaders(@PathVariable int status, ServerWebExchange exchange) { + HttpHeaders httpHeaders = exchange.getRequest().getHeaders().entrySet().stream() + .filter(entry -> entry.getKey().startsWith("X-Test-")) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, + (list1, list2) -> Stream.concat(list1.stream(), list2.stream()).collect(Collectors.toList()), + HttpHeaders::new)); + + return ResponseEntity.status(status).headers(httpHeaders).body(Collections.singletonMap("status", status)); + } + @RequestMapping(path = "/post/empty", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_VALUE) public Mono emptyResponse() { return Mono.empty();