Skip to content

Commit 51aa244

Browse files
ryanjbaxterryanjbaxter
authored
Merge pull request #3598 from raccoonback/prevent-duplicated-encoding-request-parameters-filter
Fix: prevent duplicated encoding request parameters filter
2 parents 12ace1c + b158666 commit 51aa244

File tree

4 files changed

+88
-19
lines changed

4 files changed

+88
-19
lines changed

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.util.MultiValueMap;
3030
import org.springframework.web.server.ServerWebExchange;
3131
import org.springframework.web.util.UriComponentsBuilder;
32+
import org.springframework.web.util.UriUtils;
3233

3334
import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator;
3435
import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap;
@@ -57,14 +58,19 @@ public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
5758
MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>(request.getQueryParams());
5859
queryParams.remove(config.getName());
5960

60-
URI newUri = UriComponentsBuilder.fromUri(request.getURI())
61-
.replaceQueryParams(unmodifiableMultiValueMap(queryParams))
62-
.build()
63-
.toUri();
61+
try {
62+
MultiValueMap<String, String> encodedQueryParams = UriUtils.encodeQueryParams(queryParams);
63+
URI newUri = UriComponentsBuilder.fromUri(request.getURI())
64+
.replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams))
65+
.build(true)
66+
.toUri();
6467

65-
ServerHttpRequest updatedRequest = exchange.getRequest().mutate().uri(newUri).build();
66-
67-
return chain.filter(exchange.mutate().request(updatedRequest).build());
68+
ServerHttpRequest updatedRequest = exchange.getRequest().mutate().uri(newUri).build();
69+
return chain.filter(exchange.mutate().request(updatedRequest).build());
70+
}
71+
catch (IllegalArgumentException ex) {
72+
throw new IllegalStateException("Invalid URI query: \"" + queryParams + "\"");
73+
}
6874
}
6975

7076
@Override

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,14 @@
2626
import org.springframework.cloud.gateway.filter.GatewayFilterChain;
2727
import org.springframework.http.server.reactive.ServerHttpRequest;
2828
import org.springframework.util.Assert;
29+
import org.springframework.util.LinkedMultiValueMap;
30+
import org.springframework.util.MultiValueMap;
2931
import org.springframework.web.server.ServerWebExchange;
3032
import org.springframework.web.util.UriComponentsBuilder;
33+
import org.springframework.web.util.UriUtils;
3134

3235
import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator;
36+
import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap;
3337

