From 0962dbf2c0d6d1729a6600b04c9547543d710222 Mon Sep 17 00:00:00 2001 From: Florin Pastorel Jurcovici Date: Wed, 4 Dec 2019 18:54:35 +0200 Subject: [PATCH] Fixes handling of non-standard status in decorated responses. Fixes gh-1450 --- .../gateway/filter/NettyRoutingFilter.java | 41 ++++++----- .../NettyRoutingFilterIntegrationTests.java | 69 ++++++++++++++++++- .../test/resources/application-logging.yml | 1 + 3 files changed, 94 insertions(+), 17 deletions(-) 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 8ad99de6..271c9000 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 @@ -43,6 +43,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.AbstractServerHttpResponse; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.http.server.reactive.ServerHttpResponseDecorator; import org.springframework.util.StringUtils; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; @@ -163,22 +164,7 @@ public class NettyRoutingFilter implements GlobalFilter, Ordered { contentTypeValue); } - HttpStatus status = HttpStatus.resolve(res.status().code()); - if (status != null) { - response.setStatusCode(status); - } - else if (response instanceof AbstractServerHttpResponse) { - // https://jira.spring.io/browse/SPR-16748 - ((AbstractServerHttpResponse) response) - .setStatusCodeValue(res.status().code()); - } - else { - // TODO: log warning here, not throw error? - throw new IllegalStateException( - "Unable to set status code on response: " - + res.status().code() + ", " - + response.getClass()); - } + setResponseStatus(res, response); // make sure headers filters run after setting status so it is // available in response @@ -217,6 +203,29 @@ public class NettyRoutingFilter implements GlobalFilter, Ordered { return responseFlux.then(chain.filter(exchange)); } + private void setResponseStatus(HttpClientResponse clientResponse, + ServerHttpResponse response) { + HttpStatus status = HttpStatus.resolve(clientResponse.status().code()); + if (status != null) { + response.setStatusCode(status); + } + else { + while (response instanceof ServerHttpResponseDecorator) { + response = ((ServerHttpResponseDecorator) response).getDelegate(); + } + if (response instanceof AbstractServerHttpResponse) { + ((AbstractServerHttpResponse) response) + .setStatusCodeValue(clientResponse.status().code()); + } + else { + // TODO: log warning here, not throw error? + throw new IllegalStateException("Unable to set status code " + + clientResponse.status().code() + " on response of type " + + response.getClass().getName()); + } + } + } + private HttpClient httpClientWithTimeoutFrom(Route route) { Integer connectTimeout = (Integer) route.getMetadata().get(CONNECT_TIMEOUT_ATTR); if (connectTimeout != null) { diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java index 1bdf622f..7e804a73 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java @@ -18,16 +18,24 @@ package org.springframework.cloud.gateway.filter; import org.junit.Test; import org.junit.runner.RunWith; +import reactor.core.publisher.Mono; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.gateway.test.BaseWebClientTests; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.http.server.reactive.ServerHttpResponseDecorator; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.reactive.server.WebTestClient; +import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.data.Offset.offset; @@ -38,9 +46,11 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen @SpringBootTest(properties = "spring.cloud.gateway.httpclient.response-timeout=3s", webEnvironment = RANDOM_PORT) @DirtiesContext -@SuppressWarnings("unchecked") public class NettyRoutingFilterIntegrationTests extends BaseWebClientTests { + @Autowired + private ResponseDecoratingFilter responseDecorator; + @Test public void responseTimeoutWorks() { testClient.get().uri("/delay/5").exchange().expectStatus() @@ -62,6 +72,33 @@ public class NettyRoutingFilterIntegrationTests extends BaseWebClientTests { .isEqualTo("localhost:" + port); } + @Test + public void canHandleDecoratedResponseWithNonStandardStatusValue() { + final int NON_STANDARD_STATUS = 480; + responseDecorator.decorateResponseTimes(1); + testClient.mutate().baseUrl("http://localhost:" + port).build().get() + .uri("/status/" + NON_STANDARD_STATUS).exchange().expectStatus() + .isEqualTo(NON_STANDARD_STATUS); + } + + @Test + public void canHandleUndecoratedResponseWithNonStandardStatusValue() { + final int NON_STANDARD_STATUS = 480; + responseDecorator.decorateResponseTimes(0); + testClient.mutate().baseUrl("http://localhost:" + port).build().get() + .uri("/status/" + NON_STANDARD_STATUS).exchange().expectStatus() + .isEqualTo(NON_STANDARD_STATUS); + } + + @Test + public void canHandleMultiplyDecoratedResponseWithNonStandardStatusValue() { + final int NON_STANDARD_STATUS = 142; + responseDecorator.decorateResponseTimes(14); + testClient.mutate().baseUrl("http://localhost:" + port).build().get() + .uri("/status/" + NON_STANDARD_STATUS).exchange().expectStatus() + .isEqualTo(NON_STANDARD_STATUS); + } + @Test public void shouldApplyConnectTimeoutPerRoute() { long currentTimeMillisBeforeCall = System.currentTimeMillis(); @@ -97,6 +134,36 @@ public class NettyRoutingFilterIntegrationTests extends BaseWebClientTests { @Import(DefaultTestConfig.class) public static class TestConfig { + @Bean + @Order(RouteToRequestUrlFilter.HIGHEST_PRECEDENCE) + public ResponseDecoratingFilter decoratingFilter() { + return new ResponseDecoratingFilter(); + } + + } + + public static final class ResponseDecoratingFilter implements GlobalFilter, Ordered { + + int decorationIterations = 1; + + public void decorateResponseTimes(int times) { + decorationIterations = times; + } + + @Override + public int getOrder() { + return RouteToRequestUrlFilter.HIGHEST_PRECEDENCE; + } + + @Override + public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { + ServerHttpResponse decorator = exchange.getResponse(); + for (int counter = 0; counter < decorationIterations; counter++) { + decorator = new ServerHttpResponseDecorator(decorator); + } + return chain.filter(exchange.mutate().response(decorator).build()); + } + } } diff --git a/spring-cloud-gateway-core/src/test/resources/application-logging.yml b/spring-cloud-gateway-core/src/test/resources/application-logging.yml index c07e955a..4bda232b 100644 --- a/spring-cloud-gateway-core/src/test/resources/application-logging.yml +++ b/spring-cloud-gateway-core/src/test/resources/application-logging.yml @@ -4,5 +4,6 @@ logging: org.springframework.http.server.reactive: DEBUG org.springframework.web.reactive: DEBUG org.springframework.boot.autoconfigure.web: DEBUG + org.springframework.cloud.gateway.actuate: DEBUG reactor.netty: DEBUG redisratelimiter: DEBUG \ No newline at end of file