Skip to content

Commit 6a20987

Browse files
bclozelbclozel
committed
Add missing URI template variables in RedirectViews
Prior to this commit, the URL handler mapping would expose the matching pattern, the path within mapping and matching URI variables as request attributes. This was the case when the mapping would use the `AntPathMatcher` as matching infrastructure, but not when using the `PathPattern` variant. In this case, the map of URI variables would be `null`. This could throw `IllegalArgumentException` when `RedirectView` instances were relying on the presence of specific variables. This commit ensures that URI variables are also extracted when the `PathPatternParser` is used. Fixes gh-33422
1 parent 2ae62de commit 6a20987

File tree

2 files changed

+143
-92
lines changed

2 files changed

+143
-92
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ protected Object lookupHandler(
219219
validateHandler(handler, request);
220220
String pathWithinMapping = pattern.extractPathWithinPattern(path.pathWithinApplication()).value();
221221
pathWithinMapping = UrlPathHelper.defaultInstance.removeSemicolonContent(pathWithinMapping);
222-
return buildPathExposingHandler(handler, pattern.getPatternString(), pathWithinMapping, null);
222+
PathPattern.PathMatchInfo pathMatchInfo = pattern.matchAndExtract(path);
223+
Map<String, String> uriVariables = (pathMatchInfo != null ? pathMatchInfo.getUriVariables(): null);
224+
return buildPathExposingHandler(handler, pattern.getPatternString(), pathWithinMapping, uriVariables);
223225
}
224226

