Skip to content

Commit

Permalink
Use PathPatternRequestMatcher by Default
Browse files Browse the repository at this point in the history
  • Loading branch information
jzheaux committed Jan 16, 2025
1 parent f398e6a commit aceb953
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.springframework.security.config.annotation.web.ServletRegistrationsSupport.RegistrationMapping;
import org.springframework.security.config.annotation.web.configurers.AbstractConfigAttributeRequestMatcherRegistry;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher;
Expand All @@ -53,8 +54,8 @@
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.DispatcherServlet;
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
import org.springframework.web.util.pattern.PathPatternParser;

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

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

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

@Override
public RequestMatcher pattern(HttpMethod method, String pattern) {
Assert.state(!AbstractRequestMatcherRegistry.this.anyRequestConfigured,
"Can't configure mvcMatchers after anyRequest");
AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null);
MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0);
RequestMatcher mvc;
if (!AbstractRequestMatcherRegistry.this.context.containsBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME)) {
throw new NoSuchBeanDefinitionException("A Bean named " + HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME
+ " of type " + HandlerMappingIntrospector.class.getName()
+ " is required to use MvcRequestMatcher. Please ensure Spring Security & Spring MVC are configured in a shared ApplicationContext.");
}
HandlerMappingIntrospector introspector = AbstractRequestMatcherRegistry.this.context
.getBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME, HandlerMappingIntrospector.class);
if (introspector.allHandlerMappingsUsePathPatternParser()) {
PathPatternParser pathPatternParser = AbstractRequestMatcherRegistry.this.context
.getBeanProvider(PathPatternParser.class)
.getIfUnique(() -> PathPatternParser.defaultInstance);
mvc = PathPatternRequestMatcher.withPathPatternParser(pathPatternParser).pattern(method, pattern);
}
else {
mvc = createMvcMatchers(method, pattern).get(0);
}
return new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant);
}

Expand Down Expand Up @@ -466,13 +485,8 @@ public boolean matches(HttpServletRequest request) {
ServletRegistration registration = request.getServletContext().getServletRegistration(name);
Assert.notNull(registration,
() -> computeErrorMessage(request.getServletContext().getServletRegistrations().values()));
try {
Class<?> clazz = Class.forName(registration.getClassName());
return DispatcherServlet.class.isAssignableFrom(clazz);
}
catch (ClassNotFoundException ex) {
return false;
}
return new RegistrationMapping(registration, request.getHttpServletMapping().getPattern())
.isDispatcherServlet();
}

}
Expand All @@ -481,15 +495,15 @@ static class DispatcherServletDelegatingRequestMatcher implements RequestMatcher

private final AntPathRequestMatcher ant;

private final MvcRequestMatcher mvc;
private final RequestMatcher mvc;

private final RequestMatcher dispatcherServlet;

DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, MvcRequestMatcher mvc) {
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, RequestMatcher mvc) {
this(ant, mvc, new OrRequestMatcher(new MockMvcRequestMatcher(), new DispatcherServletRequestMatcher()));
}

DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, MvcRequestMatcher mvc,
DispatcherServletDelegatingRequestMatcher(AntPathRequestMatcher ant, RequestMatcher mvc,
RequestMatcher dispatcherServlet) {
this.ant = ant;
this.mvc = mvc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,25 @@

package org.springframework.security.config.http;

import jakarta.servlet.http.HttpServletRequest;
import org.w3c.dom.Element;

import org.springframework.beans.factory.FactoryBean;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.beans.factory.xml.ParserContext;
import org.springframework.http.HttpMethod;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
import org.springframework.web.util.pattern.PathPatternParser;

/**
* Defines the {@link RequestMatcher} types supported by the namespace.
Expand All @@ -40,7 +45,7 @@
public enum MatcherType {

ant(AntPathRequestMatcher.class), regex(RegexRequestMatcher.class), ciRegex(RegexRequestMatcher.class),
mvc(MvcRequestMatcher.class);
mvc(MvcRequestMatcherFactoryBean.class);

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

Expand Down Expand Up @@ -100,4 +105,56 @@ static MatcherType fromElementOrMvc(Element elt) {
return MatcherType.fromElement(elt);
}

private static class MvcRequestMatcherFactoryBean implements FactoryBean<RequestMatcher>, RequestMatcher {

private final HandlerMappingIntrospector introspector;

private final String pattern;

private PathPatternParser pathPatternParser = PathPatternParser.defaultInstance;

private String servletPath;

private HttpMethod method;

public MvcRequestMatcherFactoryBean(HandlerMappingIntrospector introspector, String pattern) {
this.introspector = introspector;
this.pattern = pattern;
}

@Override
public RequestMatcher getObject() {
if (this.introspector.allHandlerMappingsUsePathPatternParser()) {
return PathPatternRequestMatcher.withPathPatternParser(this.pathPatternParser)
.servletPath(this.servletPath)
.pattern(this.method, this.pattern);
}
return new MvcRequestMatcher.Builder(this.introspector).servletPath(this.servletPath)
.pattern(this.method, this.pattern);
}

@Override
public Class<?> getObjectType() {
return RequestMatcher.class;
}

@Override
public boolean matches(HttpServletRequest request) {
return getObject().matches(request);
}

public void setPathPatternParser(PathPatternParser pathPatternParser) {
this.pathPatternParser = pathPatternParser;
}

public void setServletPath(String servletPath) {
this.servletPath = servletPath;
}

public void setMethod(HttpMethod method) {
this.method = method;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ import org.springframework.security.web.access.IpAddressAuthorizationManager
import org.springframework.security.web.access.intercept.AuthorizationFilter
import org.springframework.security.web.access.intercept.RequestAuthorizationContext
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher
import org.springframework.security.web.util.matcher.AnyRequestMatcher
import org.springframework.security.web.util.matcher.RequestMatcher
import org.springframework.util.ClassUtils
import org.springframework.web.servlet.handler.HandlerMappingIntrospector
import org.springframework.web.util.pattern.PathPatternParser
import java.util.function.Supplier

/**
Expand Down Expand Up @@ -292,11 +294,20 @@ class AuthorizeHttpRequestsDsl : AbstractRequestMatcherDsl {
PatternType.ANT -> requests.requestMatchers(rule.httpMethod, rule.pattern).access(rule.rule)
PatternType.MVC -> {
val introspector = requests.applicationContext.getBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME, HandlerMappingIntrospector::class.java)
val mvcMatcher = MvcRequestMatcher.Builder(introspector)
.servletPath(rule.servletPath)
.pattern(rule.pattern)
mvcMatcher.setMethod(rule.httpMethod)
requests.requestMatchers(mvcMatcher).access(rule.rule)
if (introspector.allHandlerMappingsUsePathPatternParser()) {
val pathPatternParser: PathPatternParser = requests.applicationContext.getBeanProvider(PathPatternParser::class.java)
.getIfUnique({PathPatternParser.defaultInstance})
val mvcMatcher = PathPatternRequestMatcher.withPathPatternParser(pathPatternParser)
.servletPath(rule.servletPath)
.pattern(rule.httpMethod, rule.pattern)
requests.requestMatchers(mvcMatcher).access(rule.rule)
} else {
val mvcMatcher = MvcRequestMatcher.Builder(introspector)
.servletPath(rule.servletPath)
.pattern(rule.pattern)
mvcMatcher.setMethod(rule.httpMethod)
requests.requestMatchers(mvcMatcher).access(rule.rule)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.util.pattern.PathPattern;

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

private String getPattern(SecurityFilterChain chain) {
RequestMatcher requestMatcher = ((DefaultSecurityFilterChain) chain).getRequestMatcher();
return (String) ReflectionTestUtils.getField(requestMatcher, "pattern");
Object pattern = ReflectionTestUtils.getField(requestMatcher, "pattern");
return (pattern instanceof PathPattern) ? pattern.toString() : (String) pattern;
}

private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import jakarta.servlet.DispatcherType;
import jakarta.servlet.Servlet;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -39,6 +40,7 @@
import org.springframework.security.web.servlet.MockServletContext;
import org.springframework.security.web.servlet.TestMockHttpServletMappings;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher;
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
Expand All @@ -49,10 +51,13 @@
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.DispatcherServlet;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.InstanceOfAssertFactories.type;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -91,11 +96,13 @@ public void setUp() {
given(this.context.getServletContext()).willReturn(MockServletContext.mvc());
ObjectProvider<RequestMatcherBuilder> requestMatcherFactories = new ObjectProvider<>() {
@Override
public RequestMatcherBuilder getObject() throws BeansException {
public @NotNull RequestMatcherBuilder getObject() throws BeansException {
return AbstractRequestMatcherRegistryTests.this.matcherRegistry.new DefaultRequestMatcherBuilder();
}
};
given(this.context.getBeanProvider(RequestMatcherBuilder.class)).willReturn(requestMatcherFactories);
HandlerMappingIntrospector introspector = mock(HandlerMappingIntrospector.class);
given(this.context.getBean(any(), eq(HandlerMappingIntrospector.class))).willReturn(introspector);
this.matcherRegistry.setApplicationContext(this.context);
mockMvcIntrospector(true);
}
Expand Down Expand Up @@ -231,23 +238,23 @@ public void requestMatchersWhenNoDispatcherServletMockMvcThenMvcRequestMatcherTy
assertThat(requestMatchers).hasSize(1);
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
.extracting((matcher) -> matcher.requestMatcher(request))
.isInstanceOf(MvcRequestMatcher.class);
.isInstanceOf(PathPatternRequestMatcher.class);
servletContext.addServlet("servletOne", Servlet.class).addMapping("/one");
servletContext.addServlet("servletTwo", Servlet.class).addMapping("/two");
requestMatchers = this.matcherRegistry.requestMatchers("/**");
assertThat(requestMatchers).isNotEmpty();
assertThat(requestMatchers).hasSize(1);
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
.extracting((matcher) -> matcher.requestMatcher(request))
.isInstanceOf(MvcRequestMatcher.class);
.isInstanceOf(PathPatternRequestMatcher.class);
servletContext.addServlet("servletOne", Servlet.class);
servletContext.addServlet("servletTwo", Servlet.class);
requestMatchers = this.matcherRegistry.requestMatchers("/**");
assertThat(requestMatchers).isNotEmpty();
assertThat(requestMatchers).hasSize(1);
assertThat(requestMatchers.get(0)).asInstanceOf(type(DispatcherServletDelegatingRequestMatcher.class))
.extracting((matcher) -> matcher.requestMatcher(request))
.isInstanceOf(MvcRequestMatcher.class);
.isInstanceOf(PathPatternRequestMatcher.class);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ Corresponds to the `realmName` property on `BasicAuthenticationEntryPoint`.
Defines the `RequestMatcher` strategy used in the `FilterChainProxy` and the beans created by the `intercept-url` to match incoming requests.
Options are currently `mvc`, `ant`, `regex` and `ciRegex`, for Spring MVC, ant, regular-expression and case-insensitive regular-expression respectively.
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.
Ant paths are matched using an `AntPathRequestMatcher`, regular expressions are matched using a `RegexRequestMatcher` and for Spring MVC path matching the `MvcRequestMatcher` is used.
Ant paths are matched using an `AntPathRequestMatcher`, regular expressions are matched using a `RegexRequestMatcher` and for Spring MVC path matching the `PathPatternRequestMatcher` is used.
See the Javadoc for these classes for more details on exactly how the matching is performed.
MVC is the default strategy if Spring MVC is present in the classpath, if not, Ant paths are used.

Expand Down
Loading

0 comments on commit aceb953

Please sign in to comment.