diff --git a/config/src/main/java/org/springframework/security/config/http/MatcherType.java b/config/src/main/java/org/springframework/security/config/http/MatcherType.java index 78dab717dc..7cb211e9c7 100644 --- a/config/src/main/java/org/springframework/security/config/http/MatcherType.java +++ b/config/src/main/java/org/springframework/security/config/http/MatcherType.java @@ -35,7 +35,7 @@ public enum MatcherType { } public BeanDefinition createMatcher(String path, String method) { - if ("/**".equals(path) && method == null) { + if (("/**".equals(path) || "**".equals(path)) && method == null) { return new RootBeanDefinition(AnyRequestMatcher.class); } diff --git a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java index 9a394ceb20..c72ed6e1e1 100644 --- a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java @@ -13,17 +13,25 @@ import org.springframework.util.StringUtils; * Matcher which compares a pre-defined ant-style pattern against the URL * ({@code servletPath + pathInfo}) of an {@code HttpServletRequest}. * The query string of the URL is ignored and matching is case-insensitive. + *
+ * Using a pattern value of {@code /**} or {@code **} is treated as a universal + * match, which will match any request. Patterns which end with {@code /**} (and have no other wildcards) + * are optimized by using a substring match — a pattern of {@code /aaa/**} will match {@code /aaa}, + * {@code /aaa/} and any sub-directories, such as {@code /aaa/bbb/ccc}. + *
+ * For all other cases, Spring's {@link AntPathMatcher} is used to perform the match. See the Spring documentation + * for this class for comprehensive information on the syntax used. * * @author Luke Taylor * @since 3.1 * - * @see AntPathMatcher + * @see org.springframework.util.AntPathMatcher */ public final class AntPathRequestMatcher implements RequestMatcher { - private final static Log logger = LogFactory.getLog(AntPathRequestMatcher.class); - - private static final AntPathMatcher antMatcher = new AntPathMatcher(); + private static final Log logger = LogFactory.getLog(AntPathRequestMatcher.class); + private static final String MATCH_ALL = "/**"; + private final Matcher matcher; private final String pattern; private final HttpMethod httpMethod; @@ -45,7 +53,23 @@ public final class AntPathRequestMatcher implements RequestMatcher { */ public AntPathRequestMatcher(String pattern, String httpMethod) { Assert.hasText(pattern, "Pattern cannot be null or empty"); - this.pattern = pattern.toLowerCase(); + + if (pattern.equals(MATCH_ALL) || pattern.equals("**")) { + pattern = MATCH_ALL; + matcher = null; + } else { + pattern = pattern.toLowerCase(); + + // If the pattern ends with {@code /**} and has no other wildcards, then optimize to a sub-path match + if (pattern.endsWith(MATCH_ALL) && pattern.indexOf('?') == -1 && + pattern.indexOf("*") == pattern.length() - 2) { + matcher = new SubpathMatcher(pattern.substring(0, pattern.length() - 3)); + } else { + matcher = new SpringAntMatcher(pattern); + } + } + + this.pattern = pattern; this.httpMethod = StringUtils.hasText(httpMethod) ? HttpMethod.valueOf(httpMethod) : null; } @@ -57,9 +81,32 @@ public final class AntPathRequestMatcher implements RequestMatcher { */ public boolean matches(HttpServletRequest request) { if (httpMethod != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { + if (logger.isDebugEnabled()) { + logger.debug("Request '" + request.getMethod() + " " + getRequestPath(request) + "'" + + " doesn't match '" + httpMethod + " " + pattern); + } + return false; } + if (pattern.equals(MATCH_ALL)) { + if (logger.isDebugEnabled()) { + logger.debug("Request '" + getRequestPath(request) + "' matched by universal pattern '/**'"); + } + + return true; + } + + String url = getRequestPath(request); + + if (logger.isDebugEnabled()) { + logger.debug("Checking match of request : '" + url + "'; against '" + pattern + "'"); + } + + return matcher.matches(url); + } + + private String getRequestPath(HttpServletRequest request) { String url = request.getServletPath(); if (request.getPathInfo() != null) { @@ -68,12 +115,7 @@ public final class AntPathRequestMatcher implements RequestMatcher { url = url.toLowerCase(); - if (logger.isDebugEnabled()) { - logger.debug("Checking match of request : '" + url + "'; against '" + pattern + "'"); - } - - // TODO: Optimise, since the pattern is fixed. - return antMatcher.match(pattern, url); + return url; } public String getPattern() { @@ -96,11 +138,47 @@ public final class AntPathRequestMatcher implements RequestMatcher { sb.append("Ant [pattern='").append(pattern).append("'"); if (httpMethod != null) { - sb.append(", " + httpMethod); + sb.append(", ").append(httpMethod); } sb.append("]"); return sb.toString(); } + + private static interface Matcher { + boolean matches(String path); + } + + private static class SpringAntMatcher implements Matcher { + private static final AntPathMatcher antMatcher = new AntPathMatcher(); + + private final String pattern; + + private SpringAntMatcher(String pattern) { + this.pattern = pattern; + } + + public boolean matches(String path) { + return antMatcher.match(pattern, path); + } + } + + /** + * Optimized matcher for trailing wildcards + */ + private static class SubpathMatcher implements Matcher { + private final String subpath; + private final int length; + + private SubpathMatcher(String subpath) { + assert !subpath.contains("*"); + this.subpath = subpath; + this.length = subpath.length(); + } + + public boolean matches(String path) { + return path.startsWith(subpath) && (path.length() == length || path.charAt(length) == '/'); + } + } } diff --git a/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java new file mode 100644 index 0000000000..16588d3db8 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/util/AntPathRequestMatcherTests.java @@ -0,0 +1,92 @@ +package org.springframework.security.web.util; + +import static org.junit.Assert.*; + +import org.junit.*; +import org.springframework.mock.web.MockHttpServletRequest; + +/** + * @author Luke Taylor + */ +public class AntPathRequestMatcherTests { + + @Test + public void singleWildcardMatchesAnyPath() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**"); + assertEquals("/**", matcher.getPattern()); + + assertTrue(matcher.matches(createRequest("/blah"))); + + matcher = new AntPathRequestMatcher("**"); + assertTrue(matcher.matches(createRequest("/blah"))); + assertTrue(matcher.matches(createRequest(""))); + } + + @Test + public void trailingWildcardMatchesCorrectly() { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/blah/blAh/**"); + assertTrue(matcher.matches(createRequest("/BLAH/blah"))); + assertFalse(matcher.matches(createRequest("/blah/bleh"))); + assertTrue(matcher.matches(createRequest("/blah/blah/"))); + assertTrue(matcher.matches(createRequest("/blah/blah/xxx"))); + assertFalse(matcher.matches(createRequest("/blah/blaha"))); + assertFalse(matcher.matches(createRequest("/blah/bleh/"))); + MockHttpServletRequest request = createRequest("/blah/"); + + request.setPathInfo("blah/bleh"); + assertTrue(matcher.matches(request)); + + matcher = new AntPathRequestMatcher("/bl?h/blAh/**"); + assertTrue(matcher.matches(createRequest("/BLAH/Blah/aaa/"))); + assertTrue(matcher.matches(createRequest("/bleh/Blah"))); + + matcher = new AntPathRequestMatcher("/blAh/**/blah/**"); + assertTrue(matcher.matches(createRequest("/blah/blah"))); + assertFalse(matcher.matches(createRequest("/blah/bleh"))); + assertTrue(matcher.matches(createRequest("/blah/aaa/blah/bbb"))); + } + + @Test + public void exactMatchOnlyMatchesIdenticalPath() throws Exception { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/login.html"); + assertTrue(matcher.matches(createRequest("/login.html"))); + assertFalse(matcher.matches(createRequest("/login.html/"))); + assertFalse(matcher.matches(createRequest("/login.html/blah"))); + } + + @Test + public void httpMethodSpecificMatchOnlyMatchesRequestsWithCorrectMethod() throws Exception { + AntPathRequestMatcher matcher = new AntPathRequestMatcher("/blah", "GET"); + MockHttpServletRequest request = createRequest("/blah"); + request.setMethod("GET"); + assertTrue(matcher.matches(request)); + request.setMethod("POST"); + assertFalse(matcher.matches(request)); + } + + @Test + public void equalsBehavesCorrectly() throws Exception { + // Both universal wildcard options should be equal + assertEquals(new AntPathRequestMatcher("/**"), new AntPathRequestMatcher("**")); + assertEquals(new AntPathRequestMatcher("/xyz"), new AntPathRequestMatcher("/xyz")); + assertEquals(new AntPathRequestMatcher("/xyz", "POST"), new AntPathRequestMatcher("/xyz", "POST")); + assertFalse(new AntPathRequestMatcher("/xyz", "POST").equals(new AntPathRequestMatcher("/xyz", "GET"))); + assertFalse(new AntPathRequestMatcher("/xyz").equals(new AntPathRequestMatcher("/xxx"))); + assertFalse(new AntPathRequestMatcher("/xyz").equals(new AnyRequestMatcher())); + } + + @Test + public void toStringIsOk() throws Exception { + new AntPathRequestMatcher("/blah").toString(); + new AntPathRequestMatcher("/blah", "GET").toString(); + } + + private MockHttpServletRequest createRequest(String path) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setQueryString("doesntMatter"); + request.setServletPath(path); + request.setMethod("POST"); + + return request; + } +}