diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/DedupeResponseHeaderGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/DedupeResponseHeaderGatewayFilterFactory.java index 2594dbefe0..68d2d27542 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/DedupeResponseHeaderGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/DedupeResponseHeaderGatewayFilterFactory.java @@ -92,8 +92,11 @@ public GatewayFilter apply(Config config) { return new GatewayFilter() { @Override public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { - return chain.filter(exchange) - .then(Mono.fromRunnable(() -> dedupe(exchange.getResponse().getHeaders(), config))); + return chain.filter(exchange).then(Mono.fromRunnable(() -> { + if (!exchange.getResponse().isCommitted()) { + dedupe(exchange.getResponse().getHeaders(), config); + } + })); } @Override diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteLocationResponseHeaderGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteLocationResponseHeaderGatewayFilterFactory.java index e3ce9e7d8a..439a9ae8e9 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteLocationResponseHeaderGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteLocationResponseHeaderGatewayFilterFactory.java @@ -135,7 +135,11 @@ public GatewayFilter apply(Config config) { return new GatewayFilter() { @Override public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { - return chain.filter(exchange).then(Mono.fromRunnable(() -> rewriteLocation(exchange, config))); + return chain.filter(exchange).then(Mono.fromRunnable(() -> { + if (!exchange.getResponse().isCommitted()) { + rewriteLocation(exchange, config); + } + })); } @Override diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteResponseHeaderGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteResponseHeaderGatewayFilterFactory.java index f7f62d901a..a683b796a9 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteResponseHeaderGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteResponseHeaderGatewayFilterFactory.java @@ -61,7 +61,11 @@ public GatewayFilter apply(Config config) { return new GatewayFilter() { @Override public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { - return chain.filter(exchange).then(Mono.fromRunnable(() -> rewriteHeaders(exchange, config))); + return chain.filter(exchange).then(Mono.fromRunnable(() -> { + if (!exchange.getResponse().isCommitted()) { + rewriteHeaders(exchange, config); + } + })); } @Override diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SecureHeadersGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SecureHeadersGatewayFilterFactory.java index 40ba973c67..c74bdc0d45 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SecureHeadersGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SecureHeadersGatewayFilterFactory.java @@ -106,9 +106,11 @@ public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { Set headersToAddToResponse = assembleHeaders(originalConfig, properties); Config config = originalConfig.withDefaults(properties); - return chain.filter(exchange) - .then(Mono - .fromRunnable(() -> applySecurityHeaders(responseHeaders, headersToAddToResponse, config))); + return chain.filter(exchange).then(Mono.fromRunnable(() -> { + if (!exchange.getResponse().isCommitted()) { + applySecurityHeaders(responseHeaders, headersToAddToResponse, config); + } + })); } @Override diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SetResponseHeaderGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SetResponseHeaderGatewayFilterFactory.java index 01c401e601..77e3052b6e 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SetResponseHeaderGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SetResponseHeaderGatewayFilterFactory.java @@ -39,8 +39,11 @@ public GatewayFilter apply(NameValueConfig config) { public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { String value = ServerWebExchangeUtils.expand(exchange, config.getValue()); String name = Objects.requireNonNull(config.name, "name must not be null"); - return chain.filter(exchange) - .then(Mono.fromRunnable(() -> exchange.getResponse().getHeaders().set(name, value))); + return chain.filter(exchange).then(Mono.fromRunnable(() -> { + if (!exchange.getResponse().isCommitted()) { + exchange.getResponse().getHeaders().set(name, value); + } + })); } @Override diff --git a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/ResponseHeaderCommittedTests.java b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/ResponseHeaderCommittedTests.java new file mode 100644 index 0000000000..82e50c0e17 --- /dev/null +++ b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/ResponseHeaderCommittedTests.java @@ -0,0 +1,186 @@ +/* + * Copyright 2013-present 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; + +import org.junit.jupiter.api.Test; +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.filter.GatewayFilter; +import org.springframework.cloud.gateway.filter.GatewayFilterChain; +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.HttpStatus; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.web.server.ServerWebExchange; + +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * Tests for response header filters when response is already committed. This verifies the + * fix for issue #3718 where filters like RequestRateLimiter commit the response, but + * post-processing response header filters still try to modify headers. + * + * @author issue #3718 + */ +@SpringBootTest(webEnvironment = RANDOM_PORT) +@DirtiesContext +class ResponseHeaderCommittedTests extends BaseWebClientTests { + + @Test + void setResponseHeaderShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.set-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN) + .expectHeader() + .doesNotExist("X-Custom-Header"); + } + + @Test + void removeResponseHeaderShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.remove-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void dedupeResponseHeaderShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.dedupe-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void rewriteResponseHeaderShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.rewrite-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void rewriteLocationResponseHeaderShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.rewritelocation-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void secureHeadersShouldNotThrowWhenResponseCommitted() { + testClient.get() + .uri("/committed") + .header("Host", "www.secureheaders-committed.org") + .exchange() + .expectStatus() + .isEqualTo(HttpStatus.FORBIDDEN); + } + + /** + * A filter that commits the response without calling chain.filter(), simulating + * behavior of filters like RequestRateLimiter when denying a request. + */ + static class CommitResponseFilter implements GatewayFilter { + + @Override + public Mono filter(ServerWebExchange exchange, GatewayFilterChain chain) { + exchange.getResponse().setStatusCode(HttpStatus.FORBIDDEN); + return exchange.getResponse().setComplete(); + } + + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + static class TestConfig { + + @Value("${test.uri}") + String uri; + + @Bean + public RouteLocator testRouteLocator(RouteLocatorBuilder builder) { + return builder.routes() + // SetResponseHeader test + .route("set_response_header_committed_test", r -> r.path("/committed") + .and() + .host("www.set-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()).setResponseHeader("X-Custom-Header", "value")) + .uri(uri)) + // RemoveResponseHeader test + .route("remove_response_header_committed_test", + r -> r.path("/committed") + .and() + .host("www.remove-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()).removeResponseHeader("X-Custom-Header")) + .uri(uri)) + // DedupeResponseHeader test + .route("dedupe_response_header_committed_test", + r -> r.path("/committed") + .and() + .host("www.dedupe-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()) + .dedupeResponseHeader("X-Custom-Header", "RETAIN_FIRST")) + .uri(uri)) + // RewriteResponseHeader test + .route("rewrite_response_header_committed_test", + r -> r.path("/committed") + .and() + .host("www.rewrite-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()) + .rewriteResponseHeader("X-Custom-Header", "pattern", "replacement")) + .uri(uri)) + // RewriteLocationResponseHeader test + .route("rewrite_location_response_header_committed_test", + r -> r.path("/committed") + .and() + .host("www.rewritelocation-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()) + .rewriteLocationResponseHeader("AS_IN_REQUEST", "Location", null, null)) + .uri(uri)) + // SecureHeaders test + .route("secure_headers_committed_test", + r -> r.path("/committed") + .and() + .host("www.secureheaders-committed.org") + .filters(f -> f.filter(new CommitResponseFilter()).secureHeaders()) + .uri(uri)) + .build(); + } + + } + +}