Skip to content

Commit 69bbdce

Browse files
rstoyanchevrstoyanchev
committed
HandlerMappingIntrospector ensures initialized RequestPath
Closes gh-26814
1 parent 0f31830 commit 69bbdce

File tree

4 files changed

+117
-44
lines changed

4 files changed

+117
-44
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -968,7 +968,9 @@ protected void doService(HttpServletRequest request, HttpServletResponse respons
968968
restoreAttributesAfterInclude(request, attributesSnapshot);
969969
}
970970
}
971-
ServletRequestPathUtils.setParsedRequestPath(previousRequestPath, request);
971+
if (this.parseRequestPath) {
972+
ServletRequestPathUtils.setParsedRequestPath(previousRequestPath, request);
973+
}
972974
}
973975
}
974976

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

Lines changed: 76 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 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.
@@ -23,6 +23,7 @@
2323
import java.util.Map;
2424
import java.util.Properties;
2525
import java.util.concurrent.ConcurrentHashMap;
26+
import java.util.function.BiFunction;
2627
import java.util.stream.Collectors;
2728

2829
import javax.servlet.http.HttpServletRequest;
@@ -36,6 +37,7 @@
3637
import org.springframework.core.io.ClassPathResource;
3738
import org.springframework.core.io.Resource;
3839
import org.springframework.core.io.support.PropertiesLoaderUtils;
40+
import org.springframework.http.server.RequestPath;
3941
import org.springframework.lang.Nullable;
4042
import org.springframework.util.Assert;
4143
import org.springframework.util.ClassUtils;
@@ -46,6 +48,7 @@
4648
import org.springframework.web.servlet.HandlerExecutionChain;
4749
import org.springframework.web.servlet.HandlerInterceptor;
4850
import org.springframework.web.servlet.HandlerMapping;
51+
import org.springframework.web.util.ServletRequestPathUtils;
4952
import org.springframework.web.util.UrlPathHelper;
5053

