Skip to content

Commit 478fab3

Browse files
raccoonbackraccoonback
committed
Prevent double encoding in removeRequestParameter method
The removeRequestParameter method was causing URLs to be encoded twice under certain conditions. This fix ensures proper handling of parameters to avoid redundant encoding. Signed-off-by: raccoonback <[email protected]>
1 parent 962c8c3 commit 478fab3

File tree

2 files changed

+63
-74
lines changed

2 files changed

+63
-74
lines changed

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

Lines changed: 5 additions & 2 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

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

Lines changed: 58 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -32,167 +32,153 @@
3232
class BeforeFilterFunctionsTests {
3333

3434
@Test
35-
void rewriteRequestParameter() {
36-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
37-
.param("foo", "bar")
38-
.param("baz", "qux")
35+
void setPath() {
36+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
3937
.buildRequest(null);
4038

4139
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
4240

43-
ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request);
41+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
4442

45-
assertThat(result.param("foo")).isPresent().hasValue("replacement");
46-
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo=replacement");
43+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path");
4744
}
4845

4946
@Test
50-
void rewriteOnlyFirstRequestParameter() {
51-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
52-
.param("foo", "bar_1")
53-
.param("foo", "bar_2")
54-
.param("foo", "bar_3")
55-
.param("baz", "qux")
47+
void setEncodedPath() {
48+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
5649
.buildRequest(null);
5750

5851
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
5952

60-
ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request);
53+
ServerRequest result = BeforeFilterFunctions.setPath("/new/é").apply(request);
6154

62-
assertThat(result.param("foo")).isPresent().hasValue("replacement");
63-
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo=replacement");
55+
assertThat(result.uri().toString()).hasToString("http://localhost/new/%C3%A9");
6456
}
6557

6658
@Test
67-
void rewriteEncodedRequestParameter() {
68-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
69-
.param("foo[]", "bar")
70-
.param("baz", "qux")
59+
void setPathWithParameters() {
60+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
61+
.queryParam("foo", "bar")
7162
.buildRequest(null);
7263

7364
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
7465

75-
ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo[]", "replacement[]").apply(request);
66+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
7667

77-
assertThat(result.param("foo[]")).isPresent().hasValue("replacement[]");
78-
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo%5B%5D=replacement%5B%5D");
68+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo=bar");
7969
}
8070

8171
@Test
82-
void rewriteRequestParameterWithEncodedRemainParameters() {
83-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
84-
.param("foo", "bar")
85-
.param("baz[]", "qux[]")
72+
void setPathWithEncodedParameters() {
73+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
74+
.queryParam("foo[]", "bar[]")
8675
.buildRequest(null);
8776

8877
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
8978

90-
ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request);
79+
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
9180

92-
assertThat(result.param("foo")).isPresent().hasValue("replacement");
93-
assertThat(result.uri().toString()).hasToString("http://localhost/path?baz%5B%5D=qux%5B%5D&foo=replacement");
81+
assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo%5B%5D=bar%5B%5D");
9482
}
9583

9684
@Test
97-
void rewriteRequestParameterWithEncodedPath() {
98-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path/é/last")
99-
.param("foo", "bar")
85+
void removeRequestParameter() {
86+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
87+
.queryParam("foo", "bar")
88+
.queryParam("baz", "qux")
10089
.buildRequest(null);
10190

10291
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
10392

104-
ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request);
93+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
10594

106-
assertThat(result.param("foo")).isPresent().hasValue("replacement");
107-
assertThat(result.uri().toString()).hasToString("http://localhost/path/%C3%A9/last?foo=replacement");
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");
10898
}
10999

110100
@Test
111-
void setPath() {
112-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
113-
.buildRequest(null);
114-
115-
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
116-
117-
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
118-
119-
assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path");
120-
}
121-
122-
@Test
123-
void setEncodedPath() {
124-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
125-
.buildRequest(null);
101+
void removeEncodedRequestParameter() {
102+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
103+
.queryParam("foo[]", "bar")
104+
.queryParam("baz", "qux")
105+
.buildRequest(null);
126106

127107
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
128108

129-
ServerRequest result = BeforeFilterFunctions.setPath("/new/é").apply(request);
109+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo[]").apply(request);
130110

131-
assertThat(result.uri().toString()).isEqualTo("http://localhost/new/%C3%A9");
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");
132114
}
133115

134116
@Test
135-
void setPathWithParameters() {
136-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
137-
.queryParam("foo", "bar")
138-
.buildRequest(null);
117+
void removeRequestParameterWithEncodedRemainParameters() {
118+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path")
119+
.queryParam("foo", "bar")
120+
.queryParam("baz[]", "qux[]")
121+
.buildRequest(null);
139122

140123
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
141124

142-
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
125+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
143126

144-
assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path?foo=bar");
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");
145130
}
146131

147132
@Test
148-
void setPathWithEncodedParameters() {
149-
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path")
150-
.queryParam("foo[]", "bar[]")
151-
.buildRequest(null);
133+
void removeRequestParameterWithEncodedPath() {
134+
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/é")
135+
.queryParam("foo", "bar")
136+
.buildRequest(null);
152137

153138
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
154139

155-
ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request);
140+
ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request);
156141

157-
assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path?foo%5B%5D=bar%5B%5D");
142+
assertThat(result.param("foo")).isEmpty();
143+
assertThat(result.uri().toString()).hasToString("http://localhost/%C3%A9");
158144
}
159145

160146
@Test
161147
void stripPrefix() {
162148
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3")
163-
.buildRequest(null);
149+
.buildRequest(null);
164150

165151
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
166152

167153
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
168154

169-
assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3");
155+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3");
170156
}
171157

172158
@Test
173159
void stripPrefixWithEncodedPath() {
174160
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3/é")
175-
.buildRequest(null);
161+
.buildRequest(null);
176162

177163
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
178164

179165
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
180166

181-
assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3/%C3%A9");
167+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3/%C3%A9");
182168
}
183169

184170
@Test
185171
void stripPrefixWithEncodedParameters() {
186172
MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3")
187-
.queryParam("baz[]", "qux[]")
188-
.buildRequest(null);
173+
.queryParam("baz[]", "qux[]")
174+
.buildRequest(null);
189175

190176
ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());
191177

192178
ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request);
193179

194180
assertThat(result.param("baz[]")).isPresent().hasValue("qux[]");
195-
assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3?baz%5B%5D=qux%5B%5D");
181+
assertThat(result.uri().toString()).hasToString("http://localhost/depth3?baz%5B%5D=qux%5B%5D");
196182
}
197183

198184
}

0 commit comments

Comments
 (0)