Skip to content

Commit 41a238a

Browse files
author
chenwenshun
committed
Fix isCommitted() check in response header filters
Add isCommitted() check to prevent UnsupportedOperationException when modifying response headers after request is rejected by filters like RequestRateLimiter. Affected filters: - SetResponseHeaderGatewayFilterFactory - DedupeResponseHeaderGatewayFilterFactory - SecureHeadersGatewayFilterFactory - RewriteResponseHeaderGatewayFilterFactory - RewriteLocationResponseHeaderGatewayFilterFactory Fixes gh-3718 Signed-off-by: chenwenshun <chenwenshun@gmail.com>
1 parent 7020613 commit 41a238a

File tree

6 files changed

+211
-9
lines changed

6 files changed

+211
-9
lines changed

‎spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/DedupeResponseHeaderGatewayFilterFactory.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,11 @@ public GatewayFilter apply(Config config) {
9292
return new GatewayFilter() {
9393
@Override
9494
public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
95-
return chain.filter(exchange)
96-
.then(Mono.fromRunnable(() -> dedupe(exchange.getResponse().getHeaders(), config)));
95+
return chain.filter(exchange).then(Mono.fromRunnable(() -> {
96+
if (!exchange.getResponse().isCommitted()) {
97+
dedupe(exchange.getResponse().getHeaders(), config);
98+
}
99+
}));
97100
}
98101

99102
@Override

‎spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteLocationResponseHeaderGatewayFilterFactory.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,11 @@ public GatewayFilter apply(Config config) {
135135
return new GatewayFilter() {
136136
@Override
137137
public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
138-
return chain.filter(exchange).then(Mono.fromRunnable(() -> rewriteLocation(exchange, config)));
138+
return chain.filter(exchange).then(Mono.fromRunnable(() -> {
139+
if (!exchange.getResponse().isCommitted()) {
140+
rewriteLocation(exchange, config);
141+
}
142+
}));
139143
}
140144

141145
@Override

‎spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteResponseHeaderGatewayFilterFactory.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ public GatewayFilter apply(Config config) {
6161
return new GatewayFilter() {
6262
@Override
6363
public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
64-
return chain.filter(exchange).then(Mono.fromRunnable(() -> rewriteHeaders(exchange, config)));
64+
return chain.filter(exchange).then(Mono.fromRunnable(() -> {
65+
if (!exchange.getResponse().isCommitted()) {
66+
rewriteHeaders(exchange, config);
67+
}
68+
}));
6569
}
6670

6771
@Override

‎spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SecureHeadersGatewayFilterFactory.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
106106
Set<String> headersToAddToResponse = assembleHeaders(originalConfig, properties);
107107

108108
Config config = originalConfig.withDefaults(properties);
109-
return chain.filter(exchange)
110-
.then(Mono
111-
.fromRunnable(() -> applySecurityHeaders(responseHeaders, headersToAddToResponse, config)));
109+
return chain.filter(exchange).then(Mono.fromRunnable(() -> {
110+
if (!exchange.getResponse().isCommitted()) {
111+
applySecurityHeaders(responseHeaders, headersToAddToResponse, config);
112+
}
113+
}));
112114
}
113115

114116
@Override

‎spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/SetResponseHeaderGatewayFilterFactory.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@ public GatewayFilter apply(NameValueConfig config) {
3939
public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
4040
String value = ServerWebExchangeUtils.expand(exchange, config.getValue());
4141
String name = Objects.requireNonNull(config.name, "name must not be null");
42-
return chain.filter(exchange)
43-
.then(Mono.fromRunnable(() -> exchange.getResponse().getHeaders().set(name, value)));
42+
return chain.filter(exchange).then(Mono.fromRunnable(() -> {
43+
if (!exchange.getResponse().isCommitted()) {
44+
exchange.getResponse().getHeaders().set(name, value);
45+
}
46+
}));
4447
}
4548

4649
@Override

‎spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/ResponseHeaderCommittedTests.java‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
* Copyright 2013-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.gateway.filter.factory;
18+
19+
import org.junit.jupiter.api.Test;
20+
import reactor.core.publisher.Mono;
21+
22+
import org.springframework.beans.factory.annotation.Value;
23+
import org.springframework.boot.SpringBootConfiguration;
24+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
25+
import org.springframework.boot.test.context.SpringBootTest;
26+
import org.springframework.cloud.gateway.filter.GatewayFilter;
27+
import org.springframework.cloud.gateway.filter.GatewayFilterChain;
28+
import org.springframework.cloud.gateway.route.RouteLocator;
29+
import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder;
30+
import org.springframework.cloud.gateway.test.BaseWebClientTests;
31+
import org.springframework.context.annotation.Bean;
32+
import org.springframework.context.annotation.Import;
33+
import org.springframework.http.HttpStatus;
34+
import org.springframework.test.annotation.DirtiesContext;
35+
import org.springframework.web.server.ServerWebExchange;
36+
37+
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
38+
39+
/**
40+
* Tests for response header filters when response is already committed. This verifies the
41+
* fix for issue #3718 where filters like RequestRateLimiter commit the response, but
42+
* post-processing response header filters still try to modify headers.
43+
*
44+
* @author issue #3718
45+
*/
46+
@SpringBootTest(webEnvironment = RANDOM_PORT)
47+
@DirtiesContext
48+
class ResponseHeaderCommittedTests extends BaseWebClientTests {
49+
50+
@Test
51+
void setResponseHeaderShouldNotThrowWhenResponseCommitted() {
52+
testClient.get()
53+
.uri("/committed")
54+
.header("Host", "www.set-committed.org")
55+
.exchange()
56+
.expectStatus()
57+
.isEqualTo(HttpStatus.FORBIDDEN)
58+
.expectHeader()
59+
.doesNotExist("X-Custom-Header");
60+
}
61+
62+
@Test
63+
void removeResponseHeaderShouldNotThrowWhenResponseCommitted() {
64+
testClient.get()
65+
.uri("/committed")
66+
.header("Host", "www.remove-committed.org")
67+
.exchange()
68+
.expectStatus()
69+
.isEqualTo(HttpStatus.FORBIDDEN);
70+
}
71+
72+
@Test
73+
void dedupeResponseHeaderShouldNotThrowWhenResponseCommitted() {
74+
testClient.get()
75+
.uri("/committed")
76+
.header("Host", "www.dedupe-committed.org")
77+
.exchange()
78+
.expectStatus()
79+
.isEqualTo(HttpStatus.FORBIDDEN);
80+
}
81+
82+
@Test
83+
void rewriteResponseHeaderShouldNotThrowWhenResponseCommitted() {
84+
testClient.get()
85+
.uri("/committed")
86+
.header("Host", "www.rewrite-committed.org")
87+
.exchange()
88+
.expectStatus()
89+
.isEqualTo(HttpStatus.FORBIDDEN);
90+
}
91+
92+
@Test
93+
void rewriteLocationResponseHeaderShouldNotThrowWhenResponseCommitted() {
94+
testClient.get()
95+
.uri("/committed")
96+
.header("Host", "www.rewritelocation-committed.org")
97+
.exchange()
98+
.expectStatus()
99+
.isEqualTo(HttpStatus.FORBIDDEN);
100+
}
101+
102+
@Test
103+
void secureHeadersShouldNotThrowWhenResponseCommitted() {
104+
testClient.get()
105+
.uri("/committed")
106+
.header("Host", "www.secureheaders-committed.org")
107+
.exchange()
108+
.expectStatus()
109+
.isEqualTo(HttpStatus.FORBIDDEN);
110+
}
111+
112+
/**
113+
* A filter that commits the response without calling chain.filter(), simulating
114+
* behavior of filters like RequestRateLimiter when denying a request.
115+
*/
116+
static class CommitResponseFilter implements GatewayFilter {
117+
118+
@Override
119+
public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
120+
exchange.getResponse().setStatusCode(HttpStatus.FORBIDDEN);
121+
return exchange.getResponse().setComplete();
122+
}
123+
124+
}
125+
126+
@EnableAutoConfiguration
127+
@SpringBootConfiguration
128+
@Import(DefaultTestConfig.class)
129+
static class TestConfig {
130+
131+
@Value("${test.uri}")
132+
String uri;
133+
134+
@Bean
135+
public RouteLocator testRouteLocator(RouteLocatorBuilder builder) {
136+
return builder.routes()
137+
// SetResponseHeader test
138+
.route("set_response_header_committed_test", r -> r.path("/committed")
139+
.and()
140+
.host("www.set-committed.org")
141+
.filters(f -> f.filter(new CommitResponseFilter()).setResponseHeader("X-Custom-Header", "value"))
142+
.uri(uri))
143+
// RemoveResponseHeader test
144+
.route("remove_response_header_committed_test",
145+
r -> r.path("/committed")
146+
.and()
147+
.host("www.remove-committed.org")
148+
.filters(f -> f.filter(new CommitResponseFilter()).removeResponseHeader("X-Custom-Header"))
149+
.uri(uri))
150+
// DedupeResponseHeader test
151+
.route("dedupe_response_header_committed_test",
152+
r -> r.path("/committed")
153+
.and()
154+
.host("www.dedupe-committed.org")
155+
.filters(f -> f.filter(new CommitResponseFilter())
156+
.dedupeResponseHeader("X-Custom-Header", "RETAIN_FIRST"))
157+
.uri(uri))
158+
// RewriteResponseHeader test
159+
.route("rewrite_response_header_committed_test",
160+
r -> r.path("/committed")
161+
.and()
162+
.host("www.rewrite-committed.org")
163+
.filters(f -> f.filter(new CommitResponseFilter())
164+
.rewriteResponseHeader("X-Custom-Header", "pattern", "replacement"))
165+
.uri(uri))
166+
// RewriteLocationResponseHeader test
167+
.route("rewrite_location_response_header_committed_test",
168+
r -> r.path("/committed")
169+
.and()
170+
.host("www.rewritelocation-committed.org")
171+
.filters(f -> f.filter(new CommitResponseFilter())
172+
.rewriteLocationResponseHeader("AS_IN_REQUEST", "Location", null, null))
173+
.uri(uri))
174+
// SecureHeaders test
175+
.route("secure_headers_committed_test",
176+
r -> r.path("/committed")
177+
.and()
178+
.host("www.secureheaders-committed.org")
179+
.filters(f -> f.filter(new CommitResponseFilter()).secureHeaders())
180+
.uri(uri))
181+
.build();
182+
}
183+
184+
}
185+
186+
}

0 commit comments

Comments
 (0)