Skip to content

Commit 4998631

Browse files
ryanjbaxterryanjbaxter
authored
Merge pull request #3658 from raccoonback/prevent-duplicated-encoding-in-mvc
Prevent duplicated encoding of `setPath()`, `stripPrefix()` in mvc
2 parents e6de344 + 6f056e4 commit 4998631

File tree

2 files changed

+194
-5
lines changed

2 files changed

+194
-5
lines changed

‎spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctions.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.springframework.web.servlet.function.ServerRequest;
4646
import org.springframework.web.util.UriComponentsBuilder;
4747
import org.springframework.web.util.UriTemplate;
48+
import org.springframework.web.util.UriUtils;
4849

4950
import static org.springframework.cloud.gateway.server.mvc.common.MvcUtils.CIRCUITBREAKER_EXECUTION_EXCEPTION_ATTR;
5051
import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap;
@@ -214,10 +215,12 @@ public static Function<ServerRequest, ServerRequest> removeRequestParameter(Stri
214215
MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>(request.params());
215216
queryParams.remove(name);
216217

218+
MultiValueMap<String, String> encodedQueryParams = UriUtils.encodeQueryParams(queryParams);
219+
217220
// remove from uri
218221
URI newUri = UriComponentsBuilder.fromUri(request.uri())
219-
.replaceQueryParams(unmodifiableMultiValueMap(queryParams))
220-
.build()
222+
.replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams))
223+
.build(true)
221224
.toUri();
222225

223226
// remove resolved params from request
@@ -350,9 +353,11 @@ public static Function<ServerRequest, ServerRequest> setPath(String path) {
350353
return request -> {
351354
Map<String, Object> uriVariables = MvcUtils.getUriTemplateVariables(request);
352355
URI uri = uriTemplate.expand(uriVariables);
353-
String newPath = uri.getRawPath();
354356

355-
URI prefixedUri = UriComponentsBuilder.fromUri(request.uri()).replacePath(newPath).build().toUri();
357+
URI prefixedUri = UriComponentsBuilder.fromUri(request.uri())
358+
.replacePath(uri.getRawPath())
359+
.build(true)
360+
.toUri();
356361
return ServerRequest.from(request).uri(prefixedUri).build();
357362
};
358363
}
@@ -407,7 +412,7 @@ public static Function<ServerRequest, ServerRequest> stripPrefix(int parts) {
407412

408413
URI prefixedUri = UriComponentsBuilder.fromUri(request.uri())
409414
.replacePath(newPath.toString())
410-
.build()
415+
.build(true)
411416
.toUri();
412417
return ServerRequest.from(request).uri(prefixedUri).build();
413418
};

‎spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
* Copyright 2013-2025 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.server.mvc.filter;
18+
19+
import java.util.Collections;
20+
21+
import org.junit.jupiter.api.Test;
22+
23+
import org.springframework.mock.web.MockHttpServletRequest;
24+
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
25+
import org.springframework.web.servlet.function.ServerRequest;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
29+
/**
30+
* @author raccoonback
31+
*/
32+
class BeforeFilterFunctionsTests {
33+
34+
@Test
35+
void setPath() {
36+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
37+
.buildRequest(null);
38+
39+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
40+
41+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
42+
43+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path");
44+
}
45+
46+
@Test
47+
void setEncodedPath() {
48+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
49+
.buildRequest(null);
50+
51+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
52+
53+
ServerRequest result = BeforeFilterFunctions.setPath("/new/é").apply(request);
54+
55+
assertThat(result.uri().toString()).hasToString("http://localhost/new/%C3%A9");
56+
}
57+
58+
@Test
59+
void setPathWithParameters() {
60+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
61+
.queryParam("foo", "bar")
62+
.buildRequest(null);
63+
64+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
65+
66+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
67+
68+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo=bar");
69+
}
70+
71+
@Test
72+
void setPathWithEncodedParameters() {
73+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
74+
.queryParam("foo[]", "bar[]")
75+
.buildRequest(null);
76+
77+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
78+
79+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
80+
81+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo%5B%5D=bar%5B%5D");
82+
}
83+
84+
@Test
85+
void removeRequestParameter() {
86+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
87+
.queryParam("foo", "bar")
88+
.queryParam("baz", "qux")
89+
.buildRequest(null);
90+
91+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
92+
93+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
94+
95+
assertThat(result.param("foo")).isEmpty();
96+
assertThat(result.param("baz")).isPresent().hasValue("qux");
97+
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux");
98+
}
99+
100+
@Test
101+
void removeEncodedRequestParameter() {
102+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
103+
.queryParam("foo[]", "bar")
104+
.queryParam("baz", "qux")
105+
.buildRequest(null);
106+
107+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
108+
109+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo[]").apply(request);
110+
111+
assertThat(result.param("foo[]")).isEmpty();
112+
assertThat(result.param("baz")).isPresent().hasValue("qux");
113+
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux");
114+
}
115+
116+
@Test
117+
void removeRequestParameterWithEncodedRemainParameters() {
118+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
119+
.queryParam("foo", "bar")
120+
.queryParam("baz[]", "qux[]")
121+
.buildRequest(null);
122+
123+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
124+
125+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
126+
127+
assertThat(result.param("foo")).isEmpty();
128+
assertThat(result.param("baz[]")).isPresent().hasValue("qux[]");
129+
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz%5B%5D=qux%5B%5D");
130+
}
131+
132+
@Test
133+
void removeRequestParameterWithEncodedPath() {
134+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/é")
135+
.queryParam("foo", "bar")
136+
.buildRequest(null);
137+
138+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
139+
140+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
141+
142+
assertThat(result.param("foo")).isEmpty();
143+
assertThat(result.uri().toString()).hasToString("http://localhost/%C3%A9");
144+
}
145+
146+
@Test
147+
void stripPrefix() {
148+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3")
149+
.buildRequest(null);
150+
151+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
152+
153+
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
154+
155+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3");
156+
}
157+
158+
@Test
159+
void stripPrefixWithEncodedPath() {
160+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3/é")
161+
.buildRequest(null);
162+
163+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
164+
165+
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
166+
167+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3/%C3%A9");
168+
}
169+
170+
@Test
171+
void stripPrefixWithEncodedParameters() {
172+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3")
173+
.queryParam("baz[]", "qux[]")
174+
.buildRequest(null);
175+
176+
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
177+
178+
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
179+
180+
assertThat(result.param("baz[]")).isPresent().hasValue("qux[]");
181+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3?baz%5B%5D=qux%5B%5D");
182+
}
183+
184+
}

0 commit comments

Comments
 (0)