diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/AdaptCachedBodyGlobalFilter.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/AdaptCachedBodyGlobalFilter.java index 1ed0da77..08ec58a7 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/AdaptCachedBodyGlobalFilter.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/AdaptCachedBodyGlobalFilter.java @@ -70,18 +70,13 @@ public class AdaptCachedBodyGlobalFilter return chain.filter(exchange); } - final Mono requestProcessingFinishedFlag = Mono - .just("requestProcessingFinishedFlag"); - return ServerWebExchangeUtils - .cacheRequestBody(exchange, (serverHttpRequest) -> chain - .filter(exchange.mutate().request(serverHttpRequest).build()) - // suppress empty mono response from FilteringWebHandler, see - // https://github.com/spring-cloud/spring-cloud-gateway/issues/1315 - .then(requestProcessingFinishedFlag)) - // when request body is empty - we return from cacheRequestBody() with - // empty mono - so we will process that request - .switchIfEmpty(chain.filter(exchange).then(requestProcessingFinishedFlag)) - .then(); + return ServerWebExchangeUtils.cacheRequestBody(exchange, (serverHttpRequest) -> { + // don't mutate and build if same request object + if (serverHttpRequest == exchange.getRequest()) { + return chain.filter(exchange); + } + return chain.filter(exchange.mutate().request(serverHttpRequest).build()); + }); } @Override diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java index 821fe805..f4114230 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java @@ -322,7 +322,8 @@ public final class ServerWebExchangeUtils { * @param exchange the available ServerWebExchange. * @param cacheDecoratedRequest if true, the ServerHttpRequestDecorator will be * cached. - * @param function a function that accepts the created ServerHttpRequestDecorator. + * @param function a function that accepts a ServerHttpRequest. It can be the created + * ServerHttpRequestDecorator or the originial if there is no body. * @param generic type for the return {@link Mono}. * @return Mono of type T created by the function parameter. */ @@ -330,39 +331,37 @@ public final class ServerWebExchangeUtils { boolean cacheDecoratedRequest, Function> function) { // Join all the DataBuffers so we have a single DataBuffer for the body - return DataBufferUtils.join(exchange.getRequest().getBody()) - .flatMap(dataBuffer -> { - if (dataBuffer.readableByteCount() > 0) { - if (log.isTraceEnabled()) { - log.trace("retaining body in exchange attribute"); - } - exchange.getAttributes().put(CACHED_REQUEST_BODY_ATTR, - dataBuffer); - } + return DataBufferUtils.join(exchange.getRequest().getBody()).map(dataBuffer -> { + if (dataBuffer.readableByteCount() > 0) { + if (log.isTraceEnabled()) { + log.trace("retaining body in exchange attribute"); + } + exchange.getAttributes().put(CACHED_REQUEST_BODY_ATTR, dataBuffer); + } - ServerHttpRequestDecorator decorator = new ServerHttpRequestDecorator( - exchange.getRequest()) { - @Override - public Flux getBody() { - return Mono.fromSupplier(() -> { - if (exchange.getAttributeOrDefault( - CACHED_REQUEST_BODY_ATTR, null) == null) { - // probably == downstream closed - return null; - } - // TODO: deal with Netty - NettyDataBuffer pdb = (NettyDataBuffer) dataBuffer; - return pdb.factory() - .wrap(pdb.getNativeBuffer().retainedSlice()); - }).flux(); + ServerHttpRequest decorator = new ServerHttpRequestDecorator( + exchange.getRequest()) { + @Override + public Flux getBody() { + return Mono.fromSupplier(() -> { + if (exchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, + null) == null) { + // probably == downstream closed + return null; } - }; - if (cacheDecoratedRequest) { - exchange.getAttributes().put( - CACHED_SERVER_HTTP_REQUEST_DECORATOR_ATTR, decorator); - } - return function.apply(decorator); - }); + // TODO: deal with Netty + NettyDataBuffer pdb = (NettyDataBuffer) dataBuffer; + return pdb.factory().wrap(pdb.getNativeBuffer().retainedSlice()); + }).flux(); + } + }; + if (cacheDecoratedRequest) { + exchange.getAttributes().put(CACHED_SERVER_HTTP_REQUEST_DECORATOR_ATTR, + decorator); + } + return decorator; + // return function.apply(decorator)/*.then(monoVoid())*/; + }).switchIfEmpty(Mono.just(exchange.getRequest())).flatMap(function); } } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java index 0fbd0add..12934c4d 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java @@ -70,7 +70,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen // default filter AddResponseHeader suppresses bug // https://github.com/spring-cloud/spring-cloud-gateway/issues/1315, // so we use only PrefixPath filter -@ActiveProfiles("only-prefix-filter") +@ActiveProfiles("retrytests") public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTests { @Rule @@ -135,7 +135,7 @@ public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTest public void retryFilterPostOneTime() { testClient.post().uri( "/retrypost?key=retryFilterPostOneTime&expectedbody=HelloGateway&count=1") - .header(HttpHeaders.HOST, "www.retrypostconfig.org") + .header(HttpHeaders.HOST, "www.retrypostonceconfig.org") .syncBody("HelloGateway").exchange().expectStatus().isOk(); assertThat(this.capture.toString()).contains("setting new iteration in attr 0"); assertThat(this.capture.toString()) @@ -220,7 +220,7 @@ public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTest } @RequestMapping("/httpbin/retrypost") - public ResponseEntity retry(@RequestParam("key") String key, + public ResponseEntity retrypost(@RequestParam("key") String key, @RequestParam(name = "count", defaultValue = "3") int count, @RequestParam("expectedbody") String expectedbody, @RequestBody String body) { diff --git a/spring-cloud-gateway-core/src/test/resources/application-only-prefix-filter.yml b/spring-cloud-gateway-core/src/test/resources/application-only-prefix-filter.yml deleted file mode 100644 index 70790b9e..00000000 --- a/spring-cloud-gateway-core/src/test/resources/application-only-prefix-filter.yml +++ /dev/null @@ -1,5 +0,0 @@ -spring: - cloud: - gateway: - default-filters: - - PrefixPath=/httpbin diff --git a/spring-cloud-gateway-core/src/test/resources/application-retrytests.yml b/spring-cloud-gateway-core/src/test/resources/application-retrytests.yml new file mode 100644 index 00000000..ea5e1be8 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/resources/application-retrytests.yml @@ -0,0 +1,31 @@ +spring: + cloud: + gateway: + default-filters: + - PrefixPath=/httpbin + routes: + # ===================================== + - id: retry_test + uri: ${test.uri} + predicates: + - Path=/retry + filters: + - Retry + # ===================================== + - id: retry_post_once_test + uri: ${test.uri} + predicates: + - Host=**.retrypostonceconfig.org + filters: + - name: Retry + args: + methods: GET,POST + # ===================================== + - id: retry_post_test + uri: ${test.uri} + predicates: + - Host=**.retrypostconfig.org + filters: + - name: Retry + args: + methods: GET,POST diff --git a/spring-cloud-gateway-core/src/test/resources/application.yml b/spring-cloud-gateway-core/src/test/resources/application.yml index c7a25b73..d1c7f30e 100644 --- a/spring-cloud-gateway-core/src/test/resources/application.yml +++ b/spring-cloud-gateway-core/src/test/resources/application.yml @@ -253,24 +253,6 @@ spring: - AddResponseHeader=X-Request-Foo, Bar - RemoveResponseHeader=X-Request-Foo - # ===================================== - - id: retry_test - uri: ${test.uri} - predicates: - - Path=/retry - filters: - - Retry - - # ===================================== - - id: retry_post_test - uri: ${test.uri} - predicates: - - Host=**.retrypostconfig.org - filters: - - name: Retry - args: - methods: GET,POST - # ===================================== - id: secure_headers_test uri: ${test.uri}