3438
/**
3539
* @author Fredrich Ombico
@@ -59,14 +63,25 @@ public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
5963
ServerHttpRequest req = exchange.getRequest();
6064

6165
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(req.getURI());
62-
if (req.getQueryParams().containsKey(config.getName())) {
63-
uriComponentsBuilder.replaceQueryParam(config.getName(), config.getReplacement());
66+
67+
MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>(req.getQueryParams());
68+
if (queryParams.containsKey(config.getName())) {
69+
queryParams.remove(config.getName());
70+
queryParams.add(config.getName(), config.getReplacement());
6471
}
6572

66-
URI uri = uriComponentsBuilder.build().toUri();
67-
ServerHttpRequest request = req.mutate().uri(uri).build();
73+
try {
74+
MultiValueMap<String, String> encodedQueryParams = UriUtils.encodeQueryParams(queryParams);
75+
URI uri = uriComponentsBuilder.replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams))
76+
.build(true)
77+
.toUri();
6878

69-
return chain.filter(exchange.mutate().request(request).build());
79+
ServerHttpRequest request = req.mutate().uri(uri).build();
80+
return chain.filter(exchange.mutate().request(request).build());
81+
}
82+
catch (IllegalArgumentException ex) {
83+
throw new IllegalStateException("Invalid URI query: \"" + queryParams + "\"");
84+
}
7085
}
7186

7287
@Override

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

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
/**
3838
* @author Thirunavukkarasu Ravichandran
3939
*/
40-
public class RemoveRequestParameterGatewayFilterFactoryTests {
40+
class RemoveRequestParameterGatewayFilterFactoryTests {
4141

4242
private ServerWebExchange exchange;
4343

@@ -46,15 +46,15 @@ public class RemoveRequestParameterGatewayFilterFactoryTests {
4646
private ArgumentCaptor<ServerWebExchange> captor;
4747

4848
@BeforeEach
49-
public void setUp() {
49+
void setUp() {
5050
filterChain = mock(GatewayFilterChain.class);
5151
captor = ArgumentCaptor.forClass(ServerWebExchange.class);
5252
when(filterChain.filter(captor.capture())).thenReturn(Mono.empty());
5353

5454
}
5555

5656
@Test
57-
public void removeRequestParameterFilterWorks() {
57+
void removeRequestParameterFilterWorks() {
5858
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost")
5959
.queryParam("foo", singletonList("bar"))
6060
.build();
@@ -70,7 +70,7 @@ public void removeRequestParameterFilterWorks() {
7070
}
7171

7272
@Test
73-
public void removeRequestParameterFilterWorksWhenParamIsNotPresentInRequest() {
73+
void removeRequestParameterFilterWorksWhenParamIsNotPresentInRequest() {
7474
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost").build();
7575
exchange = MockServerWebExchange.from(request);
7676
NameConfig config = new NameConfig();
@@ -84,7 +84,7 @@ public void removeRequestParameterFilterWorksWhenParamIsNotPresentInRequest() {
8484
}
8585

8686
@Test
87-
public void removeRequestParameterFilterShouldOnlyRemoveSpecifiedParam() {
87+
void removeRequestParameterFilterShouldOnlyRemoveSpecifiedParam() {
8888
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost")
8989
.queryParam("foo", "bar")
9090
.queryParam("abc", "xyz")
@@ -102,7 +102,7 @@ public void removeRequestParameterFilterShouldOnlyRemoveSpecifiedParam() {
102102
}
103103

104104
@Test
105-
public void removeRequestParameterFilterShouldHandleRemainingParamsWhichRequiringEncoding() {
105+
void removeRequestParameterFilterShouldHandleRemainingParamsWhichRequiringEncoding() {
106106
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost")
107107
.queryParam("foo", "bar")
108108
.queryParam("aaa", "abc xyz")
@@ -123,4 +123,40 @@ public void removeRequestParameterFilterShouldHandleRemainingParamsWhichRequirin
123123
assertThat(actualRequest.getQueryParams()).containsEntry("ccc", singletonList(",xyz"));
124124
}
125125

126+
@Test
127+
void removeRequestParameterFilterShouldHandleEncodedParameterName() {
128+
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost")
129+
.queryParam("foo", "bar")
130+
.queryParam("baz[]", "qux")
131+
.build();
132+
exchange = MockServerWebExchange.from(request);
133+
NameConfig config = new NameConfig();
134+
config.setName("baz[]");
135+
GatewayFilter filter = new RemoveRequestParameterGatewayFilterFactory().apply(config);
136+
137+
filter.filter(exchange, filterChain);
138+
139+
ServerHttpRequest actualRequest = captor.getValue().getRequest();
140+
assertThat(actualRequest.getQueryParams()).doesNotContainKey("baz[]");
141+
assertThat(actualRequest.getQueryParams()).containsEntry("foo", singletonList("bar"));
142+
}
143+
144+
@Test
145+
void removeRequestParameterFilterShouldMaintainEncodedParameters() {
146+
MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost")
147+
.queryParam("foo", "bar")
148+
.queryParam("baz[]", "qux")
149+
.build();
150+
exchange = MockServerWebExchange.from(request);
151+
NameConfig config = new NameConfig();
152+
config.setName("foo");
153+
GatewayFilter filter = new RemoveRequestParameterGatewayFilterFactory().apply(config);
154+
155+
filter.filter(exchange, filterChain);
156+
157+
ServerHttpRequest actualRequest = captor.getValue().getRequest();
158+
assertThat(actualRequest.getQueryParams()).doesNotContainKey("foo");
159+
assertThat(actualRequest.getQueryParams()).containsEntry("baz[]", singletonList("qux"));
160+
}
161+
126162
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,23 @@ void rewriteRequestParameterFilterDoesNotAddParamIfNameNotFound() {
7171
}
7272

7373
@Test
74-
void rewriteRequestParameterFilterWorksWithSpecialCharacters() {
74+
void rewriteRequestParameterFilterWithSpecialCharactersInParameterValue() {
7575
testRewriteRequestParameterFilter("campaign", "black friday~(1.A-B_C!)", "campaign=old&color=green",
7676
Map.of("campaign", List.of("black friday~(1.A-B_C!)"), "color", List.of("green")));
7777
}
7878

79+
@Test
80+
void rewriteRequestParameterFilterWithSpecialCharactersInParameterName() {
81+
testRewriteRequestParameterFilter("campaign[]", "red", "campaign%5B%5D=blue&color=green",
82+
Map.of("campaign[]", List.of("red"), "color", List.of("green")));
83+
}
84+
85+
@Test
86+
void rewriteRequestParameterFilterKeepsOtherParamsEncoded() {
87+
testRewriteRequestParameterFilter("color", "white", "campaign%5B%5D=blue&color=green",
88+
Map.of("campaign[]", List.of("blue"), "color", List.of("white")));
89+
}
90+
7991
private void testRewriteRequestParameterFilter(String name, String replacement, String query,
8092
Map<String, List<String>> expectedQueryParams) {
8193
GatewayFilter filter = new RewriteRequestParameterGatewayFilterFactory()

0 commit comments

Comments
 (0)