Improving tests for path matching logic
diff --git a/NOTICE b/NOTICE
index afda186..7d8eb88 100644
--- a/NOTICE
+++ b/NOTICE
@@ -9,7 +9,7 @@
available at http://www.javaspecialists.eu/archive/Issue015.html,
with continued modifications.
-Certain parts (StringUtils etc.) of the source code for this
+Certain parts (StringUtils, AntPathMatcherTests, etc.) of the source code for this
product was copied for simplicity and to reduce dependencies
from the source code developed by the Spring Framework Project
(http://www.springframework.org).
diff --git a/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java b/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java
index f33f85e..22e5cd2 100644
--- a/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java
+++ b/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java
@@ -77,8 +77,15 @@
this.pathSeparator = (pathSeparator != null ? pathSeparator : DEFAULT_PATH_SEPARATOR);
}
-
+ /**
+ * Checks if {@code path} is a pattern (i.e. contains a '*', or '?'). For example the {@code /foo/**} would return {@code true}, while {@code /bar/} would return {@code false}.
+ * @param path the string to check
+ * @return this method returns {@code true} if {@code path} contains a '*' or '?', otherwise, {@code false}
+ */
public boolean isPattern(String path) {
+ if (path == null) {
+ return false;
+ }
return (path.indexOf('*') != -1 || path.indexOf('?') != -1);
}
@@ -106,12 +113,12 @@
* <code>false</code> if it didn't
*/
protected boolean doMatch(String pattern, String path, boolean fullMatch) {
- if (path.startsWith(this.pathSeparator) != pattern.startsWith(this.pathSeparator)) {
+ if (path == null || path.startsWith(this.pathSeparator) != pattern.startsWith(this.pathSeparator)) {
return false;
}
- String[] pattDirs = StringUtils.tokenizeToStringArray(pattern, this.pathSeparator);
- String[] pathDirs = StringUtils.tokenizeToStringArray(path, this.pathSeparator);
+ String[] pattDirs = StringUtils.tokenizeToStringArray(pattern, this.pathSeparator, false, true);
+ String[] pathDirs = StringUtils.tokenizeToStringArray(path, this.pathSeparator, false, true);
int pattIdxStart = 0;
int pattIdxEnd = pattDirs.length - 1;
@@ -393,33 +400,26 @@
* and '<code>path</code>', but does <strong>not</strong> enforce this.
*/
public String extractPathWithinPattern(String pattern, String path) {
- String[] patternParts = StringUtils.tokenizeToStringArray(pattern, this.pathSeparator);
- String[] pathParts = StringUtils.tokenizeToStringArray(path, this.pathSeparator);
+ String[] patternParts = StringUtils.tokenizeToStringArray(pattern, this.pathSeparator, false, true);
+ String[] pathParts = StringUtils.tokenizeToStringArray(path, this.pathSeparator, false, true);
+ StringBuilder builder = new StringBuilder();
+ boolean pathStarted = false;
- StringBuilder buffer = new StringBuilder();
-
- // Add any path parts that have a wildcarded pattern part.
- int puts = 0;
- for (int i = 0; i < patternParts.length; i++) {
- String patternPart = patternParts[i];
- if ((patternPart.indexOf('*') > -1 || patternPart.indexOf('?') > -1) && pathParts.length >= i + 1) {
- if (puts > 0 || (i == 0 && !pattern.startsWith(this.pathSeparator))) {
- buffer.append(this.pathSeparator);
+ for (int segment = 0; segment < patternParts.length; segment++) {
+ String patternPart = patternParts[segment];
+ if (patternPart.indexOf('*') > -1 || patternPart.indexOf('?') > -1) {
+ for (; segment < pathParts.length; segment++) {
+ if (pathStarted || (segment == 0 && !pattern.startsWith(this.pathSeparator))) {
+ builder.append(this.pathSeparator);
+ }
+ builder.append(pathParts[segment]);
+ pathStarted = true;
}
- buffer.append(pathParts[i]);
- puts++;
}
}
- // Append any trailing path parts.
- for (int i = patternParts.length; i < pathParts.length; i++) {
- if (puts > 0 || i > 0) {
- buffer.append(this.pathSeparator);
- }
- buffer.append(pathParts[i]);
- }
-
- return buffer.toString();
+ return builder.toString();
}
+
}
diff --git a/core/src/test/java/org/apache/shiro/util/AntPathMatcherTests.java b/core/src/test/java/org/apache/shiro/util/AntPathMatcherTests.java
new file mode 100644
index 0000000..383686a
--- /dev/null
+++ b/core/src/test/java/org/apache/shiro/util/AntPathMatcherTests.java
@@ -0,0 +1,330 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.shiro.util;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * Unit tests for {@link AntPathMatcher}.
+ *
+ * Adapted from <a href="https://github.com/spring-projects/spring-framework/blob/b92d249f450920e48e640af6bbd0bd509e7d707d/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java"/>Spring Framework's similar AntPathMatcherTests</a>
+ */
+public class AntPathMatcherTests {
+
+ private final AntPathMatcher pathMatcher = new AntPathMatcher();
+
+ @Test
+ public void match() {
+ // test exact matching
+ assertTrue(pathMatcher.match("test", "test"));
+ assertTrue(pathMatcher.match("/test", "/test"));
+ assertTrue(pathMatcher.match("https://example.org", "https://example.org"));
+ assertFalse(pathMatcher.match("/test.jpg", "test.jpg"));
+ assertFalse(pathMatcher.match("test", "/test"));
+ assertFalse(pathMatcher.match("/test", "test"));
+
+ // test matching with ?'s
+ assertTrue(pathMatcher.match("t?st", "test"));
+ assertTrue(pathMatcher.match("??st", "test"));
+ assertTrue(pathMatcher.match("tes?", "test"));
+ assertTrue(pathMatcher.match("te??", "test"));
+ assertTrue(pathMatcher.match("?es?", "test"));
+ assertFalse(pathMatcher.match("tes?", "tes"));
+ assertFalse(pathMatcher.match("tes?", "testt"));
+ assertFalse(pathMatcher.match("tes?", "tsst"));
+
+ // test matching with *'s
+ assertTrue(pathMatcher.match("*", "test"));
+ assertTrue(pathMatcher.match("test*", "test"));
+ assertTrue(pathMatcher.match("test*", "testTest"));
+ assertTrue(pathMatcher.match("test/*", "test/Test"));
+ assertTrue(pathMatcher.match("test/*", "test/t"));
+ assertTrue(pathMatcher.match("test/*", "test/"));
+ assertTrue(pathMatcher.match("*test*", "AnothertestTest"));
+ assertTrue(pathMatcher.match("*test", "Anothertest"));
+ assertTrue(pathMatcher.match("*.*", "test."));
+ assertTrue(pathMatcher.match("*.*", "test.test"));
+ assertTrue(pathMatcher.match("*.*", "test.test.test"));
+ assertTrue(pathMatcher.match("test*aaa", "testblaaaa"));
+ assertFalse(pathMatcher.match("test*", "tst"));
+ assertFalse(pathMatcher.match("test*", "tsttest"));
+ assertFalse(pathMatcher.match("test*", "test/"));
+ assertFalse(pathMatcher.match("test*", "test/t"));
+ assertFalse(pathMatcher.match("test/*", "test"));
+ assertFalse(pathMatcher.match("*test*", "tsttst"));
+ assertFalse(pathMatcher.match("*test", "tsttst"));
+ assertFalse(pathMatcher.match("*.*", "tsttst"));
+ assertFalse(pathMatcher.match("test*aaa", "test"));
+ assertFalse(pathMatcher.match("test*aaa", "testblaaab"));
+
+ // test matching with ?'s and /'s
+ assertTrue(pathMatcher.match("/?", "/a"));
+ assertTrue(pathMatcher.match("/?/a", "/a/a"));
+ assertTrue(pathMatcher.match("/a/?", "/a/b"));
+ assertTrue(pathMatcher.match("/??/a", "/aa/a"));
+ assertTrue(pathMatcher.match("/a/??", "/a/bb"));
+ assertTrue(pathMatcher.match("/?", "/a"));
+
+ // test matching with **'s
+ assertTrue(pathMatcher.match("/**", "/testing/testing"));
+ assertTrue(pathMatcher.match("/*/**", "/testing/testing"));
+ assertTrue(pathMatcher.match("/**/*", "/testing/testing"));
+ assertTrue(pathMatcher.match("/bla/**/bla", "/bla/testing/testing/bla"));
+ assertTrue(pathMatcher.match("/bla/**/bla", "/bla/testing/testing/bla/bla"));
+ assertTrue(pathMatcher.match("/**/test", "/bla/bla/test"));
+ assertTrue(pathMatcher.match("/bla/**/**/bla", "/bla/bla/bla/bla/bla/bla"));
+ assertTrue(pathMatcher.match("/bla*bla/test", "/blaXXXbla/test"));
+ assertTrue(pathMatcher.match("/*bla/test", "/XXXbla/test"));
+ assertFalse(pathMatcher.match("/bla*bla/test", "/blaXXXbl/test"));
+ assertFalse(pathMatcher.match("/*bla/test", "XXXblab/test"));
+ assertFalse(pathMatcher.match("/*bla/test", "XXXbl/test"));
+
+ assertFalse(pathMatcher.match("/????", "/bala/bla"));
+ assertFalse(pathMatcher.match("/**/*bla", "/bla/bla/bla/bbb"));
+
+ assertTrue(pathMatcher.match("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing/"));
+ assertTrue(pathMatcher.match("/*bla*/**/bla/*", "/XXXblaXXXX/testing/testing/bla/testing"));
+ assertTrue(pathMatcher.match("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing"));
+ assertTrue(pathMatcher.match("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing.jpg"));
+
+ assertTrue(pathMatcher.match("*bla*/**/bla/**", "XXXblaXXXX/testing/testing/bla/testing/testing/"));
+ assertTrue(pathMatcher.match("*bla*/**/bla/*", "XXXblaXXXX/testing/testing/bla/testing"));
+ assertTrue(pathMatcher.match("*bla*/**/bla/**", "XXXblaXXXX/testing/testing/bla/testing/testing"));
+ assertFalse(pathMatcher.match("*bla*/**/bla/*", "XXXblaXXXX/testing/testing/bla/testing/testing"));
+
+ assertFalse(pathMatcher.match("/x/x/**/bla", "/x/x/x/"));
+
+ assertTrue(pathMatcher.match("/foo/bar/**", "/foo/bar"));
+
+ assertTrue(pathMatcher.match("", ""));
+ }
+
+ @Test
+ public void matchWithNullPath() {
+ assertFalse(pathMatcher.match("/test", null));
+ assertFalse(pathMatcher.match("/", null));
+ assertFalse(pathMatcher.match(null, null));
+ }
+
+ @Test
+ public void matchStart() {
+ // test exact matching
+ assertTrue(pathMatcher.matchStart("test", "test"));
+ assertTrue(pathMatcher.matchStart("/test", "/test"));
+ assertFalse(pathMatcher.matchStart("/test.jpg", "test.jpg"));
+ assertFalse(pathMatcher.matchStart("test", "/test"));
+ assertFalse(pathMatcher.matchStart("/test", "test"));
+
+ // test matching with ?'s
+ assertTrue(pathMatcher.matchStart("t?st", "test"));
+ assertTrue(pathMatcher.matchStart("??st", "test"));
+ assertTrue(pathMatcher.matchStart("tes?", "test"));
+ assertTrue(pathMatcher.matchStart("te??", "test"));
+ assertTrue(pathMatcher.matchStart("?es?", "test"));
+ assertFalse(pathMatcher.matchStart("tes?", "tes"));
+ assertFalse(pathMatcher.matchStart("tes?", "testt"));
+ assertFalse(pathMatcher.matchStart("tes?", "tsst"));
+
+ // test matching with *'s
+ assertTrue(pathMatcher.matchStart("*", "test"));
+ assertTrue(pathMatcher.matchStart("test*", "test"));
+ assertTrue(pathMatcher.matchStart("test*", "testTest"));
+ assertTrue(pathMatcher.matchStart("test/*", "test/Test"));
+ assertTrue(pathMatcher.matchStart("test/*", "test/t"));
+ assertTrue(pathMatcher.matchStart("test/*", "test/"));
+ assertTrue(pathMatcher.matchStart("*test*", "AnothertestTest"));
+ assertTrue(pathMatcher.matchStart("*test", "Anothertest"));
+ assertTrue(pathMatcher.matchStart("*.*", "test."));
+ assertTrue(pathMatcher.matchStart("*.*", "test.test"));
+ assertTrue(pathMatcher.matchStart("*.*", "test.test.test"));
+ assertTrue(pathMatcher.matchStart("test*aaa", "testblaaaa"));
+ assertFalse(pathMatcher.matchStart("test*", "tst"));
+ assertFalse(pathMatcher.matchStart("test*", "test/"));
+ assertFalse(pathMatcher.matchStart("test*", "tsttest"));
+ assertFalse(pathMatcher.matchStart("test*", "test/"));
+ assertFalse(pathMatcher.matchStart("test*", "test/t"));
+ assertTrue(pathMatcher.matchStart("test/*", "test"));
+ assertTrue(pathMatcher.matchStart("test/t*.txt", "test"));
+ assertFalse(pathMatcher.matchStart("*test*", "tsttst"));
+ assertFalse(pathMatcher.matchStart("*test", "tsttst"));
+ assertFalse(pathMatcher.matchStart("*.*", "tsttst"));
+ assertFalse(pathMatcher.matchStart("test*aaa", "test"));
+ assertFalse(pathMatcher.matchStart("test*aaa", "testblaaab"));
+
+ // test matching with ?'s and /'s
+ assertTrue(pathMatcher.matchStart("/?", "/a"));
+ assertTrue(pathMatcher.matchStart("/?/a", "/a/a"));
+ assertTrue(pathMatcher.matchStart("/a/?", "/a/b"));
+ assertTrue(pathMatcher.matchStart("/??/a", "/aa/a"));
+ assertTrue(pathMatcher.matchStart("/a/??", "/a/bb"));
+ assertTrue(pathMatcher.matchStart("/?", "/a"));
+
+ // test matching with **'s
+ assertTrue(pathMatcher.matchStart("/**", "/testing/testing"));
+ assertTrue(pathMatcher.matchStart("/*/**", "/testing/testing"));
+ assertTrue(pathMatcher.matchStart("/**/*", "/testing/testing"));
+ assertTrue(pathMatcher.matchStart("test*/**", "test/"));
+ assertTrue(pathMatcher.matchStart("test*/**", "test/t"));
+ assertTrue(pathMatcher.matchStart("/bla/**/bla", "/bla/testing/testing/bla"));
+ assertTrue(pathMatcher.matchStart("/bla/**/bla", "/bla/testing/testing/bla/bla"));
+ assertTrue(pathMatcher.matchStart("/**/test", "/bla/bla/test"));
+ assertTrue(pathMatcher.matchStart("/bla/**/**/bla", "/bla/bla/bla/bla/bla/bla"));
+ assertTrue(pathMatcher.matchStart("/bla*bla/test", "/blaXXXbla/test"));
+ assertTrue(pathMatcher.matchStart("/*bla/test", "/XXXbla/test"));
+ assertFalse(pathMatcher.matchStart("/bla*bla/test", "/blaXXXbl/test"));
+ assertFalse(pathMatcher.matchStart("/*bla/test", "XXXblab/test"));
+ assertFalse(pathMatcher.matchStart("/*bla/test", "XXXbl/test"));
+
+ assertFalse(pathMatcher.matchStart("/????", "/bala/bla"));
+ assertTrue(pathMatcher.matchStart("/**/*bla", "/bla/bla/bla/bbb"));
+
+ assertTrue(pathMatcher.matchStart("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing/"));
+ assertTrue(pathMatcher.matchStart("/*bla*/**/bla/*", "/XXXblaXXXX/testing/testing/bla/testing"));
+ assertTrue(pathMatcher.matchStart("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing"));
+ assertTrue(pathMatcher.matchStart("/*bla*/**/bla/**", "/XXXblaXXXX/testing/testing/bla/testing/testing.jpg"));
+
+ assertTrue(pathMatcher.matchStart("*bla*/**/bla/**", "XXXblaXXXX/testing/testing/bla/testing/testing/"));
+ assertTrue(pathMatcher.matchStart("*bla*/**/bla/*", "XXXblaXXXX/testing/testing/bla/testing"));
+ assertTrue(pathMatcher.matchStart("*bla*/**/bla/**", "XXXblaXXXX/testing/testing/bla/testing/testing"));
+ assertTrue(pathMatcher.matchStart("*bla*/**/bla/*", "XXXblaXXXX/testing/testing/bla/testing/testing"));
+
+ assertTrue(pathMatcher.matchStart("/x/x/**/bla", "/x/x/x/"));
+
+ assertTrue(pathMatcher.matchStart("", ""));
+ }
+
+ @Test
+ public void uniqueDeliminator() {
+ pathMatcher.setPathSeparator(".");
+
+ // test exact matching
+ assertTrue(pathMatcher.match("test", "test"));
+ assertTrue(pathMatcher.match(".test", ".test"));
+ assertFalse(pathMatcher.match(".test/jpg", "test/jpg"));
+ assertFalse(pathMatcher.match("test", ".test"));
+ assertFalse(pathMatcher.match(".test", "test"));
+
+ // test matching with ?'s
+ assertTrue(pathMatcher.match("t?st", "test"));
+ assertTrue(pathMatcher.match("??st", "test"));
+ assertTrue(pathMatcher.match("tes?", "test"));
+ assertTrue(pathMatcher.match("te??", "test"));
+ assertTrue(pathMatcher.match("?es?", "test"));
+ assertFalse(pathMatcher.match("tes?", "tes"));
+ assertFalse(pathMatcher.match("tes?", "testt"));
+ assertFalse(pathMatcher.match("tes?", "tsst"));
+
+ // test matching with *'s
+ assertTrue(pathMatcher.match("*", "test"));
+ assertTrue(pathMatcher.match("test*", "test"));
+ assertTrue(pathMatcher.match("test*", "testTest"));
+ assertTrue(pathMatcher.match("*test*", "AnothertestTest"));
+ assertTrue(pathMatcher.match("*test", "Anothertest"));
+ assertTrue(pathMatcher.match("*/*", "test/"));
+ assertTrue(pathMatcher.match("*/*", "test/test"));
+ assertTrue(pathMatcher.match("*/*", "test/test/test"));
+ assertTrue(pathMatcher.match("test*aaa", "testblaaaa"));
+ assertFalse(pathMatcher.match("test*", "tst"));
+ assertFalse(pathMatcher.match("test*", "tsttest"));
+ assertFalse(pathMatcher.match("*test*", "tsttst"));
+ assertFalse(pathMatcher.match("*test", "tsttst"));
+ assertFalse(pathMatcher.match("*/*", "tsttst"));
+ assertFalse(pathMatcher.match("test*aaa", "test"));
+ assertFalse(pathMatcher.match("test*aaa", "testblaaab"));
+
+ // test matching with ?'s and .'s
+ assertTrue(pathMatcher.match(".?", ".a"));
+ assertTrue(pathMatcher.match(".?.a", ".a.a"));
+ assertTrue(pathMatcher.match(".a.?", ".a.b"));
+ assertTrue(pathMatcher.match(".??.a", ".aa.a"));
+ assertTrue(pathMatcher.match(".a.??", ".a.bb"));
+ assertTrue(pathMatcher.match(".?", ".a"));
+
+ // test matching with **'s
+ assertTrue(pathMatcher.match(".**", ".testing.testing"));
+ assertTrue(pathMatcher.match(".*.**", ".testing.testing"));
+ assertTrue(pathMatcher.match(".**.*", ".testing.testing"));
+ assertTrue(pathMatcher.match(".bla.**.bla", ".bla.testing.testing.bla"));
+ assertTrue(pathMatcher.match(".bla.**.bla", ".bla.testing.testing.bla.bla"));
+ assertTrue(pathMatcher.match(".**.test", ".bla.bla.test"));
+ assertTrue(pathMatcher.match(".bla.**.**.bla", ".bla.bla.bla.bla.bla.bla"));
+ assertTrue(pathMatcher.match(".bla*bla.test", ".blaXXXbla.test"));
+ assertTrue(pathMatcher.match(".*bla.test", ".XXXbla.test"));
+ assertFalse(pathMatcher.match(".bla*bla.test", ".blaXXXbl.test"));
+ assertFalse(pathMatcher.match(".*bla.test", "XXXblab.test"));
+ assertFalse(pathMatcher.match(".*bla.test", "XXXbl.test"));
+ }
+
+ @Test
+ public void extractPathWithinPattern() throws Exception {
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/commit.html", "/docs/commit.html"), "");
+
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/*", "/docs/cvs/commit"), "cvs/commit");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/cvs/*.html", "/docs/cvs/commit.html"), "commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/**", "/docs/cvs/commit"), "cvs/commit");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/cvs/commit.html"), "cvs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/commit.html"), "commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/*.html", "/commit.html"), "commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/*.html", "/docs/commit.html"), "docs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("*.html", "/commit.html"), "/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("*.html", "/docs/commit.html"), "/docs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("**/*.*", "/docs/commit.html"), "/docs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("*", "/docs/commit.html"), "/docs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("**/commit.html", "/docs/cvs/other/commit.html"), "/docs/cvs/other/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/commit.html", "/docs/cvs/other/commit.html"), "cvs/other/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/**/**/**", "/docs/cvs/other/commit.html"), "cvs/other/commit.html");
+
+ assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/*", "/docs/cvs/commit"), "docs/cvs/commit");
+ assertEquals(pathMatcher.extractPathWithinPattern("/docs/c?s/*.html", "/docs/cvs/commit.html"), "cvs/commit.html");
+ assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/**", "/docs/cvs/commit"), "docs/cvs/commit");
+ assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/**/*.html", "/docs/cvs/commit.html"), "docs/cvs/commit.html");
+ }
+
+ @Test
+ public void spaceInTokens() {
+ assertTrue(pathMatcher.match("/group/sales/members", "/group/sales/members"));
+ assertFalse(pathMatcher.match("/group/sales/members", "/Group/ sales/Members"));
+ }
+
+ @Test
+ public void isPattern() {
+ assertTrue(pathMatcher.isPattern("/test/*"));
+ assertTrue(pathMatcher.isPattern("/test/**/name"));
+ assertTrue(pathMatcher.isPattern("/test?"));
+
+ assertFalse(pathMatcher.isPattern("/test/{name}"));
+ assertFalse(pathMatcher.isPattern("/test/name"));
+ assertFalse(pathMatcher.isPattern("/test/foo{bar"));
+ }
+
+ @Test
+ public void matches() {
+ assertTrue(pathMatcher.matches("/foo/*", "/foo/"));
+ }
+
+ @Test
+ public void isPatternWithNullPath() {
+ assertFalse(pathMatcher.isPattern(null));
+ }
+}
\ No newline at end of file
diff --git a/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java b/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
index be1da71..aee80d1 100644
--- a/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
+++ b/web/src/main/java/org/apache/shiro/web/filter/PathMatchingFilter.java
@@ -20,7 +20,6 @@
import org.apache.shiro.util.AntPathMatcher;
import org.apache.shiro.util.PatternMatcher;
-import org.apache.shiro.util.StringUtils;
import org.apache.shiro.web.servlet.AdviceFilter;
import org.apache.shiro.web.util.WebUtils;
import org.owasp.encoder.Encode;
@@ -123,16 +122,24 @@
*/
protected boolean pathsMatch(String path, ServletRequest request) {
String requestURI = getPathWithinApplication(request);
- if (requestURI != null && !DEFAULT_PATH_SEPARATOR.equals(requestURI)
- && requestURI.endsWith(DEFAULT_PATH_SEPARATOR)) {
- requestURI = requestURI.substring(0, requestURI.length() - 1);
- }
- if (path != null && !DEFAULT_PATH_SEPARATOR.equals(path)
- && path.endsWith(DEFAULT_PATH_SEPARATOR)) {
- path = path.substring(0, path.length() - 1);
- }
+
log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(requestURI));
- return pathsMatch(path, requestURI);
+ boolean match = pathsMatch(path, requestURI);
+
+ if (!match) {
+ if (requestURI != null && !DEFAULT_PATH_SEPARATOR.equals(requestURI)
+ && requestURI.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ requestURI = requestURI.substring(0, requestURI.length() - 1);
+ }
+ if (path != null && !DEFAULT_PATH_SEPARATOR.equals(path)
+ && path.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ path = path.substring(0, path.length() - 1);
+ }
+ log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(requestURI));
+ match = pathsMatch(path, requestURI);
+ }
+
+ return match;
}
/**
@@ -149,7 +156,9 @@
* <code>false</code> otherwise.
*/
protected boolean pathsMatch(String pattern, String path) {
- return pathMatcher.matches(pattern, path);
+ boolean matches = pathMatcher.matches(pattern, path);
+ log.trace("Pattern [{}] matches path [{}] => [{}]", pattern, path, matches);
+ return matches;
}
/**
diff --git a/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java b/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
index c35ab9b..452d186 100644
--- a/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
+++ b/web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java
@@ -100,32 +100,34 @@
return null;
}
- String requestURI = getPathWithinApplication(request);
-
- // in spring web, the requestURI "/resource/menus" ---- "resource/menus/" bose can access the resource
- // but the pathPattern match "/resource/menus" can not match "resource/menus/"
- // user can use requestURI + "/" to simply bypassed chain filter, to bypassed shiro protect
- if(requestURI != null && !DEFAULT_PATH_SEPARATOR.equals(requestURI)
- && requestURI.endsWith(DEFAULT_PATH_SEPARATOR)) {
- requestURI = requestURI.substring(0, requestURI.length() - 1);
- }
-
+ final String requestURI = getPathWithinApplication(request);
+ final String requestURINoTrailingSlash = removeTrailingSlash(requestURI);
//the 'chain names' in this implementation are actually path patterns defined by the user. We just use them
//as the chain name for the FilterChainManager's requirements
for (String pathPattern : filterChainManager.getChainNames()) {
- if (pathPattern != null && !DEFAULT_PATH_SEPARATOR.equals(pathPattern)
- && pathPattern.endsWith(DEFAULT_PATH_SEPARATOR)) {
- pathPattern = pathPattern.substring(0, pathPattern.length() - 1);
- }
-
// If the path does match, then pass on to the subclass implementation for specific checks:
if (pathMatches(pathPattern, requestURI)) {
if (log.isTraceEnabled()) {
- log.trace("Matched path pattern [" + pathPattern + "] for requestURI [" + Encode.forHtml(requestURI) + "]. " +
- "Utilizing corresponding filter chain...");
+ log.trace("Matched path pattern [{}] for requestURI [{}]. " +
+ "Utilizing corresponding filter chain...", pathPattern, Encode.forHtml(requestURI));
}
return filterChainManager.proxy(originalChain, pathPattern);
+ } else {
+
+ // in spring web, the requestURI "/resource/menus" ---- "resource/menus/" bose can access the resource
+ // but the pathPattern match "/resource/menus" can not match "resource/menus/"
+ // user can use requestURI + "/" to simply bypassed chain filter, to bypassed shiro protect
+
+ pathPattern = removeTrailingSlash(pathPattern);
+
+ if (pathMatches(pathPattern, requestURINoTrailingSlash)) {
+ if (log.isTraceEnabled()) {
+ log.trace("Matched path pattern [{}] for requestURI [{}]. " +
+ "Utilizing corresponding filter chain...", pathPattern, Encode.forHtml(requestURINoTrailingSlash));
+ }
+ return filterChainManager.proxy(originalChain, requestURINoTrailingSlash);
+ }
}
}
@@ -163,4 +165,12 @@
protected String getPathWithinApplication(ServletRequest request) {
return WebUtils.getPathWithinApplication(WebUtils.toHttp(request));
}
+
+ private static String removeTrailingSlash(String path) {
+ if(path != null && !DEFAULT_PATH_SEPARATOR.equals(path)
+ && path.endsWith(DEFAULT_PATH_SEPARATOR)) {
+ return path.substring(0, path.length() - 1);
+ }
+ return path;
+ }
}
diff --git a/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterParameterizedTest.java b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterParameterizedTest.java
new file mode 100644
index 0000000..82720ad
--- /dev/null
+++ b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterParameterizedTest.java
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.shiro.web.filter;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+
+import java.util.stream.Stream;
+
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Unit tests for the {@link PathMatchingFilter} implementation.
+ */
+@RunWith(Parameterized.class)
+public class PathMatchingFilterParameterizedTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(PathMatchingFilterParameterizedTest.class);
+
+ private static final String CONTEXT_PATH = "/";
+ private static final String DISABLED_PATH = CONTEXT_PATH + "disabled";
+
+ private PathMatchingFilter filter;
+
+ @Parameterized.Parameter(0)
+ public String pattern;
+
+ @Parameterized.Parameter(1)
+ public HttpServletRequest request;
+
+ @Parameterized.Parameter(2)
+ public boolean shouldMatch;
+
+ /**
+ * Tests the following assumptions:
+ *
+ * <pre>
+ * URL Must match pattern Must not match pattern
+ * /foo/ /foo/* /foo* || /foo
+ * /foo/bar /foo/* /foo* || /foo
+ * /foo /foo /foo/*
+ * </pre>
+ */
+ @Parameterized.Parameters
+ public static Object[][] generateParameters() {
+
+ return Stream.of(
+ new Object[]{ "/foo/*", createRequest("/foo/"), true },
+ new Object[]{ "/foo*", createRequest("/foo/"), true },
+ new Object[]{ "/foo", createRequest("/foo/"), true },
+
+ new Object[]{ "/foo/*", createRequest("/foo/bar"), true },
+ new Object[]{ "/foo*", createRequest("/foo/bar"), false },
+ new Object[]{ "/foo", createRequest("/foo/bar"), false },
+
+ new Object[]{ "/foo", createRequest("/foo"), true },
+ new Object[]{ "/foo/*", createRequest("/foo"), false },
+ new Object[]{ "/foo/*", createRequest("/foo "), false },
+ new Object[]{ "/foo/*", createRequest("/foo /"), false },
+ new Object[]{ "/foo/*", createRequest("/foo%20"), false }, // already URL decoded, encoded would have been %2520
+ new Object[]{ "/foo/*", createRequest("/foo%20/"), false },
+ new Object[]{ "/foo/*", createRequest("/foo/%20/"), true },
+ new Object[]{ "/foo/*", createRequest("/foo/ /"), true }
+ )
+ .toArray(Object[][]::new);
+ }
+
+ public static HttpServletRequest createRequest(String requestUri) {
+ return createRequest(requestUri, "", requestUri);
+ }
+
+ public static HttpServletRequest createRequest(String requestUri, String servletPath, String pathInfo) {
+ HttpServletRequest request = createNiceMock(HttpServletRequest.class);
+ expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes();
+ expect(request.getRequestURI()).andReturn(requestUri).anyTimes();
+ expect(request.getServletPath()).andReturn(servletPath).anyTimes();
+ expect(request.getPathInfo()).andReturn(pathInfo).anyTimes();
+ replay(request);
+
+ return request;
+ }
+
+ @Before
+ public void setUp() {
+ filter = createTestInstance();
+ }
+
+ private PathMatchingFilter createTestInstance() {
+ final String NAME = "pathMatchingFilter";
+
+ PathMatchingFilter filter = new PathMatchingFilter() {
+ @Override
+ protected boolean isEnabled(ServletRequest request, ServletResponse response, String path, Object mappedValue) throws Exception {
+ return !path.equals(DISABLED_PATH);
+ }
+
+ @Override
+ protected boolean onPreHandle(ServletRequest request, ServletResponse response, Object mappedValue) throws Exception {
+ //simulate a subclass that handles the response itself (A 'false' return value indicates that the
+ //FilterChain should not continue to be executed)
+ //
+ //This method should only be called if the filter is enabled, so we know if the return value is
+ //false, then the filter was enabled. A true return value from 'onPreHandle' indicates this test
+ //filter was disabled or a path wasn't matched.
+ return false;
+ }
+ };
+ filter.setName(NAME);
+
+ return filter;
+ }
+
+ @Test
+ public void testBasicAssumptions() {
+ LOG.debug("Input pattern: [{}], input path: [{}].", this.pattern, this.request.getPathInfo());
+ boolean matchEnabled = filter.pathsMatch(this.pattern, this.request);
+ assertEquals("PathMatch can match URL end with multi Separator, ["+ this.pattern + "] - [" + this.request.getPathInfo() + "]", this.shouldMatch, matchEnabled);
+ verify(request);
+ }
+}
diff --git a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
index 963e89e..f3fd0c3 100644
--- a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
+++ b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
@@ -21,6 +21,7 @@
import org.apache.shiro.util.AntPathMatcher;
import org.apache.shiro.web.WebTest;
import org.apache.shiro.web.util.WebUtils;
+import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Test;
@@ -32,6 +33,7 @@
import javax.servlet.http.HttpServletResponse;
import static org.easymock.EasyMock.*;
+import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.*;
/**
@@ -242,4 +244,23 @@
assertNotNull(resolved);
verify(request);
}
+
+ @Test
+ public void testMultipleChainsPathEndsWithSlash() {
+ HttpServletRequest request = createNiceMock(HttpServletRequest.class);
+ HttpServletResponse response = createNiceMock(HttpServletResponse.class);
+ FilterChain chain = createNiceMock(FilterChain.class);
+
+ //Define the filter chain
+ resolver.getFilterChainManager().addToChain("/login", "authc");
+ resolver.getFilterChainManager().addToChain("/resource/*", "authcBasic");
+
+ expect(request.getServletPath()).andReturn("");
+ expect(request.getPathInfo()).andReturn("/resource/");
+ replay(request);
+
+ FilterChain resolved = resolver.getChain(request, response, chain);
+ assertThat(resolved, notNullValue());
+ verify(request);
+ }
}
