Parcourir la source

SEC-1636: Add optimizations for simple pattern cases in AntPathRequestMatcher. "/**" and "**" are treated as universal matches and a trailing "/**" is now optimized using a substring match.

Luke Taylor il y a 14 ans
Parent
commit
2be2660b13

+ 1 - 1
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);
         }
 

+ 90 - 12
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.
+ * <p>
+ * 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 &mdash; a pattern of {@code /aaa/**} will match {@code /aaa},
+ * {@code /aaa/} and any sub-directories, such as {@code /aaa/bbb/ccc}.
+ * <p>
+ * 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,23 +81,41 @@ 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;
         }
 
-        String url = request.getServletPath();
+        if (pattern.equals(MATCH_ALL)) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("Request '" + getRequestPath(request) + "' matched by universal pattern '/**'");
+            }
 
-        if (request.getPathInfo() != null) {
-            url += request.getPathInfo();
+            return true;
         }
 
-        url = url.toLowerCase();
+        String url = getRequestPath(request);
 
         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 matcher.matches(url);
+    }
+
+    private String getRequestPath(HttpServletRequest request) {
+        String url = request.getServletPath();
+
+        if (request.getPathInfo() != null) {
+            url += request.getPathInfo();
+        }
+
+        url = url.toLowerCase();
+
+        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) == '/');
+        }
+    }
 }

+ 92 - 0
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;
+    }
+}