From 6415f1204fa7bf7052971002ce5739b0d931d232 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Mon, 18 Nov 2024 08:45:13 +0900 Subject: [PATCH 01/12] Prevent duplicate encoding issue in HTTP request parameter rewrite filter Signed-off-by: raccoonback --- ...eRequestParameterGatewayFilterFactory.java | 25 +++++++++++++++---- ...estParameterGatewayFilterFactoryTests.java | 14 ++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactory.java index ecffdf15..02bccdaf 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactory.java @@ -26,10 +26,14 @@ import org.springframework.cloud.gateway.filter.GatewayFilter; import org.springframework.cloud.gateway.filter.GatewayFilterChain; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.util.Assert; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UriUtils; import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator; +import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap; /** * @author Fredrich Ombico @@ -59,14 +63,25 @@ public class RewriteRequestParameterGatewayFilterFactory ServerHttpRequest req = exchange.getRequest(); UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(req.getURI()); - if (req.getQueryParams().containsKey(config.getName())) { - uriComponentsBuilder.replaceQueryParam(config.getName(), config.getReplacement()); + + MultiValueMap queryParams = new LinkedMultiValueMap<>(req.getQueryParams()); + if (queryParams.containsKey(config.getName())) { + queryParams.remove(config.getName()); + queryParams.add(config.getName(), config.getReplacement()); } - URI uri = uriComponentsBuilder.build().toUri(); - ServerHttpRequest request = req.mutate().uri(uri).build(); + try { + MultiValueMap encodedQueryParams = UriUtils.encodeQueryParams(queryParams); + URI uri = uriComponentsBuilder.replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams)) + .build(true) + .toUri(); - return chain.filter(exchange.mutate().request(request).build()); + ServerHttpRequest request = req.mutate().uri(uri).build(); + return chain.filter(exchange.mutate().request(request).build()); + } + catch (IllegalArgumentException ex) { + throw new IllegalStateException("Invalid URI query: \"" + queryParams + "\""); + } } @Override diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactoryTests.java index 9df80c61..f7803861 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RewriteRequestParameterGatewayFilterFactoryTests.java @@ -71,11 +71,23 @@ class RewriteRequestParameterGatewayFilterFactoryTests { } @Test - void rewriteRequestParameterFilterWorksWithSpecialCharacters() { + void rewriteRequestParameterFilterWithSpecialCharactersInParameterValue() { testRewriteRequestParameterFilter("campaign", "black friday~(1.A-B_C!)", "campaign=old&color=green", Map.of("campaign", List.of("black friday~(1.A-B_C!)"), "color", List.of("green"))); } + @Test + void rewriteRequestParameterFilterWithSpecialCharactersInParameterName() { + testRewriteRequestParameterFilter("campaign[]", "red", "campaign%5B%5D=blue&color=green", + Map.of("campaign[]", List.of("red"), "color", List.of("green"))); + } + + @Test + void rewriteRequestParameterFilterKeepsOtherParamsEncoded() { + testRewriteRequestParameterFilter("color", "white", "campaign%5B%5D=blue&color=green", + Map.of("campaign[]", List.of("blue"), "color", List.of("white"))); + } + private void testRewriteRequestParameterFilter(String name, String replacement, String query, Map> expectedQueryParams) { GatewayFilter filter = new RewriteRequestParameterGatewayFilterFactory() From 49b6d0a0a3af744e8ca364becfa8c8ac4b18e58e Mon Sep 17 00:00:00 2001 From: raccoonback Date: Mon, 18 Nov 2024 08:45:35 +0900 Subject: [PATCH 02/12] Prevent duplicate encoding issue in HTTP request parameter remove filter Signed-off-by: raccoonback --- ...eRequestParameterGatewayFilterFactory.java | 20 ++++++---- ...estParameterGatewayFilterFactoryTests.java | 37 +++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactory.java index a2e2c92a..9d17f6ef 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactory.java @@ -29,6 +29,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UriUtils; import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator; import static org.springframework.util.CollectionUtils.unmodifiableMultiValueMap; @@ -57,14 +58,19 @@ public class RemoveRequestParameterGatewayFilterFactory MultiValueMap queryParams = new LinkedMultiValueMap<>(request.getQueryParams()); queryParams.remove(config.getName()); - URI newUri = UriComponentsBuilder.fromUri(request.getURI()) - .replaceQueryParams(unmodifiableMultiValueMap(queryParams)) - .build() - .toUri(); + try { + MultiValueMap encodedQueryParams = UriUtils.encodeQueryParams(queryParams); + URI newUri = UriComponentsBuilder.fromUri(request.getURI()) + .replaceQueryParams(unmodifiableMultiValueMap(encodedQueryParams)) + .build(true) + .toUri(); - ServerHttpRequest updatedRequest = exchange.getRequest().mutate().uri(newUri).build(); - - return chain.filter(exchange.mutate().request(updatedRequest).build()); + ServerHttpRequest updatedRequest = exchange.getRequest().mutate().uri(newUri).build(); + return chain.filter(exchange.mutate().request(updatedRequest).build()); + } + catch (IllegalArgumentException ex) { + throw new IllegalStateException("Invalid URI query: \"" + queryParams + "\""); + } } @Override diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java index a1fdf471..c5accf12 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java @@ -123,4 +123,41 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { assertThat(actualRequest.getQueryParams()).containsEntry("ccc", singletonList(",xyz")); } + + @Test + public void removeRequestParameterFilterShouldHandleEncodedParameterName() { + MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") + .queryParam("foo", "bar") + .queryParam("baz[]", "qux") + .build(); + exchange = MockServerWebExchange.from(request); + NameConfig config = new NameConfig(); + config.setName("baz[]"); + GatewayFilter filter = new RemoveRequestParameterGatewayFilterFactory().apply(config); + + filter.filter(exchange, filterChain); + + ServerHttpRequest actualRequest = captor.getValue().getRequest(); + assertThat(actualRequest.getQueryParams()).doesNotContainKey("baz[]"); + assertThat(actualRequest.getQueryParams()).containsEntry("foo", singletonList("bar")); + } + + @Test + public void removeRequestParameterFilterShouldMaintainEncodedParameters() { + MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") + .queryParam("foo", "bar") + .queryParam("baz[]", "qux") + .build(); + exchange = MockServerWebExchange.from(request); + NameConfig config = new NameConfig(); + config.setName("foo"); + GatewayFilter filter = new RemoveRequestParameterGatewayFilterFactory().apply(config); + + filter.filter(exchange, filterChain); + + ServerHttpRequest actualRequest = captor.getValue().getRequest(); + assertThat(actualRequest.getQueryParams()).doesNotContainKey("foo"); + assertThat(actualRequest.getQueryParams()).containsEntry("baz[]", singletonList("qux")); + } + } From c62f2b44599d112111803a4480d9cba1934e08e5 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Mon, 18 Nov 2024 08:50:49 +0900 Subject: [PATCH 03/12] Remove unnecessary public modifiers from test methods Signed-off-by: raccoonback --- ...questParameterGatewayFilterFactoryTests.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java index c5accf12..a96aef38 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RemoveRequestParameterGatewayFilterFactoryTests.java @@ -37,7 +37,7 @@ import static org.mockito.Mockito.when; /** * @author Thirunavukkarasu Ravichandran */ -public class RemoveRequestParameterGatewayFilterFactoryTests { +class RemoveRequestParameterGatewayFilterFactoryTests { private ServerWebExchange exchange; @@ -46,7 +46,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { private ArgumentCaptor captor; @BeforeEach - public void setUp() { + void setUp() { filterChain = mock(GatewayFilterChain.class); captor = ArgumentCaptor.forClass(ServerWebExchange.class); when(filterChain.filter(captor.capture())).thenReturn(Mono.empty()); @@ -54,7 +54,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { } @Test - public void removeRequestParameterFilterWorks() { + void removeRequestParameterFilterWorks() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") .queryParam("foo", singletonList("bar")) .build(); @@ -70,7 +70,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { } @Test - public void removeRequestParameterFilterWorksWhenParamIsNotPresentInRequest() { + void removeRequestParameterFilterWorksWhenParamIsNotPresentInRequest() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost").build(); exchange = MockServerWebExchange.from(request); NameConfig config = new NameConfig(); @@ -84,7 +84,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { } @Test - public void removeRequestParameterFilterShouldOnlyRemoveSpecifiedParam() { + void removeRequestParameterFilterShouldOnlyRemoveSpecifiedParam() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") .queryParam("foo", "bar") .queryParam("abc", "xyz") @@ -102,7 +102,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { } @Test - public void removeRequestParameterFilterShouldHandleRemainingParamsWhichRequiringEncoding() { + void removeRequestParameterFilterShouldHandleRemainingParamsWhichRequiringEncoding() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") .queryParam("foo", "bar") .queryParam("aaa", "abc xyz") @@ -123,9 +123,8 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { assertThat(actualRequest.getQueryParams()).containsEntry("ccc", singletonList(",xyz")); } - @Test - public void removeRequestParameterFilterShouldHandleEncodedParameterName() { + void removeRequestParameterFilterShouldHandleEncodedParameterName() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") .queryParam("foo", "bar") .queryParam("baz[]", "qux") @@ -143,7 +142,7 @@ public class RemoveRequestParameterGatewayFilterFactoryTests { } @Test - public void removeRequestParameterFilterShouldMaintainEncodedParameters() { + void removeRequestParameterFilterShouldMaintainEncodedParameters() { MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost") .queryParam("foo", "bar") .queryParam("baz[]", "qux") From 1bc974e82430898ee4f950ae1a5ea97a96575494 Mon Sep 17 00:00:00 2001 From: spring-builds Date: Thu, 13 Mar 2025 13:28:24 +0000 Subject: [PATCH 04/12] Bumping versions Signed-off-by: raccoonback --- README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index 9d84e36e..d4195c40 100644 --- a/README.adoc +++ b/README.adoc @@ -224,7 +224,7 @@ Spring Cloud Build brings along the `basepom:duplicate-finder-maven-plugin`, th [[duplicate-finder-configuration]] === Duplicate Finder configuration -Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the projecst's `pom.xml`. +Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the project's `pom.xml`. .pom.xml [source,xml] From 2f73ed234a6bd9654a17fadce3ac340a5fd7dca9 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Mon, 2 Dec 2024 23:39:39 +0900 Subject: [PATCH 05/12] fix typo Signed-off-by: raccoonback --- .../cloud/gateway/filter/NettyWriteResponseFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java index c06c175b..3dae9d95 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java @@ -116,7 +116,7 @@ public class NettyWriteResponseFilter implements GlobalFilter, Ordered { byteBuf.release(); return buffer; } - throw new IllegalArgumentException("Unkown DataBufferFactory type " + bufferFactory.getClass()); + throw new IllegalArgumentException("Unknown DataBufferFactory type " + bufferFactory.getClass()); } private void cleanup(ServerWebExchange exchange) { From 3ecb4b62dbd89f67bca1151a95c561cf7ee15fa9 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Mon, 2 Dec 2024 23:40:54 +0900 Subject: [PATCH 06/12] dispose the connection regardless of the channel state Signed-off-by: raccoonback --- .../gateway/filter/NettyWriteResponseFilter.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java index 3dae9d95..278c428b 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyWriteResponseFilter.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.publisher.SignalType; import reactor.netty.Connection; import org.springframework.core.Ordered; @@ -98,8 +99,12 @@ public class NettyWriteResponseFilter implements GlobalFilter, Ordered { return (isStreamingMediaType(contentType) ? response.writeAndFlushWith(body.map(Flux::just)) : response.writeWith(body)); - })).doOnCancel(() -> cleanup(exchange)) - .doOnError(throwable -> cleanup(exchange)); + })) + .doFinally(signalType -> { + if (signalType == SignalType.CANCEL || signalType == SignalType.ON_ERROR) { + cleanup(exchange); + } + }); // @formatter:on } @@ -121,7 +126,7 @@ public class NettyWriteResponseFilter implements GlobalFilter, Ordered { private void cleanup(ServerWebExchange exchange) { Connection connection = exchange.getAttribute(CLIENT_RESPONSE_CONN_ATTR); - if (connection != null && connection.channel().isActive()) { + if (connection != null) { connection.dispose(); } } From 1f10961bd729559ed043cb2fdf6f8e3d47fa484f Mon Sep 17 00:00:00 2001 From: spring-builds Date: Thu, 13 Mar 2025 13:28:24 +0000 Subject: [PATCH 07/12] Bumping versions Signed-off-by: raccoonback --- README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index 9d84e36e..d4195c40 100644 --- a/README.adoc +++ b/README.adoc @@ -224,7 +224,7 @@ Spring Cloud Build brings along the `basepom:duplicate-finder-maven-plugin`, th [[duplicate-finder-configuration]] === Duplicate Finder configuration -Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the projecst's `pom.xml`. +Duplicate finder is *enabled by default* and will run in the `verify` phase of your Maven build, but it will only take effect in your project if you add the `duplicate-finder-maven-plugin` to the `build` section of the project's `pom.xml`. .pom.xml [source,xml] From 07aca3e63e7279e9265779083a416848e5af238d Mon Sep 17 00:00:00 2001 From: raccoonback Date: Sun, 5 Jan 2025 15:54:56 +0900 Subject: [PATCH 08/12] Prevent double encoding in setPath method The setPath 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 --- .../mvc/filter/BeforeFilterFunctions.java | 6 +- .../filter/BeforeFilterFunctionsTests.java | 160 ++++++++++++++++++ 2 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java 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 93a3eb7c..4f273ea2 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 @@ -350,9 +350,11 @@ public abstract class BeforeFilterFunctions { return request -> { Map uriVariables = MvcUtils.getUriTemplateVariables(request); URI uri = uriTemplate.expand(uriVariables); - String newPath = uri.getRawPath(); - URI prefixedUri = UriComponentsBuilder.fromUri(request.uri()).replacePath(newPath).build().toUri(); + URI prefixedUri = UriComponentsBuilder.fromUri(request.uri()) + .replacePath(uri.getRawPath()) + .build(true) + .toUri(); return ServerRequest.from(request).uri(prefixedUri).build(); }; } 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 new file mode 100644 index 00000000..d72fe496 --- /dev/null +++ b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/filter/BeforeFilterFunctionsTests.java @@ -0,0 +1,160 @@ +/* + * Copyright 2013-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.server.mvc.filter; + +import java.util.Collections; + +import org.junit.jupiter.api.Test; + +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import org.springframework.web.servlet.function.ServerRequest; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author raccoonback + */ +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); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.setPath("/new/path").apply(request); + + assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path"); + } + + @Test + void setEncodedPath() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") + .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"); + } + + @Test + void setPathWithParameters() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") + .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"); + } + + @Test + void setPathWithEncodedParameters() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/legacy/path") + .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"); + } + +} From 962c8c34e5b631a9839ec5dfa74ef50b051538b0 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Sun, 5 Jan 2025 15:57:37 +0900 Subject: [PATCH 09/12] Prevent double encoding in stripPrefix method The stripPrefix 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 --- .../mvc/filter/BeforeFilterFunctions.java | 2 +- .../filter/BeforeFilterFunctionsTests.java | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) 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 4f273ea2..bf4a5180 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 @@ -409,7 +409,7 @@ public abstract class BeforeFilterFunctions { URI prefixedUri = UriComponentsBuilder.fromUri(request.uri()) .replacePath(newPath.toString()) - .build() + .build(true) .toUri(); return ServerRequest.from(request).uri(prefixedUri).build(); }; 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 d72fe496..7944baac 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 @@ -157,4 +157,42 @@ class BeforeFilterFunctionsTests { assertThat(result.uri().toString()).isEqualTo("http://localhost/new/path?foo%5B%5D=bar%5B%5D"); } + @Test + void stripPrefix() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3") + .buildRequest(null); + + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + + ServerRequest result = BeforeFilterFunctions.stripPrefix(2).apply(request); + + assertThat(result.uri().toString()).isEqualTo("http://localhost/depth3"); + } + + @Test + void stripPrefixWithEncodedPath() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3/é") + .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"); + } + + @Test + void stripPrefixWithEncodedParameters() { + MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get("http://localhost/depth1/depth2/depth3") + .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"); + } + } From 478fab319f43efd0d6fe7e1df893493158164429 Mon Sep 17 00:00:00 2001 From: raccoonback Date: Fri, 14 Mar 2025 08:10:03 +0900 Subject: [PATCH 10/12] 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 --- .../mvc/filter/BeforeFilterFunctions.java | 7 +- .../filter/BeforeFilterFunctionsTests.java | 172 ++++++++---------- 2 files changed, 84 insertions(+), 95 deletions(-) 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"); } } From 5b08d91a8b157bd2150c764dabed880468485c32 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 14 Mar 2025 09:07:30 -0400 Subject: [PATCH 11/12] Adding 4.1.x to GitHub build action --- .github/workflows/maven.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 6aceddb1..d9851135 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -5,9 +5,9 @@ name: Build on: push: - branches: [ main, 3.1.x ] + branches: [ main, 4.1.x, 3.1.x ] pull_request: - branches: [ main, 3.1.x ] + branches: [ main, 4.1.x, 3.1.x ] jobs: build: From 470068bc0b56582a35d974d87eb3e3b1f6dced67 Mon Sep 17 00:00:00 2001 From: Abel Salgado Romero Date: Fri, 14 Mar 2025 10:14:44 +0100 Subject: [PATCH 12/12] Add enum values to JavaDocs to show in docs Signed-off-by: Abel Salgado Romero --- .../cloud/gateway/config/HttpClientProperties.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java index 85b3b027..0f586c5a 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java @@ -169,7 +169,7 @@ public class HttpClientProperties { public static class Pool { - /** Type of pool for HttpClient to use, defaults to ELASTIC. */ + /** Type of pool for HttpClient to use (elastic, fixed or disabled). */ private PoolType type = PoolType.ELASTIC; /** The channel pool map name, defaults to proxy. */ @@ -302,7 +302,10 @@ public class HttpClientProperties { public static class Proxy { - /** proxyType for proxy configuration of Netty HttpClient. */ + /** + * proxyType for proxy configuration of Netty HttpClient (http, socks4 or + * socks5). + */ private ProxyProvider.Proxy type = ProxyProvider.Proxy.HTTP; /** Hostname for proxy configuration of Netty HttpClient. */