Avoid pathVar-requestParam name collision

Closes gh-34499
spring-projects · Feb 27, 2025 · f62251a · f62251a
1 parent f92f9c1
commit f62251a
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 13 deletions.
diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java
@@ -493,7 +493,7 @@ private String appendQueryParams(
 
 			UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
 			for (Map.Entry<String, List<String>> entry : requestParams.entrySet()) {
-				String nameVar = entry.getKey().replace(":", "%3A"); // suppress treatment as regex
+				String nameVar = "queryParam-" + entry.getKey().replace(":", "%3A"); // suppress treatment as regex
 				uriVars.put(nameVar, entry.getKey());
 				for (int j = 0; j < entry.getValue().size(); j++) {
 					String valueVar = nameVar + "[" + j + "]";

diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java
@@ -79,17 +79,19 @@ void queryParamsWithUriTemplate() {
 
 		assertThat(uriTemplate)
 				.isEqualTo("/path?" +
-						"{param1}={param1[0]}&" +
-						"{param2}={param2[0]}&" +
-						"{param2}={param2[1]}");
+						"{queryParam-param1}={queryParam-param1[0]}&" +
+						"{queryParam-param2}={queryParam-param2[0]}&" +
+						"{queryParam-param2}={queryParam-param2[1]}");
 
 		assertThat(requestValues.getUriVariables())
-				.containsOnlyKeys("param1", "param2", "param1[0]", "param2[0]", "param2[1]")
-				.containsEntry("param1", "param1")
-				.containsEntry("param2", "param2")
-				.containsEntry("param1[0]", "1st value")
-				.containsEntry("param2[0]", "2nd value A")
-				.containsEntry("param2[1]", "2nd value B");
+				.containsOnlyKeys(
+						"queryParam-param1", "queryParam-param2", "queryParam-param1[0]",
+						"queryParam-param2[0]", "queryParam-param2[1]")
+				.containsEntry("queryParam-param1", "param1")
+				.containsEntry("queryParam-param2", "param2")
+				.containsEntry("queryParam-param1[0]", "1st value")
+				.containsEntry("queryParam-param2[0]", "2nd value A")
+				.containsEntry("queryParam-param2[1]", "2nd value B");
 
 		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
 				.encode()
@@ -107,7 +109,7 @@ void queryParamWithSemicolon() {
 				.build();
 
 		String uriTemplate = requestValues.getUriTemplate();
-		assertThat(uriTemplate).isEqualTo("/path?{userId%3Aeq}={userId%3Aeq[0]}");
+		assertThat(uriTemplate).isEqualTo("/path?{queryParam-userId%3Aeq}={queryParam-userId%3Aeq[0]}");
 
 		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
 				.encode()
@@ -162,7 +164,7 @@ void requestPartAndRequestParam() {
 		String uriTemplate = requestValues.getUriTemplate();
 		assertThat(uriTemplate).isNotNull();
 
-		assertThat(uriTemplate).isEqualTo("/path?{query param}={query param[0]}");
+		assertThat(uriTemplate).isEqualTo("/path?{queryParam-query param}={queryParam-query param[0]}");
 
 		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
 				.encode()

diff --git a/.../test/java/org/springframework/web/service/invoker/PathVariableArgumentResolverTests.java b/.../test/java/org/springframework/web/service/invoker/PathVariableArgumentResolverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-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.
@@ -16,11 +16,15 @@
 
 package org.springframework.web.service.invoker;
 
+import java.net.URI;
+
 import org.junit.jupiter.api.Test;
 
 import org.springframework.lang.Nullable;
 import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.service.annotation.GetExchange;
+import org.springframework.web.util.DefaultUriBuilderFactory;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -46,6 +50,17 @@ void pathVariable() {
 		assertPathVariable("id", "test");
 	}
 
+	@Test // gh-34499
+	void pathVariableAndRequestParamWithSameName() {
+		this.service.executeWithPathVarAndRequestParam("{transfer-id}", "aValue");
+
+		assertPathVariable("transfer-id", "{transfer-id}");
+
+		HttpRequestValues values = this.client.getRequestValues();
+		URI uri = (new DefaultUriBuilderFactory()).expand(values.getUriTemplate(), values.getUriVariables());
+		assertThat(uri.toString()).isEqualTo("/transfers/%7Btransfer-id%7D?transfer-id=aValue");
+	}
+
 	@SuppressWarnings("SameParameterValue")
 	private void assertPathVariable(String name, @Nullable String expectedValue) {
 		assertThat(this.client.getRequestValues().getUriVariables().get(name)).isEqualTo(expectedValue);
@@ -57,6 +72,11 @@ private interface Service {
 		@GetExchange
 		void execute(@PathVariable String id);
 
+		@GetExchange("/transfers/{transfer-id}")
+		void executeWithPathVarAndRequestParam(
+				@PathVariable("transfer-id") String transferId,
+				@RequestParam("transfer-id") String transferIdParam);
+
 	}
 
 }