Skip to content

Commit d1f888a

Browse files
committed
Empty path mapping behaves consistently
An empty path mapping in an @RequestMapping now consistently matches to empty paths regardless of whether there are both type and method level, annotations, or method-level only. Closes gh-22543
1 parent f839c1f commit d1f888a

File tree

17 files changed

+96
-41
lines changed

17 files changed

+96
-41
lines changed

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnection.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public final class MockMvcWebConnection implements WebConnection {
6363

6464
private final MockMvc mockMvc;
6565

66+
@Nullable
6667
private final String contextPath;
6768

6869
private WebClient webClient;
@@ -91,7 +92,7 @@ public MockMvcWebConnection(MockMvc mockMvc, WebClient webClient) {
9192
* @param webClient the {@link WebClient} to use (never {@code null})
9293
* @param contextPath the contextPath to use
9394
*/
94-
public MockMvcWebConnection(MockMvc mockMvc, WebClient webClient, String contextPath) {
95+
public MockMvcWebConnection(MockMvc mockMvc, WebClient webClient, @Nullable String contextPath) {
9596
Assert.notNull(mockMvc, "MockMvc must not be null");
9697
Assert.notNull(webClient, "WebClient must not be null");
9798
validateContextPath(contextPath);

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/ForwardController.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,7 +28,7 @@ public class ForwardController {
2828

2929
@RequestMapping("/forward")
3030
public String forward() {
31-
return "forward:/";
31+
return "forward:/a";
3232
}
3333

3434
}

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HelloController.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,7 +28,7 @@
2828
@RestController
2929
public class HelloController {
3030

31-
@RequestMapping
31+
@RequestMapping("/a")
3232
public String header(HttpServletRequest request) {
3333
return "hello";
3434
}

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcConnectionBuilderSupportTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -151,7 +151,7 @@ static class Config {
151151
@RestController
152152
static class ContextPathController {
153153

154-
@RequestMapping
154+
@RequestMapping("/def")
155155
public String contextPath(HttpServletRequest request) {
156156
return request.getContextPath();
157157
}

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebClientBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -157,7 +157,7 @@ static class Config {
157157
@RestController
158158
static class ContextPathController {
159159

160-
@RequestMapping
160+
@RequestMapping("/test")
161161
public String contextPath(HttpServletRequest request) {
162162
return "mvc";
163163
}

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/MockMvcWebConnectionTests.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818

1919
import java.io.IOException;
2020

21+
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
2122
import com.gargoylesoftware.htmlunit.Page;
2223
import com.gargoylesoftware.htmlunit.WebClient;
2324
import org.junit.Test;
@@ -44,7 +45,7 @@ public class MockMvcWebConnectionTests {
4445

4546
@Test
4647
public void contextPathNull() throws IOException {
47-
this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, this.webClient));
48+
this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, this.webClient, null));
4849
Page page = this.webClient.getPage("http://localhost/context/a");
4950
assertThat(page.getWebResponse().getStatusCode(), equalTo(200));
5051
}
@@ -59,8 +60,21 @@ public void contextPathExplicit() throws IOException {
5960
@Test
6061
public void contextPathEmpty() throws IOException {
6162
this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, this.webClient, ""));
62-
Page page = this.webClient.getPage("http://localhost/context/a");
63-
assertThat(page.getWebResponse().getStatusCode(), equalTo(200));
63+
try {
64+
this.webClient.getPage("http://localhost/context/a");
65+
fail("Empty context path (root context) should not match to a URL with a context path");
66+
}
67+
catch (FailingHttpStatusCodeException ex) {
68+
assertEquals(404, ex.getStatusCode());
69+
}
70+
this.webClient.setWebConnection(new MockMvcWebConnection(this.mockMvc, this.webClient));
71+
try {
72+
this.webClient.getPage("http://localhost/context/a");
73+
fail("No context is the same providing an empty context path");
74+
}
75+
catch (FailingHttpStatusCodeException ex) {
76+
assertEquals(404, ex.getStatusCode());
77+
}
6478
}
6579

6680
@Test

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/webdriver/MockMvcHtmlUnitDriverBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -145,7 +145,7 @@ static class Config {
145145
@RestController
146146
static class ContextPathController {
147147

148-
@RequestMapping
148+
@RequestMapping("/test")
149149
public String contextPath(HttpServletRequest request) {
150150
return EXPECTED_BODY;
151151
}

spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/RedirectTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public String getPerson(@PathVariable String name, Model model) {
115115
return "persons/index";
116116
}
117117

118-
@PostMapping
118+
@PostMapping("/persons")
119119
public String save(@Valid Person person, Errors errors, RedirectAttributes redirectAttrs) {
120120
if (errors.hasErrors()) {
121121
return "persons/add";

spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616

1717
package org.springframework.web.reactive.result.condition;
1818

19-
import java.util.ArrayList;
2019
import java.util.Arrays;
2120
import java.util.Collection;
21+
import java.util.Collections;
2222
import java.util.Iterator;
2323
import java.util.List;
2424
import java.util.Set;
@@ -29,6 +29,7 @@
2929
import org.springframework.lang.Nullable;
3030
import org.springframework.web.server.ServerWebExchange;
3131
import org.springframework.web.util.pattern.PathPattern;
32+
import org.springframework.web.util.pattern.PathPatternParser;
3233

3334
/**
3435
* A logical disjunction (' || ') request condition that matches a request
@@ -40,6 +41,10 @@
4041
*/
4142
public final class PatternsRequestCondition extends AbstractRequestCondition<PatternsRequestCondition> {
4243

44+
private static final SortedSet<PathPattern> EMPTY_PATTERNS =
45+
new TreeSet<>(Collections.singleton(new PathPatternParser().parse("")));
46+
47+
4348
private final SortedSet<PathPattern> patterns;
4449

4550

@@ -55,7 +60,7 @@ public PatternsRequestCondition(PathPattern... patterns) {
5560
* Creates a new instance with the given URL patterns.
5661
*/
5762
public PatternsRequestCondition(List<PathPattern> patterns) {
58-
this(new TreeSet<>(patterns));
63+
this(patterns.isEmpty() ? EMPTY_PATTERNS : new TreeSet<>(patterns));
5964
}
6065

6166

@@ -89,19 +94,23 @@ protected String getToStringInfix() {
8994
*/
9095
@Override
9196
public PatternsRequestCondition combine(PatternsRequestCondition other) {
92-
List<PathPattern> combined = new ArrayList<>();
97+
SortedSet<PathPattern> combined;
9398
if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) {
99+
combined = new TreeSet<>();
94100
for (PathPattern pattern1 : this.patterns) {
95101
for (PathPattern pattern2 : other.patterns) {
96102
combined.add(pattern1.combine(pattern2));
97103
}
98104
}
99105
}
100106
else if (!this.patterns.isEmpty()) {
101-
combined.addAll(this.patterns);
107+
combined = this.patterns;
102108
}
103109
else if (!other.patterns.isEmpty()) {
104-
combined.addAll(other.patterns);
110+
combined = other.patterns;
111+
}
112+
else {
113+
combined = EMPTY_PATTERNS;
105114
}
106115
return new PatternsRequestCondition(combined);
107116
}

spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,19 @@ public void matchPatternContainsExtension() throws Exception {
140140
assertNull(match);
141141
}
142142

143+
@Test // gh-22543
144+
public void matchWithEmptyPatterns() {
145+
PatternsRequestCondition condition = new PatternsRequestCondition();
146+
assertEquals(new PatternsRequestCondition(this.parser.parse("")), condition);
147+
assertNotNull(condition.getMatchingCondition(MockServerWebExchange.from(get(""))));
148+
assertNull(condition.getMatchingCondition(MockServerWebExchange.from(get("/anything"))));
149+
150+
condition = condition.combine(new PatternsRequestCondition());
151+
assertEquals(new PatternsRequestCondition(this.parser.parse("")), condition);
152+
assertNotNull(condition.getMatchingCondition(MockServerWebExchange.from(get(""))));
153+
assertNull(condition.getMatchingCondition(MockServerWebExchange.from(get("/anything"))));
154+
}
155+
143156
@Test
144157
public void compareToConsistentWithEquals() throws Exception {
145158
PatternsRequestCondition c1 = createPatternsCondition("/foo*");

spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.web.reactive.result.method.RequestMappingInfo;
3535
import org.springframework.web.server.ServerWebExchange;
3636
import org.springframework.web.util.pattern.PathPattern;
37+
import org.springframework.web.util.pattern.PathPatternParser;
3738
import org.springframework.web.util.pattern.PatternParseException;
3839

3940
import static java.util.Arrays.asList;
@@ -61,7 +62,8 @@ public class RequestMappingInfoTests {
6162
public void createEmpty() {
6263
RequestMappingInfo info = paths().build();
6364

64-
assertEquals(0, info.getPatternsCondition().getPatterns().size());
65+
PathPattern emptyPattern = (new PathPatternParser()).parse("");
66+
assertEquals(Collections.singleton(emptyPattern), info.getPatternsCondition().getPatterns());
6567
assertEquals(0, info.getMethodsCondition().getMethods().size());
6668
assertEquals(true, info.getConsumesCondition().isEmpty());
6769
assertEquals(true, info.getProducesCondition().isEmpty());

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,14 +253,12 @@ public void handleMatchBestMatchingPatternAttribute() {
253253
assertSame(handlerMethod, mapped);
254254
}
255255

256-
@Test
256+
@Test // gh-22543
257257
public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() {
258-
RequestMappingInfo key = paths().build();
259-
ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2"));
260-
this.handlerMapping.handleMatch(key, handlerMethod, exchange);
261-
262-
PathPattern bestMatch = (PathPattern) exchange.getAttributes().get(BEST_MATCHING_PATTERN_ATTRIBUTE);
263-
assertEquals("/1/2", bestMatch.getPatternString());
258+
ServerWebExchange exchange = MockServerWebExchange.from(get(""));
259+
this.handlerMapping.handleMatch(paths().build(), handlerMethod, exchange);
260+
PathPattern pattern = (PathPattern) exchange.getAttributes().get(BEST_MATCHING_PATTERN_ATTRIBUTE);
261+
assertEquals("", pattern.getPatternString());
264262
}
265263

266264
@Test

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ private PatternsRequestCondition(Set<String> patterns, PatternsRequestCondition
135135

136136

137137
private static Set<String> prependLeadingSlash(Collection<String> patterns) {
138+
if (patterns.isEmpty()) {
139+
return Collections.singleton("");
140+
}
138141
Set<String> result = new LinkedHashSet<>(patterns.size());
139142
for (String pattern : patterns) {
140143
if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) {

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,7 @@ public void matchSuffixPattern() {
115115
assertEquals("/{foo}", match.getPatterns().iterator().next());
116116
}
117117

118-
// SPR-8410
119-
120-
@Test
118+
@Test // SPR-8410
121119
public void matchSuffixPatternUsingFileExtensions() {
122120
String[] patterns = new String[] {"/jobs/{jobName}"};
123121
List<String> extensions = Arrays.asList("json");
@@ -183,6 +181,19 @@ public void matchPatternContainsExtension() {
183181
assertNull(match);
184182
}
185183

184+
@Test // gh-22543
185+
public void matchWithEmptyPatterns() {
186+
PatternsRequestCondition condition = new PatternsRequestCondition();
187+
assertEquals(new PatternsRequestCondition(""), condition);
188+
assertNotNull(condition.getMatchingCondition(new MockHttpServletRequest("GET", "")));
189+
assertNull(condition.getMatchingCondition(new MockHttpServletRequest("GET", "/anything")));
190+
191+
condition = condition.combine(new PatternsRequestCondition());
192+
assertEquals(new PatternsRequestCondition(""), condition);
193+
assertNotNull(condition.getMatchingCondition(new MockHttpServletRequest("GET", "")));
194+
assertNull(condition.getMatchingCondition(new MockHttpServletRequest("GET", "/anything")));
195+
}
196+
186197
@Test
187198
public void compareEqualPatterns() {
188199
PatternsRequestCondition c1 = new PatternsRequestCondition("/foo*");

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,12 @@ public void handleMatchBestMatchingPatternAttribute() {
295295
assertEquals("/{path1}/2", request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE));
296296
}
297297

298-
@Test
298+
@Test // gh-22543
299299
public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() {
300-
RequestMappingInfo key = RequestMappingInfo.paths().build();
301-
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2");
302-
303-
this.handlerMapping.handleMatch(key, "/1/2", request);
304-
305-
assertEquals("/1/2", request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE));
300+
String path = "";
301+
MockHttpServletRequest request = new MockHttpServletRequest("GET", path);
302+
this.handlerMapping.handleMatch(RequestMappingInfo.paths().build(), path, request);
303+
assertEquals(path, request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE));
306304
}
307305

308306
@Test

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class RequestMappingInfoTests {
4949
public void createEmpty() {
5050
RequestMappingInfo info = paths().build();
5151

52-
assertEquals(0, info.getPatternsCondition().getPatterns().size());
52+
assertEquals(Collections.singleton(""), info.getPatternsCondition().getPatterns()); // gh-22543
5353
assertEquals(0, info.getMethodsCondition().getMethods().size());
5454
assertEquals(true, info.getConsumesCondition().isEmpty());
5555
assertEquals(true, info.getProducesCondition().isEmpty());

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,13 +796,19 @@ public void equivalentMappingsWithSameMethodName() {
796796
.hasMessageContaining("Ambiguous mapping");
797797
}
798798

799-
@Test
799+
@Test // gh-22543
800800
public void unmappedPathMapping() throws Exception {
801801
initServletWithControllers(UnmappedPathController.class);
802802

803803
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bogus-unmapped");
804804
MockHttpServletResponse response = new MockHttpServletResponse();
805805
getServlet().service(request, response);
806+
assertEquals(404, response.getStatus());
807+
808+
request = new MockHttpServletRequest("GET", "");
809+
response = new MockHttpServletResponse();
810+
getServlet().service(request, response);
811+
assertEquals(200, response.getStatus());
806812
assertEquals("get", response.getContentAsString());
807813
}
808814

0 commit comments

Comments
 (0)