Skip to content

Commit 25cfbe8

Browse files
jzheauxjzheaux
committed
Use PathPatternRequestMatcher by Default
1 parent 772f0cc commit 25cfbe8

File tree

9 files changed

+144
-38
lines changed

9 files changed

+144
-38
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.springframework.security.config.annotation.web.ServletRegistrationsSupport.RegistrationMapping;
4444
import org.springframework.security.config.annotation.web.configurers.AbstractConfigAttributeRequestMatcherRegistry;
4545
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
46+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
4647
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
4748
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
4849
import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher;
@@ -53,8 +54,8 @@
5354
import org.springframework.util.Assert;
5455
import org.springframework.util.ClassUtils;
5556
import org.springframework.web.context.WebApplicationContext;
56-
import org.springframework.web.servlet.DispatcherServlet;
5757
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
58+
import org.springframework.web.util.pattern.PathPatternParser;
5859

5960
/**
6061
* A base class for registering {@link RequestMatcher}'s. For example, it might allow for
@@ -234,7 +235,7 @@ private boolean anyPathsDontStartWithLeadingSlash(String... patterns) {
234235
return false;
235236
}
236237

237-
private RequestMatcher resolve(AntPathRequestMatcher ant, MvcRequestMatcher mvc, ServletContext servletContext) {
238+
private RequestMatcher resolve(AntPathRequestMatcher ant, RequestMatcher mvc, ServletContext servletContext) {
238239
ServletRegistrationsSupport registrations = new ServletRegistrationsSupport(servletContext);
239240
Collection<RegistrationMapping> mappings = registrations.mappings();
240241
if (mappings.isEmpty()) {
@@ -280,10 +281,10 @@ private static String computeErrorMessage(Collection<? extends ServletRegistrati
280281

281282
/**
282283
* <p>
283-
* If the {@link HandlerMappingIntrospector} is available in the classpath, maps to an
284-
* {@link MvcRequestMatcher} that does not care which {@link HttpMethod} is used. This
285-
* matcher will use the same rules that Spring MVC uses for matching. For example,
286-
* often times a mapping of the path "/path" will match on "/path", "/path/",
284+
* If the {@link HandlerMappingIntrospector} is available in the classpath, maps to a
285+
* {@link PathPatternRequestMatcher} that does not care which {@link HttpMethod} is
286+
* used. This matcher will use the same rules that Spring MVC uses for matching. For
287+
* example, often times a mapping of the path "/path" will match on "/path", "/path/",
287288
* "/path.html", etc. If the {@link HandlerMappingIntrospector} is not available, maps
288289
* to an {@link AntPathRequestMatcher}.
289290
* </p>
@@ -408,8 +409,26 @@ class DefaultRequestMatcherBuilder implements RequestMatcherBuilder {
408409

409410
@Override
410411
public RequestMatcher pattern(HttpMethod method, String pattern) {
412+
Assert.state(!AbstractRequestMatcherRegistry.this.anyRequestConfigured,
413+
"Can't configure mvcMatchers after anyRequest");
411414
AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null);
412-
MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0);
415+
RequestMatcher mvc;
416+
if (!AbstractRequestMatcherRegistry.this.context.containsBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME)) {
417+
throw new NoSuchBeanDefinitionException("A Bean named " + HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME
418+
+ " of type " + HandlerMappingIntrospector.class.getName()
419+
+ " is required to use MvcRequestMatcher. Please ensure Spring Security & Spring MVC are configured in a shared ApplicationContext.");
420+
}
421+
HandlerMappingIntrospector introspector = AbstractRequestMatcherRegistry.this.context
422+
.getBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME, HandlerMappingIntrospector.class);
423+
if (introspector.allHandlerMappingsUsePathPatternParser()) {
424+
PathPatternParser pathPatternParser = AbstractRequestMatcherRegistry.this.context
425+
.getBeanProvider(PathPatternParser.class)
426+
.getIfUnique(() -> PathPatternParser.defaultInstance);
427+
mvc = PathPatternRequestMatcher.withPathPatternParser(pathPatternParser).pattern(method, pattern);
428+
}
429+
else {
430+
mvc = createMvcMatchers(method, pattern).get(0);
431+
}
413432
return new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant);
414433
}
415434

@@ -466,13 +485,8 @@ public boolean matches(HttpServletRequest request) {
466485
ServletRegistration registration = request.getServletContext().getServletRegistration(name);
467486
Assert.notNull(registration,
468487
() -> computeErrorMessage(request.getServletContext().getServletRegistrations().values()));
469-
try {
470-
Class<?> clazz = Class.forName(registration.getClassName());
471-
return DispatcherServlet.class.isAssignableFrom(clazz);
472-
}
473-
catch (ClassNotFoundException ex) {
474-
return false;
475-
}
488+
return new RegistrationMapping(registration, request.getHttpServletMapping().getPattern())
489+
.isDispatcherServlet();
476490
}
477491

478492
}
@@ -481,15 +495,15 @@ static class DispatcherServletDelegatingRequestMatcher implements RequestMatcher
481495

482496
private final AntPathRequestMatcher ant;
483497

484-
private final MvcRequestMatcher mvc;
498+
private final RequestMatcher mvc;
485499

486500
private final RequestMatcher dispatcherServlet;
487501

488-
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, MvcRequestMatcher mvc) {
502+
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, RequestMatcher mvc) {
489503
this(ant, mvc, new OrRequestMatcher(new MockMvcRequestMatcher(), new DispatcherServletRequestMatcher()));
490504
}
491505

492-
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, MvcRequestMatcher mvc,
506+
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, RequestMatcher mvc,
493507
RequestMatcher dispatcherServlet) {
494508
this.ant = ant;
495509
this.mvc = mvc;

‎config/src/main/java/org/springframework/security/config/http/MatcherType.java‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,25 @@
1616

1717
package org.springframework.security.config.http;
1818

19+
import jakarta.servlet.http.HttpServletRequest;
1920
import org.w3c.dom.Element;
2021

22+
import org.springframework.beans.factory.FactoryBean;
2123
import org.springframework.beans.factory.config.BeanDefinition;
2224
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
2325
import org.springframework.beans.factory.support.RootBeanDefinition;
2426
import org.springframework.beans.factory.xml.ParserContext;
2527
import org.springframework.http.HttpMethod;
2628
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
29+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
2730
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
2831
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
2932
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
3033
import org.springframework.security.web.util.matcher.RequestMatcher;
3134
import org.springframework.util.ClassUtils;
3235
import org.springframework.util.StringUtils;
36+
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
37+
import org.springframework.web.util.pattern.PathPatternParser;
3338

3439
/**
3540
* Defines the {@link RequestMatcher} types supported by the namespace.
@@ -40,7 +45,7 @@
4045
public enum MatcherType {
4146

4247
ant(AntPathRequestMatcher.class), regex(RegexRequestMatcher.class), ciRegex(RegexRequestMatcher.class),
43-
mvc(MvcRequestMatcher.class);
48+
mvc(MvcRequestMatcherFactoryBean.class);
4449

4550
private static final String HANDLER_MAPPING_INTROSPECTOR = "org.springframework.web.servlet.handler.HandlerMappingIntrospector";
4651

@@ -100,4 +105,56 @@ static MatcherType fromElementOrMvc(Element elt) {
100105
return MatcherType.fromElement(elt);
101106
}
102107

108+
private static class MvcRequestMatcherFactoryBean implements FactoryBean<RequestMatcher>, RequestMatcher {
109+
110+
private final HandlerMappingIntrospector introspector;
111+
112+
private final String pattern;
113+
114+
private PathPatternParser pathPatternParser = PathPatternParser.defaultInstance;
115+
116+
private String servletPath;
117+
118+
private HttpMethod method;
119+
120+
public MvcRequestMatcherFactoryBean(HandlerMappingIntrospector introspector, String pattern) {
121+
this.introspector = introspector;
122+
this.pattern = pattern;
123+
}
124+
125+
@Override
126+
public RequestMatcher getObject() {
127+
if (this.introspector.allHandlerMappingsUsePathPatternParser()) {
128+
return PathPatternRequestMatcher.withPathPatternParser(this.pathPatternParser)
129+
.servletPath(this.servletPath)
130+
.pattern(this.method, this.pattern);
131+
}
132+
return new MvcRequestMatcher.Builder(this.introspector).servletPath(this.servletPath)
133+
.pattern(this.method, this.pattern);
134+
}
135+
136+
@Override
137+
public Class<?> getObjectType() {
138+
return RequestMatcher.class;
139+
}
140+
141+
@Override
142+
public boolean matches(HttpServletRequest request) {
143+
return getObject().matches(request);
144+
}
145+
146+
public void setPathPatternParser(PathPatternParser pathPatternParser) {
147+
this.pathPatternParser = pathPatternParser;
148+
}
149+
150+
public void setServletPath(String servletPath) {
151+
this.servletPath = servletPath;
152+
}
153+
154+
public void setMethod(HttpMethod method) {
155+
this.method = method;
156+
}
157+
158+
}
159+
103160
}

‎config/src/main/kotlin/org/springframework/security/config/annotation/web/AuthorizeHttpRequestsDsl.kt‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@ import org.springframework.security.web.access.IpAddressAuthorizationManager
3232
import org.springframework.security.web.access.intercept.AuthorizationFilter
3333
import org.springframework.security.web.access.intercept.RequestAuthorizationContext
3434
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher
35+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher
3536
import org.springframework.security.web.util.matcher.AnyRequestMatcher
3637
import org.springframework.security.web.util.matcher.RequestMatcher
3738
import org.springframework.util.ClassUtils
3839
import org.springframework.web.servlet.handler.HandlerMappingIntrospector
40+
import org.springframework.web.util.pattern.PathPatternParser
3941
import java.util.function.Supplier
4042

4143
/**
@@ -292,11 +294,20 @@ class AuthorizeHttpRequestsDsl : AbstractRequestMatcherDsl {
292294
PatternType.ANT -> requests.requestMatchers(rule.httpMethod, rule.pattern).access(rule.rule)
293295
PatternType.MVC -> {
294296
val introspector = requests.applicationContext.getBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME, HandlerMappingIntrospector::class.java)
295-
val mvcMatcher = MvcRequestMatcher.Builder(introspector)
296-
.servletPath(rule.servletPath)
297-
.pattern(rule.pattern)
298-
mvcMatcher.setMethod(rule.httpMethod)
299-
requests.requestMatchers(mvcMatcher).access(rule.rule)
297+
if (introspector.allHandlerMappingsUsePathPatternParser()) {
298+
val pathPatternParser: PathPatternParser = requests.applicationContext.getBeanProvider(PathPatternParser::class.java)
299+
.getIfUnique({PathPatternParser.defaultInstance})
300+
val mvcMatcher = PathPatternRequestMatcher.withPathPatternParser(pathPatternParser)
301+
.servletPath(rule.servletPath)
302+
.pattern(rule.httpMethod, rule.pattern)
303+
requests.requestMatchers(mvcMatcher).access(rule.rule)
304+
} else {
305+
val mvcMatcher = MvcRequestMatcher.Builder(introspector)
306+
.servletPath(rule.servletPath)
307+
.pattern(rule.pattern)
308+
mvcMatcher.setMethod(rule.httpMethod)
309+
requests.requestMatchers(mvcMatcher).access(rule.rule)
310+
}
300311
}
301312
}
302313
}

‎config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
4040
import org.springframework.security.web.util.matcher.RequestMatcher;
4141
import org.springframework.test.util.ReflectionTestUtils;
42+
import org.springframework.web.util.pattern.PathPattern;
4243

4344
import static org.assertj.core.api.Assertions.assertThat;
4445
import static org.mockito.ArgumentMatchers.any;
@@ -120,7 +121,8 @@ public void mixingPatternsAndPlaceholdersDoesntCauseOrderingIssues() {
120121

121122
private String getPattern(SecurityFilterChain chain) {
122123
RequestMatcher requestMatcher = ((DefaultSecurityFilterChain) chain).getRequestMatcher();
123-
return (String) ReflectionTestUtils.getField(requestMatcher, "pattern");
124+
Object pattern = ReflectionTestUtils.getField(requestMatcher, "pattern");
125+
return (pattern instanceof PathPattern) ? pattern.toString() : (String) pattern;
124126
}
125127

126128
private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) {

‎config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import jakarta.servlet.DispatcherType;
2323
import jakarta.servlet.Servlet;
24+
import org.jetbrains.annotations.NotNull;
2425
import org.junit.jupiter.api.BeforeEach;
2526
import org.junit.jupiter.api.Test;
2627

@@ -39,6 +40,7 @@
3940
import org.springframework.security.web.servlet.MockServletContext;
4041
import org.springframework.security.web.servlet.TestMockHttpServletMappings;
4142
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
43+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
4244
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
4345
import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher;
4446
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
@@ -49,10 +51,13 @@
4951
import org.springframework.web.context.WebApplicationContext;
5052
import org.springframework.web.servlet.DispatcherServlet;
5153
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
54+
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
5255

5356
import static org.assertj.core.api.Assertions.assertThat;
5457
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
5558
import static org.assertj.core.api.InstanceOfAssertFactories.type;
59+
import static org.mockito.ArgumentMatchers.any;
60+
import static org.mockito.ArgumentMatchers.eq;
5661
import static org.mockito.BDDMockito.given;
5762
import static org.mockito.Mockito.mock;
5863
import static org.mockito.Mockito.verify;
@@ -91,11 +96,13 @@ public void setUp() {
9196
given(this.context.getServletContext()).willReturn(MockServletContext.mvc());
9297
ObjectProvider<RequestMatcherBuilder> requestMatcherFactories = new ObjectProvider<>() {
9398
@Override
94-
public RequestMatcherBuilder getObject() throws BeansException {
99+
public @NotNull RequestMatcherBuilder getObject() throws BeansException {
95100
return AbstractRequestMatcherRegistryTests.this.matcherRegistry.new DefaultRequestMatcherBuilder();
96101
}
97102
};
98103
given(this.context.getBeanProvider(RequestMatcherBuilder.class)).willReturn(requestMatcherFactories);
104+
HandlerMappingIntrospector introspector = mock(HandlerMappingIntrospector.class);
105+
given(this.context.getBean(any(), eq(HandlerMappingIntrospector.class))).willReturn(introspector);
99106
this.matcherRegistry.setApplicationContext(this.context);
100107
mockMvcIntrospector(true);
101108
}
@@ -231,23 +238,23 @@ public void requestMatchersWhenNoDispatcherServletMockMvcThenMvcRequestMatcherTy
231238
assertThat(requestMatchers).hasSize(1);
232239
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
233240
.extracting((matcher) -> matcher.requestMatcher(request))
234-
.isInstanceOf(MvcRequestMatcher.class);
241+
.isInstanceOf(PathPatternRequestMatcher.class);
235242
servletContext.addServlet("servletOne", Servlet.class).addMapping("/one");
236243
servletContext.addServlet("servletTwo", Servlet.class).addMapping("/two");
237244
requestMatchers = this.matcherRegistry.requestMatchers("/**");
238245
assertThat(requestMatchers).isNotEmpty();
239246
assertThat(requestMatchers).hasSize(1);
240247
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
241248
.extracting((matcher) -> matcher.requestMatcher(request))
242-
.isInstanceOf(MvcRequestMatcher.class);
249+
.isInstanceOf(PathPatternRequestMatcher.class);
243250
servletContext.addServlet("servletOne", Servlet.class);
244251
servletContext.addServlet("servletTwo", Servlet.class);
245252
requestMatchers = this.matcherRegistry.requestMatchers("/**");
246253
assertThat(requestMatchers).isNotEmpty();
247254
assertThat(requestMatchers).hasSize(1);
248255
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
249256
.extracting((matcher) -> matcher.requestMatcher(request))
250-
.isInstanceOf(MvcRequestMatcher.class);
257+
.isInstanceOf(PathPatternRequestMatcher.class);
251258
}
252259
}
253260

‎docs/modules/ROOT/pages/servlet/appendix/namespace/http.adoc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ Corresponds to the `realmName` property on `BasicAuthenticationEntryPoint`.
124124
Defines the `RequestMatcher` strategy used in the `FilterChainProxy` and the beans created by the `intercept-url` to match incoming requests.
125125
Options are currently `mvc`, `ant`, `regex` and `ciRegex`, for Spring MVC, ant, regular-expression and case-insensitive regular-expression respectively.
126126
A separate instance is created for each <<nsa-intercept-url,intercept-url>> element using its <<nsa-intercept-url-pattern,pattern>>, <<nsa-intercept-url-method,method>> and <<nsa-intercept-url-servlet-path,servlet-path>> attributes.
127-
Ant paths are matched using an `AntPathRequestMatcher`, regular expressions are matched using a `RegexRequestMatcher` and for Spring MVC path matching the `MvcRequestMatcher` is used.
127+
Ant paths are matched using an `AntPathRequestMatcher`, regular expressions are matched using a `RegexRequestMatcher` and for Spring MVC path matching the `PathPatternRequestMatcher` is used.
128128
See the Javadoc for these classes for more details on exactly how the matching is performed.
129129
MVC is the default strategy if Spring MVC is present in the classpath, if not, Ant paths are used.
130130

0 commit comments

Comments
 (0)