Skip to content

Commit 66d7301

Browse files
committed
PathVariable consistently reflects value up to 1st ";"
Given "/{foo}" and "/a=42;c=b", previously that would be treated as a sequence of matrix vars with an empty path variable. After the change the path variable "foo" is "a=42". This should be ok for backawards compatibility since it's unlikely for anything to rely on an empty path variable. Issue: SPR-11897
1 parent 9c08a48 commit 66d7301

File tree

3 files changed

+43
-24
lines changed

3 files changed

+43
-24
lines changed

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,33 +50,22 @@
5050
import org.springframework.web.reactive.BindingContext;
5151
import org.springframework.web.reactive.HandlerMapping;
5252
import org.springframework.web.reactive.HandlerResult;
53-
import org.springframework.web.reactive.result.method.RequestMappingInfo.BuilderConfiguration;
53+
import org.springframework.web.reactive.result.method.RequestMappingInfo.*;
5454
import org.springframework.web.server.MethodNotAllowedException;
5555
import org.springframework.web.server.NotAcceptableStatusException;
5656
import org.springframework.web.server.ServerWebExchange;
5757
import org.springframework.web.server.ServerWebInputException;
5858
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
5959
import org.springframework.web.util.pattern.PathPattern;
6060

61-
import static org.hamcrest.CoreMatchers.containsString;
62-
import static org.junit.Assert.assertEquals;
63-
import static org.junit.Assert.assertNotNull;
64-
import static org.junit.Assert.assertNull;
65-
import static org.junit.Assert.assertSame;
66-
import static org.junit.Assert.assertThat;
67-
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
68-
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.method;
69-
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.post;
70-
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.put;
71-
import static org.springframework.web.bind.annotation.RequestMethod.GET;
72-
import static org.springframework.web.bind.annotation.RequestMethod.HEAD;
73-
import static org.springframework.web.bind.annotation.RequestMethod.OPTIONS;
74-
import static org.springframework.web.method.MvcAnnotationPredicates.getMapping;
75-
import static org.springframework.web.method.MvcAnnotationPredicates.requestMapping;
76-
import static org.springframework.web.method.ResolvableMethod.on;
77-
import static org.springframework.web.reactive.HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE;
78-
import static org.springframework.web.reactive.HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE;
79-
import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths;
61+
import static org.hamcrest.CoreMatchers.*;
62+
import static org.junit.Assert.*;
63+
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
64+
import static org.springframework.web.bind.annotation.RequestMethod.*;
65+
import static org.springframework.web.method.MvcAnnotationPredicates.*;
66+
import static org.springframework.web.method.ResolvableMethod.*;
67+
import static org.springframework.web.reactive.HandlerMapping.*;
68+
import static org.springframework.web.reactive.result.method.RequestMappingInfo.*;
8069

8170
/**
8271
* Unit tests for {@link RequestMappingInfoHandlerMapping}.
@@ -287,6 +276,18 @@ public void handleMatchMatrixVariables() {
287276
assertEquals(Arrays.asList("red", "blue", "green"), matrixVariables.get("colors"));
288277
assertEquals("2012", matrixVariables.getFirst("year"));
289278
assertEquals("cars", uriVariables.get("cars"));
279+
280+
// SPR-11897
281+
exchange = MockServerWebExchange.from(get("/a=42;b=c"));
282+
handleMatch(exchange, "/{foo}");
283+
284+
matrixVariables = getMatrixVariables(exchange, "foo");
285+
uriVariables = getUriTemplateVariables(exchange);
286+
287+
assertNotNull(matrixVariables);
288+
assertEquals(1, matrixVariables.size());
289+
assertEquals("c", matrixVariables.getFirst("b"));
290+
assertEquals("a=42", uriVariables.get("foo"));
290291
}
291292

292293
@Test

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,23 @@ private Map<String, MultiValueMap<String, String>> extractMatrixVariables(
149149

150150
Map<String, MultiValueMap<String, String>> result = new LinkedHashMap<>();
151151
uriVariables.forEach((uriVarKey, uriVarValue) -> {
152+
152153
int equalsIndex = uriVarValue.indexOf('=');
153154
if (equalsIndex == -1) {
154155
return;
155156
}
156157

157-
String matrixVariables;
158-
159158
int semicolonIndex = uriVarValue.indexOf(';');
160-
if ((semicolonIndex == -1) || (semicolonIndex == 0) || (equalsIndex < semicolonIndex)) {
159+
if (semicolonIndex != -1 && semicolonIndex != 0) {
160+
uriVariables.put(uriVarKey, uriVarValue.substring(0, semicolonIndex));
161+
}
162+
163+
String matrixVariables;
164+
if (semicolonIndex == -1 || semicolonIndex == 0 || equalsIndex < semicolonIndex) {
161165
matrixVariables = uriVarValue;
162166
}
163167
else {
164168
matrixVariables = uriVarValue.substring(semicolonIndex + 1);
165-
uriVariables.put(uriVarKey, uriVarValue.substring(0, semicolonIndex));
166169
}
167170

168171
MultiValueMap<String, String> vars = WebUtils.parseMatrixVariables(matrixVariables);

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ public void handleMatchMatrixVariables() {
311311
MultiValueMap<String, String> matrixVariables;
312312
Map<String, String> uriVariables;
313313

314+
// URI var parsed into path variable + matrix params..
314315
request = new MockHttpServletRequest();
315316
handleMatch(request, "/{cars}", "/cars;colors=red,blue,green;year=2012");
316317

@@ -322,6 +323,7 @@ public void handleMatchMatrixVariables() {
322323
assertEquals("2012", matrixVariables.getFirst("year"));
323324
assertEquals("cars", uriVariables.get("cars"));
324325

326+
// URI var with regex for path variable, and URI var for matrix params..
325327
request = new MockHttpServletRequest();
326328
handleMatch(request, "/{cars:[^;]+}{params}", "/cars;colors=red,blue,green;year=2012");
327329

@@ -334,6 +336,7 @@ public void handleMatchMatrixVariables() {
334336
assertEquals("cars", uriVariables.get("cars"));
335337
assertEquals(";colors=red,blue,green;year=2012", uriVariables.get("params"));
336338

339+
// URI var with regex for path variable, and (empty) URI var for matrix params..
337340
request = new MockHttpServletRequest();
338341
handleMatch(request, "/{cars:[^;]+}{params}", "/cars");
339342

@@ -343,6 +346,18 @@ public void handleMatchMatrixVariables() {
343346
assertNull(matrixVariables);
344347
assertEquals("cars", uriVariables.get("cars"));
345348
assertEquals("", uriVariables.get("params"));
349+
350+
// SPR-11897
351+
request = new MockHttpServletRequest();
352+
handleMatch(request, "/{foo}", "/a=42;b=c");
353+
354+
matrixVariables = getMatrixVariables(request, "foo");
355+
uriVariables = getUriTemplateVariables(request);
356+
357+
assertNotNull(matrixVariables);
358+
assertEquals("42", matrixVariables.getFirst("a"));
359+
assertEquals("c", matrixVariables.getFirst("b"));
360+
assertEquals("a=42", uriVariables.get("foo"));
346361
}
347362

348363
@Test // SPR-10140, SPR-16867

0 commit comments

Comments
 (0)