diff --git a/NOTICE b/NOTICE index afda18636c..7d8eb88bb5 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 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 f33f85e646..22e5cd2f7c 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 @@ public void setPathSeparator(String pathSeparator) { 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 @@ public boolean matchStart(String pattern, String path) { * false 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 @@ private boolean matchStrings(String pattern, String str) { * and 'path', but does not 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 0000000000..383686a5cb --- /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 Spring Framework's similar AntPathMatcherTests + */ +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 be1da71a54..aee80d1b91 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 String getPathWithinApplication(ServletRequest request) { */ 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; } /** @@ -149,7 +156,9 @@ protected boolean pathsMatch(String path, ServletRequest request) { * false 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 c35ab9b290..452d186a36 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 @@ public FilterChain getChain(ServletRequest request, ServletResponse response, Fi 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 boolean pathMatches(String pattern, String path) { 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 0000000000..82720adab8 --- /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: + * + *
+     * URL                 Must match pattern      Must not match pattern
+     * /foo/               /foo/*                  /foo* || /foo
+     * /foo/bar            /foo/*                  /foo* || /foo
+     * /foo                /foo                    /foo/*
+     * 
+ */ + @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 963e89e439..f3fd0c3c3e 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 @@ public void testGetChainEndWithMultiUrlSeparator() { 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); + } }