225227
/**

‎spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java

+140-91
Original file line numberDiff line numberDiff line change
@@ -17,58 +17,48 @@
1717
package org.springframework.web.servlet.handler;
1818

1919
import java.util.Collections;
20+
import java.util.Map;
21+
import java.util.stream.Stream;
2022

2123
import org.junit.jupiter.api.Test;
2224
import org.junit.jupiter.params.ParameterizedTest;
23-
import org.junit.jupiter.params.provider.ValueSource;
25+
import org.junit.jupiter.params.provider.Arguments;
26+
import org.junit.jupiter.params.provider.MethodSource;
2427

25-
import org.springframework.beans.FatalBeanException;
2628
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
2729
import org.springframework.context.support.StaticApplicationContext;
28-
import org.springframework.web.context.support.XmlWebApplicationContext;
30+
import org.springframework.http.HttpStatus;
31+
import org.springframework.util.Assert;
2932
import org.springframework.web.servlet.HandlerExecutionChain;
3033
import org.springframework.web.servlet.HandlerInterceptor;
3134
import org.springframework.web.servlet.HandlerMapping;
35+
import org.springframework.web.servlet.mvc.ParameterizableViewController;
36+
import org.springframework.web.servlet.view.RedirectView;
3237
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
33-
import org.springframework.web.testfixture.servlet.MockServletContext;
3438
import org.springframework.web.util.UrlPathHelper;
3539
import org.springframework.web.util.WebUtils;
3640

3741
import static org.assertj.core.api.Assertions.assertThat;
38-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
42+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3943
import static org.springframework.web.servlet.HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE;
4044
import static org.springframework.web.servlet.HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE;
45+
import static org.springframework.web.servlet.HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
4146

4247
/**
43-
* @author Rod Johnson
44-
* @author Juergen Hoeller
48+
* Tests for {@link SimpleUrlHandlerMapping}.
49+
* @author Brian Clozel
4550
*/
4651
class SimpleUrlHandlerMappingTests {
4752

4853
@Test
49-
void handlerBeanNotFound() {
50-
MockServletContext sc = new MockServletContext("");
51-
XmlWebApplicationContext root = new XmlWebApplicationContext();
52-
root.setServletContext(sc);
53-
root.setConfigLocations("/org/springframework/web/servlet/handler/map1.xml");
54-
root.refresh();
55-
56-
XmlWebApplicationContext wac = new XmlWebApplicationContext();
57-
wac.setParent(root);
58-
wac.setServletContext(sc);
59-
wac.setNamespace("map2err");
60-
wac.setConfigLocations("/org/springframework/web/servlet/handler/map2err.xml");
61-
assertThatExceptionOfType(FatalBeanException.class)
62-
.isThrownBy(wac::refresh)
63-
.withCauseInstanceOf(NoSuchBeanDefinitionException.class)
64-
.satisfies(ex -> {
65-
NoSuchBeanDefinitionException cause = (NoSuchBeanDefinitionException) ex.getCause();
66-
assertThat(cause.getBeanName()).isEqualTo("mainControlle");
67-
});
54+
void shouldFailWhenHandlerBeanNotFound() {
55+
SimpleUrlHandlerMapping handlerMapping = new SimpleUrlHandlerMapping(Map.of("/welcome.html", "mainController"));
56+
assertThatThrownBy(() -> handlerMapping.setApplicationContext(new StaticApplicationContext()))
57+
.isInstanceOf(NoSuchBeanDefinitionException.class);
6858
}
6959

7060
@Test
71-
void testNewlineInRequest() throws Exception {
61+
void newlineInRequestShouldMatch() throws Exception {
7262
Object controller = new Object();
7363
UrlPathHelper urlPathHelper = new UrlPathHelper();
7464
urlPathHelper.setUrlDecode(false);
@@ -84,84 +74,143 @@ void testNewlineInRequest() throws Exception {
8474
}
8575

8676
@ParameterizedTest
87-
@ValueSource(strings = {"urlMapping", "urlMappingWithProps", "urlMappingWithPathPatterns"})
88-
void checkMappings(String beanName) throws Exception {
89-
MockServletContext sc = new MockServletContext("");
90-
XmlWebApplicationContext wac = new XmlWebApplicationContext();
91-
wac.setServletContext(sc);
92-
wac.setConfigLocations("/org/springframework/web/servlet/handler/map2.xml");
93-
wac.refresh();
94-
Object bean = wac.getBean("mainController");
95-
Object otherBean = wac.getBean("otherController");
96-
Object defaultBean = wac.getBean("starController");
97-
HandlerMapping hm = (HandlerMapping) wac.getBean(beanName);
98-
wac.close();
99-
100-
boolean usePathPatterns = (((AbstractHandlerMapping) hm).getPatternParser() != null);
77+
@MethodSource("handlerMappings")
78+
void resolveFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
79+
StaticApplicationContext applicationContext = new StaticApplicationContext();
80+
applicationContext.registerSingleton("mainController", Object.class);
81+
Object mainController = applicationContext.getBean("mainController");
82+
handlerMapping.setUrlMap(Map.of("/welcome.html", "mainController"));
83+
handlerMapping.setApplicationContext(applicationContext);
84+
85+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
10186
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/welcome.html", usePathPatterns);
102-
HandlerExecutionChain chain = getHandler(hm, request);
103-
assertThat(chain.getHandler()).isSameAs(bean);
104-
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.html");
105-
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(bean);
87+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
10688

107-
request = PathPatternsTestUtils.initRequest("GET", "/welcome.x;jsessionid=123", usePathPatterns);
108-
chain = getHandler(hm, request);
109-
assertThat(chain.getHandler()).isSameAs(otherBean);
110-
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("welcome.x");
111-
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(otherBean);
89+
assertThat(chain.getHandler()).isSameAs(mainController);
90+
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.html");
91+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
92+
}
11293

113-
request = PathPatternsTestUtils.initRequest("GET", "/app", "/welcome.x", usePathPatterns);
114-
chain = getHandler(hm, request);
115-
assertThat(chain.getHandler()).isSameAs(otherBean);
94+
@ParameterizedTest
95+
@MethodSource("handlerMappings")
96+
void resolvePatternFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
97+
StaticApplicationContext applicationContext = new StaticApplicationContext();
98+
applicationContext.registerSingleton("mainController", Object.class);
99+
Object mainController = applicationContext.getBean("mainController");
100+
handlerMapping.setUrlMap(Map.of("/welcome*", "mainController"));
101+
handlerMapping.setApplicationContext(applicationContext);
102+
103+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
104+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/welcome.x", usePathPatterns);
105+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
106+
107+
assertThat(chain.getHandler()).isSameAs(mainController);
116108
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("welcome.x");
117-
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(otherBean);
118-
119-
request = PathPatternsTestUtils.initRequest("GET", "/", usePathPatterns);
120-
request.setServletPath("/welcome.html");
121-
chain = getHandler(hm, request);
122-
assertThat(chain.getHandler()).isSameAs(bean);
123-
124-
request = PathPatternsTestUtils.initRequest("GET", "/app", "/welcome.html", usePathPatterns);
125-
chain = getHandler(hm, request);
126-
assertThat(chain.getHandler()).isSameAs(bean);
127-
128-
129-
request = PathPatternsTestUtils.initRequest("GET", "/show.html", usePathPatterns);
130-
chain = getHandler(hm, request);
131-
assertThat(chain.getHandler()).isSameAs(bean);
132-
133-
request = PathPatternsTestUtils.initRequest("GET", "/bookseats.html", usePathPatterns);
134-
chain = getHandler(hm, request);
135-
assertThat(chain.getHandler()).isSameAs(bean);
109+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
110+
}
136111

137-
request = PathPatternsTestUtils.initRequest("GET", null, "/original-welcome.html", usePathPatterns,
138-
req -> req.setAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE, "/welcome.html"));
139-
chain = getHandler(hm, request);
140-
assertThat(chain.getHandler()).isSameAs(bean);
112+
@ParameterizedTest
113+
@MethodSource("handlerMappings")
114+
void resolvePathWithParamFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
115+
StaticApplicationContext applicationContext = new StaticApplicationContext();
116+
applicationContext.registerSingleton("mainController", Object.class);
117+
Object mainController = applicationContext.getBean("mainController");
118+
handlerMapping.setUrlMap(Map.of("/welcome.x", "mainController"));
119+
handlerMapping.setApplicationContext(applicationContext);
120+
121+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
122+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/welcome.x;jsessionid=123", usePathPatterns);
123+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
124+
125+
assertThat(chain.getHandler()).isSameAs(mainController);
126+
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.x");
127+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
128+
}
141129

142-
request = PathPatternsTestUtils.initRequest("GET", null, "/original-show.html", usePathPatterns,
143-
req -> req.setAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE, "/show.html"));
144-
chain = getHandler(hm, request);
145-
assertThat(chain.getHandler()).isSameAs(bean);
130+
@ParameterizedTest
131+
@MethodSource("handlerMappings")
132+
void resolvePathWithContextFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
133+
StaticApplicationContext applicationContext = new StaticApplicationContext();
134+
applicationContext.registerSingleton("mainController", Object.class);
135+
Object mainController = applicationContext.getBean("mainController");
136+
handlerMapping.setUrlMap(Map.of("/welcome.x", "mainController"));
137+
handlerMapping.setApplicationContext(applicationContext);
138+
139+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
140+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/app", "/welcome.x", usePathPatterns);
141+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
142+
143+
assertThat(chain.getHandler()).isSameAs(mainController);
144+
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.x");
145+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
146+
}
146147

