diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctions.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctions.java index bf4a5180..650f9ca6 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctions.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctions.java @@ -45,6 +45,7 @@ import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriTemplate; +import org.springframework.web.util.UriUtils; import static org.springframework.cloud.gateway.server.mvc.common.MvcUtils.CIRCUITBREAKER_EXECUTION_EXCEPTION_ATTR; import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap; @@ -214,10 +215,12 @@ public abstract class BeforeFilterFunctions { MultiValueMap queryParams = new LinkedMultiValueMap<>(request.params()); queryParams.remove(name); + MultiValueMap encodedQueryParams = UriUtils.encodeQueryParams(queryParams); + // remove from uri URI newUri = UriComponentsBuilder.fromUri(request.uri()) - .replaceQueryParams(unmodifiableMultiValueMap(queryParams)) - .build() + .replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams)) + .build(true) .toUri(); // remove resolved params from request diff --git a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java index 7944baac..095ec4f9 100644 --- a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java +++ b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java @@ -31,168 +31,154 @@ import static org.assertj.core.api.Assertions.assertThat; */ class BeforeFilterFunctionsTests { - @Test - void rewriteRequestParameter() { - MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") - .param("foo", "bar") - .param("baz", "qux") - .buildRequest(null); - - ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); - - ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request); - - assertThat(result.param("foo")).isPresent().hasValue("replacement"); - assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo=replacement"); - } - - @Test - void rewriteOnlyFirstRequestParameter() { - MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") - .param("foo", "bar_1") - .param("foo", "bar_2") - .param("foo", "bar_3") - .param("baz", "qux") - .buildRequest(null); - - ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); - - ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request); - - assertThat(result.param("foo")).isPresent().hasValue("replacement"); - assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo=replacement"); - } - - @Test - void rewriteEncodedRequestParameter() { - MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") - .param("foo[]", "bar") - .param("baz", "qux") - .buildRequest(null); - - ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); - - ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo[]", "replacement[]").apply(request); - - assertThat(result.param("foo[]")).isPresent().hasValue("replacement[]"); - assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux&foo%5B%5D=replacement%5B%5D"); - } - - @Test - void rewriteRequestParameterWithEncodedRemainParameters() { - MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") - .param("foo", "bar") - .param("baz[]", "qux[]") - .buildRequest(null); - - ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); - - ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request); - - assertThat(result.param("foo")).isPresent().hasValue("replacement"); - assertThat(result.uri().toString()).hasToString("http://localhost/path?baz%5B%5D=qux%5B%5D&foo=replacement"); - } - - @Test - void rewriteRequestParameterWithEncodedPath() { - MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path/é/last") - .param("foo", "bar") - .buildRequest(null); - - ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); - - ServerRequest result = BeforeFilterFunctions.rewriteRequestParameter("foo", "replacement").apply(request); - - assertThat(result.param("foo")).isPresent().hasValue("replacement"); - assertThat(result.uri().toString()).hasToString("http://localhost/path/%C3%A9/last?foo=replacement"); - } - @Test void setPath() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") - .buildRequest(null); + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path"); + assertThat(result.uri().toString()).hasToString("http://localhost/new/path"); } @Test void setEncodedPath() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") - .buildRequest(null); + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.setPath("/new/é").apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/new/%C3%A9"); + assertThat(result.uri().toString()).hasToString("http://localhost/new/%C3%A9"); } @Test void setPathWithParameters() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") - .queryParam("foo", "bar") - .buildRequest(null); + .queryParam("foo", "bar") + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path?foo=bar"); + assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo=bar"); } @Test void setPathWithEncodedParameters() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") - .queryParam("foo[]", "bar[]") - .buildRequest(null); + .queryParam("foo[]", "bar[]") + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path?foo%5B%5D=bar%5B%5D"); + assertThat(result.uri().toString()).hasToString("http://localhost/new/path?foo%5B%5D=bar%5B%5D"); + } + + @Test + void removeRequestParameter() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") + .queryParam("foo", "bar") + .queryParam("baz", "qux") + .buildRequest(null); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request); + + assertThat(result.param("foo")).isEmpty(); + assertThat(result.param("baz")).isPresent().hasValue("qux"); + assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux"); + } + + @Test + void removeEncodedRequestParameter() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") + .queryParam("foo[]", "bar") + .queryParam("baz", "qux") + .buildRequest(null); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo[]").apply(request); + + assertThat(result.param("foo[]")).isEmpty(); + assertThat(result.param("baz")).isPresent().hasValue("qux"); + assertThat(result.uri().toString()).hasToString("http://localhost/path?baz=qux"); + } + + @Test + void removeRequestParameterWithEncodedRemainParameters() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/path") + .queryParam("foo", "bar") + .queryParam("baz[]", "qux[]") + .buildRequest(null); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request); + + assertThat(result.param("foo")).isEmpty(); + assertThat(result.param("baz[]")).isPresent().hasValue("qux[]"); + assertThat(result.uri().toString()).hasToString("http://localhost/path?baz%5B%5D=qux%5B%5D"); + } + + @Test + void removeRequestParameterWithEncodedPath() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/é") + .queryParam("foo", "bar") + .buildRequest(null); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.removeRequestParameter("foo").apply(request); + + assertThat(result.param("foo")).isEmpty(); + assertThat(result.uri().toString()).hasToString("http://localhost/%C3%A9"); } @Test void stripPrefix() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3") - .buildRequest(null); + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3"); + assertThat(result.uri().toString()).hasToString("http://localhost/depth3"); } @Test void stripPrefixWithEncodedPath() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3/é") - .buildRequest(null); + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request); - assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3/%C3%A9"); + assertThat(result.uri().toString()).hasToString("http://localhost/depth3/%C3%A9"); } @Test void stripPrefixWithEncodedParameters() { MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3") - .queryParam("baz[]", "qux[]") - .buildRequest(null); + .queryParam("baz[]", "qux[]") + .buildRequest(null); ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request); assertThat(result.param("baz[]")).isPresent().hasValue("qux[]"); - assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3?baz%5B%5D=qux%5B%5D"); + assertThat(result.uri().toString()).hasToString("http://localhost/depth3?baz%5B%5D=qux%5B%5D"); } }