You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by bm...@apache.org on 2021/02/01 07:13:26 UTC
[shiro] branch master updated: Improving tests for path matching
logic
This is an automated email from the ASF dual-hosted git repository.
bmarwell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shiro.git
The following commit(s) were added to refs/heads/master by this push:
new 0842c27 Improving tests for path matching logic
0842c27 is described below
commit 0842c27fa72d0da5de0c5723a66d402fe20903df
Author: Brian Demers <bd...@apache.org>
AuthorDate: Fri Dec 18 11:43:11 2020 -0500
Improving tests for path matching logic
---
NOTICE | 8 +-
.../java/org/apache/shiro/util/AntPathMatcher.java | 54 ++--
.../org/apache/shiro/util/AntPathMatcherTests.java | 330 +++++++++++++++++++++
.../shiro/web/filter/PathMatchingFilter.java | 26 +-
.../mgt/PathMatchingFilterChainResolver.java | 44 +--
.../PathMatchingFilterParameterizedTest.java | 150 ++++++++++
.../mgt/PathMatchingFilterChainResolverTest.java | 19 ++
7 files changed, 575 insertions(+), 56 deletions(-)
diff --git a/NOTICE b/NOTICE
index 9d26a95..672ed80 100644
--- a/NOTICE
+++ b/NOTICE
@@ -9,7 +9,7 @@ on initial ideas from Dr. Heinz Kabutz's publicly posted version
available at http://www.javaspecialists.eu/archive/Issue015.html,
with continued modifications.
-Certain parts (StringUtils, IpAddressMatcher, 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).
+Certain parts (StringUtils, IpAddressMatcher, 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 4ba0695..29f1511 100644
--- a/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java
+++ b/core/src/main/java/org/apache/shiro/util/AntPathMatcher.java
@@ -79,8 +79,15 @@ public class AntPathMatcher implements PatternMatcher {
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);
}
@@ -108,12 +115,12 @@ public class AntPathMatcher implements PatternMatcher {
* <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;
@@ -395,33 +402,26 @@ public class AntPathMatcher implements PatternMatcher {
* 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);
-
- 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);
+ 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;
+
+ 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 7d4df31..11eec59 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
@@ -122,16 +122,24 @@ public abstract class PathMatchingFilter extends AdviceFilter implements PathCon
*/
protected boolean pathsMatch(String path, ServletRequest request) {
String requestURI = getPathWithinApplication(request);
- if (requestURI != null && !DEFAULT_PATH_SEPARATOR.equals(requestURI)
+
+ log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(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)
+ 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);
+ path = path.substring(0, path.length() - 1);
+ }
+ log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(requestURI));
+ match = pathsMatch(path, requestURI);
}
- log.trace("Attempting to match pattern '{}' with current requestURI '{}'...", path, Encode.forHtml(requestURI));
- return pathsMatch(path, requestURI);
+
+ return match;
}
/**
@@ -148,7 +156,9 @@ public abstract class PathMatchingFilter extends AdviceFilter implements PathCon
* <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 7583060..6d81e02 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
@@ -99,32 +99,34 @@ public class PathMatchingFilterChainResolver implements FilterChainResolver {
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);
+ }
}
}
@@ -162,4 +164,12 @@ public class PathMatchingFilterChainResolver implements FilterChainResolver {
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 90f5977..db4de61 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
@@ -30,6 +30,8 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -236,4 +238,21 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
assertNotNull(resolved);
verify(request).getServletPath();
}
+
+ @Test
+ public void testMultipleChainsPathEndsWithSlash() {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ //Define the filter chain
+ resolver.getFilterChainManager().addToChain("/login", "authc");
+ resolver.getFilterChainManager().addToChain("/resource/*", "authcBasic");
+
+ when(request.getServletPath()).thenReturn("");
+ when(request.getPathInfo()).thenReturn("/resource/");
+
+ FilterChain resolved = resolver.getChain(request, response, chain);
+ assertThat(resolved, notNullValue());
+ }
}