147-
request = PathPatternsTestUtils.initRequest("GET", null, "/original-bookseats.html", usePathPatterns,
148-
req -> req.setAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE, "/bookseats.html"));
149-
chain = getHandler(hm, request);
150-
assertThat(chain.getHandler()).isSameAs(bean);
148+
@ParameterizedTest
149+
@MethodSource("handlerMappings")
150+
void resolvePathWithIncludeFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
151+
StaticApplicationContext applicationContext = new StaticApplicationContext();
152+
applicationContext.registerSingleton("mainController", Object.class);
153+
Object mainController = applicationContext.getBean("mainController");
154+
handlerMapping.setUrlMap(Map.of("/welcome.html", "mainController"));
155+
handlerMapping.setApplicationContext(applicationContext);
156+
157+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
158+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/original.html", usePathPatterns);
159+
request.setAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE, "/welcome.html");
160+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
161+
162+
assertThat(chain.getHandler()).isSameAs(mainController);
163+
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.html");
164+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
165+
}
151166

152-
request = PathPatternsTestUtils.initRequest("GET", "/", usePathPatterns);
153-
chain = getHandler(hm, request);
154-
assertThat(chain.getHandler()).isSameAs(bean);
167+
@ParameterizedTest
168+
@MethodSource("handlerMappings")
169+
void resolveDefaultPathFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
170+
StaticApplicationContext applicationContext = new StaticApplicationContext();
171+
applicationContext.registerSingleton("mainController", Object.class);
172+
Object mainController = applicationContext.getBean("mainController");
173+
handlerMapping.setDefaultHandler(mainController);
174+
handlerMapping.setApplicationContext(applicationContext);
175+
176+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
177+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", usePathPatterns);
178+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
179+
180+
assertThat(chain.getHandler()).isSameAs(mainController);
155181
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/");
182+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(mainController);
183+
}
184+
185+
@ParameterizedTest
186+
@MethodSource("handlerMappings")
187+
void resolveParameterizedControllerFromMap(SimpleUrlHandlerMapping handlerMapping) throws Exception {
188+
ParameterizableViewController viewController = new ParameterizableViewController();
189+
viewController.setView(new RedirectView("/after/{variable}"));
190+
viewController.setStatusCode(HttpStatus.PERMANENT_REDIRECT);
191+
handlerMapping.setUrlMap(Map.of("/before/{variable}", viewController));
192+
handlerMapping.setApplicationContext(new StaticApplicationContext());
193+
194+
boolean usePathPatterns = handlerMapping.getPatternParser() != null;
195+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/before/test", usePathPatterns);
196+
HandlerExecutionChain chain = getHandler(handlerMapping, request);
197+
198+
assertThat(chain.getHandler()).isSameAs(viewController);
199+
Map<String, String> variables = (Map<String, String>) request.getAttribute(URI_TEMPLATE_VARIABLES_ATTRIBUTE);
200+
assertThat(variables).containsEntry("variable", "test");
201+
assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(viewController);
202+
}
156203

157-
request = PathPatternsTestUtils.initRequest("GET", "/somePath", usePathPatterns);
158-
chain = getHandler(hm, request);
159-
assertThat(chain.getHandler()).as("Handler is correct bean").isSameAs(defaultBean);
160-
assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/somePath");
204+
static Stream<Arguments> handlerMappings() {
205+
SimpleUrlHandlerMapping defaultConfig = new SimpleUrlHandlerMapping();
206+
SimpleUrlHandlerMapping antPatternConfig = new SimpleUrlHandlerMapping();
207+
antPatternConfig.setPatternParser(null);
208+
return Stream.of(Arguments.of(defaultConfig, "with PathPattern"), Arguments.of(antPatternConfig, "with AntPathMatcher"));
161209
}
162210

163211
private HandlerExecutionChain getHandler(HandlerMapping mapping, MockHttpServletRequest request) throws Exception {
164212
HandlerExecutionChain chain = mapping.getHandler(request);
213+
Assert.notNull(chain, "No handler found for request: " + request.getRequestURI());
165214
for (HandlerInterceptor interceptor : chain.getInterceptorList()) {
166215
interceptor.preHandle(request, null, chain.getHandler());
167216
}

0 commit comments

Comments
 (0)