5154
/**
@@ -79,8 +82,7 @@ public class HandlerMappingIntrospector
7982
private List<HandlerMapping> handlerMappings;
8083

8184
@Nullable
82-
private Map<HandlerMapping, MatchableHandlerMapping> pathPatternMatchableHandlerMappings =
83-
new ConcurrentHashMap<>();
85+
private Map<HandlerMapping, PathPatternMatchableHandlerMapping> pathPatternHandlerMappings = new ConcurrentHashMap<>();
8486

8587

8688
/**
@@ -119,7 +121,7 @@ public void afterPropertiesSet() {
119121
if (this.handlerMappings == null) {
120122
Assert.notNull(this.applicationContext, "No ApplicationContext");
121123
this.handlerMappings = initHandlerMappings(this.applicationContext);
122-
this.pathPatternMatchableHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings);
124+
this.pathPatternHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings);
123125
}
124126
}
125127

@@ -136,51 +138,90 @@ public void afterPropertiesSet() {
136138
*/
137139
@Nullable
138140
public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest request) throws Exception {
139-
Assert.notNull(this.handlerMappings, "Handler mappings not initialized");
140-
Assert.notNull(this.pathPatternMatchableHandlerMappings, "Handler mappings with PathPatterns not initialized");
141-
HttpServletRequest wrapper = new RequestAttributeChangeIgnoringWrapper(request);
142-
for (HandlerMapping handlerMapping : this.handlerMappings) {
143-
Object handler = handlerMapping.getHandler(wrapper);
144-
if (handler == null) {
145-
continue;
146-
}
147-
if (handlerMapping instanceof MatchableHandlerMapping) {
148-
return this.pathPatternMatchableHandlerMappings.getOrDefault(
149-
handlerMapping, (MatchableHandlerMapping) handlerMapping);
141+
HttpServletRequest wrappedRequest = new RequestAttributeChangeIgnoringWrapper(request);
142+
return doWithMatchingMapping(wrappedRequest, false, (matchedMapping, executionChain) -> {
143+
if (matchedMapping instanceof MatchableHandlerMapping) {
144+
PathPatternMatchableHandlerMapping mapping = this.pathPatternHandlerMappings.get(matchedMapping);
145+
if (mapping != null) {
146+
RequestPath requestPath = ServletRequestPathUtils.getParsedRequestPath(request);
147+
return mapping.decorate(requestPath);
148+
}
149+
else {
150+
return (MatchableHandlerMapping) matchedMapping;
151+
}
150152
}
151153
throw new IllegalStateException("HandlerMapping is not a MatchableHandlerMapping");
152-
}
153-
return null;
154+
});
154155
}
155156

156157
@Override
157158
@Nullable
158159
public CorsConfiguration getCorsConfiguration(HttpServletRequest request) {
159-
Assert.notNull(this.handlerMappings, "Handler mappings not initialized");
160-
RequestAttributeChangeIgnoringWrapper wrapper = new RequestAttributeChangeIgnoringWrapper(request);
161-
for (HandlerMapping handlerMapping : this.handlerMappings) {
162-
HandlerExecutionChain handler = null;
163-
try {
164-
handler = handlerMapping.getHandler(wrapper);
165-
}
166-
catch (Exception ex) {
167-
// Ignore
160+
RequestAttributeChangeIgnoringWrapper wrappedRequest = new RequestAttributeChangeIgnoringWrapper(request);
161+
return doWithMatchingMappingIgnoringException(wrappedRequest, (handlerMapping, executionChain) -> {
162+
for (HandlerInterceptor interceptor : executionChain.getInterceptorList()) {
163+
if (interceptor instanceof CorsConfigurationSource) {
164+
return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrappedRequest);
165+
}
168166
}
169-
if (handler == null) {
170-
continue;
167+
if (executionChain.getHandler() instanceof CorsConfigurationSource) {
168+
return ((CorsConfigurationSource) executionChain.getHandler()).getCorsConfiguration(wrappedRequest);
171169
}
172-
for (HandlerInterceptor interceptor : handler.getInterceptorList()) {
173-
if (interceptor instanceof CorsConfigurationSource) {
174-
return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrapper);
170+
return null;
171+
});
172+
}
173+
174+
@Nullable
175+
private <T> T doWithMatchingMapping(
176+
HttpServletRequest request, boolean ignoreException,
177+
BiFunction<HandlerMapping, HandlerExecutionChain, T> matchHandler) throws Exception {
178+
179+
Assert.notNull(this.handlerMappings, "Handler mappings not initialized");
180+
Assert.notNull(this.pathPatternHandlerMappings, "Handler mappings with PathPatterns not initialized");
181+
182+
boolean parseRequestPath = !this.pathPatternHandlerMappings.isEmpty();
183+
RequestPath previousPath = null;
184+
if (parseRequestPath) {
185+
previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
186+
ServletRequestPathUtils.parseAndCache(request);
187+
}
188+
try {
189+
for (HandlerMapping handlerMapping : this.handlerMappings) {
190+
HandlerExecutionChain chain = null;
191+
try {
192+
chain = handlerMapping.getHandler(request);
193+
}
194+
catch (Exception ex) {
195+
if (!ignoreException) {
196+
throw ex;
197+
}
175198
}
199+
if (chain == null) {
200+
continue;
201+
}
202+
return matchHandler.apply(handlerMapping, chain);
176203
}
177-
if (handler.getHandler() instanceof CorsConfigurationSource) {
178-
return ((CorsConfigurationSource) handler.getHandler()).getCorsConfiguration(wrapper);
204+
}
205+
finally {
206+
if (parseRequestPath) {
207+
ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
179208
}
180209
}
181210
return null;
182211
}
183212

213+
@Nullable
214+
private <T> T doWithMatchingMappingIgnoringException(
215+
HttpServletRequest request, BiFunction<HandlerMapping, HandlerExecutionChain, T> matchHandler) {
216+
217+
try {
218+
return doWithMatchingMapping(request, true, matchHandler);
219+
}
220+
catch (Exception ex) {
221+
throw new IllegalStateException("HandlerMapping exception not suppressed", ex);
222+
}
223+
}
224+
184225

185226
private static List<HandlerMapping> initHandlerMappings(ApplicationContext applicationContext) {
186227
Map<String, HandlerMapping> beans = BeanFactoryUtils.beansOfTypeIncludingAncestors(
@@ -219,7 +260,7 @@ private static List<HandlerMapping> initFallback(ApplicationContext applicationC
219260
return result;
220261
}
221262

222-
private static Map<HandlerMapping, MatchableHandlerMapping> initPathPatternMatchableHandlerMappings(
263+
private static Map<HandlerMapping, PathPatternMatchableHandlerMapping> initPathPatternMatchableHandlerMappings(
223264
List<HandlerMapping> mappings) {
224265

225266
return mappings.stream()
@@ -241,8 +282,7 @@ private static class RequestAttributeChangeIgnoringWrapper extends HttpServletRe
241282

242283
@Override
243284
public void setAttribute(String name, Object value) {
244-
// Allow UrlPathHelper-resolved lookupPath to be saved for efficiency
245-
if (name.equals(UrlPathHelper.PATH_ATTRIBUTE)) {
285+
if (name.equals(ServletRequestPathUtils.PATH_ATTRIBUTE) || name.equals(UrlPathHelper.PATH_ATTRIBUTE)) {
246286
super.setAttribute(name, value);
247287
}
248288
}

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 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.
@@ -21,6 +21,7 @@
2121
import javax.servlet.http.HttpServletRequest;
2222

2323
import org.springframework.http.server.PathContainer;
24+
import org.springframework.http.server.RequestPath;
2425
import org.springframework.lang.Nullable;
2526
import org.springframework.util.Assert;
2627
import org.springframework.web.servlet.HandlerExecutionChain;
@@ -70,4 +71,39 @@ public RequestMatchResult match(HttpServletRequest request, String pattern) {
7071
public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception {
7172
return this.delegate.getHandler(request);
7273
}
74+
75+
MatchableHandlerMapping decorate(RequestPath requestPath) {
76+
return new MatchableHandlerMapping() {
77+
78+
@Nullable
79+
@Override
80+
public RequestMatchResult match(HttpServletRequest request, String pattern) {
81+
RequestPath previousPath = setRequestPathAttribute(request);
82+
try {
83+
return PathPatternMatchableHandlerMapping.this.match(request, pattern);
84+
}
85+
finally {
86+
ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
87+
}
88+
}
89+
90+
@Nullable
91+
@Override
92+
public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception {
93+
RequestPath previousPath = setRequestPathAttribute(request);
94+
try {
95+
return PathPatternMatchableHandlerMapping.this.getHandler(request);
96+
}
97+
finally {
98+
ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
99+
}
100+
}
101+
102+
private RequestPath setRequestPathAttribute(HttpServletRequest request) {
103+
RequestPath previous = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
104+
ServletRequestPathUtils.setParsedRequestPath(requestPath, request);
105+
return previous;
106+
}
107+
};
108+
}
73109
}

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,11 @@ void getMatchable(boolean usePathPatterns) throws Exception {
127127
context.refresh();
128128

129129
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123");
130-
131-
// Initialize the RequestPath. At runtime, ServletRequestPathFilter is expected to do that.
132-
if (usePathPatterns) {
133-
ServletRequestPathUtils.parseAndCache(request);
134-
}
135-
136130
MatchableHandlerMapping mapping = initIntrospector(context).getMatchableHandlerMapping(request);
137131

138132
assertThat(mapping).isNotNull();
139133
assertThat(request.getAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE)).as("Attribute changes not ignored").isNull();
134+
assertThat(request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE)).as("Parsed path not cleaned").isNull();
140135

141136
assertThat(mapping.match(request, "/p*/*")).isNotNull();
142137
assertThat(mapping.match(request, "/b*/*")).isNull();

0 commit comments

Comments
 (0)