diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureTheRestPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureTheRestPathElement.java index ef6e48e3dfa..21d466096f0 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureTheRestPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureTheRestPathElement.java @@ -49,13 +49,8 @@ class CaptureTheRestPathElement extends PathElement { matchingContext.candidate[candidateIndex] != separator) { return false; } - while ((candidateIndex + 1) < matchingContext.candidateLength && - matchingContext.candidate[candidateIndex + 1] == separator) { - candidateIndex++; - } - if (matchingContext.determineRemaining) { + if (matchingContext.determineRemainingPath) { matchingContext.remainingPathIndex = matchingContext.candidateLength; - return true; } if (matchingContext.extractingVariables) { matchingContext.set(variableName, new String(matchingContext.candidate, candidateIndex, diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureVariablePathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureVariablePathElement.java index 6028f28dfdf..b26660d0f67 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureVariablePathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/CaptureVariablePathElement.java @@ -22,7 +22,8 @@ import org.springframework.web.util.patterns.PathPattern.MatchingContext; /** * A path element representing capturing a piece of the path as a variable. In the pattern - * '/foo/{bar}/goo' the {bar} is represented as a {@link CaptureVariablePathElement}. + * '/foo/{bar}/goo' the {bar} is represented as a {@link CaptureVariablePathElement}. There + * must be at least one character to bind to the variable. * * @author Andy Clement */ @@ -66,6 +67,10 @@ class CaptureVariablePathElement extends PathElement { @Override public boolean matches(int candidateIndex, MatchingContext matchingContext) { int nextPos = matchingContext.scanAhead(candidateIndex); + // There must be at least one character to capture: + if (nextPos == candidateIndex) { + return false; + } CharSequence candidateCapture = null; if (constraintPattern != null) { // TODO possible optimization - only regex match if rest of pattern matches? Benefit likely to vary pattern to pattern @@ -80,13 +85,18 @@ class CaptureVariablePathElement extends PathElement { } boolean match = false; if (next == null) { - if (matchingContext.determineRemaining && nextPos > candidateIndex) { + if (matchingContext.determineRemainingPath && nextPos > candidateIndex) { matchingContext.remainingPathIndex = nextPos; match = true; } else { // Needs to be at least one character #SPR15264 match = (nextPos == matchingContext.candidateLength && nextPos > candidateIndex); + if (!match && matchingContext.isAllowOptionalTrailingSlash()) { + match = (nextPos > candidateIndex) && + (nextPos + 1) == matchingContext.candidateLength && + matchingContext.candidate[nextPos] == separator; + } } } else { diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/InternalPathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/patterns/InternalPathPatternParser.java index 07415178e17..1c090a34e5c 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/InternalPathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/InternalPathPatternParser.java @@ -35,6 +35,11 @@ public class InternalPathPatternParser { // Is the parser producing case sensitive PathPattern matchers boolean caseSensitive = true; + // If true the PathPatterns produced by the parser will allow patterns + // that don't have a trailing slash to match paths that may or may not + // have a trailing slash + private boolean matchOptionalTrailingSlash = false; + // The input data for parsing private char[] pathPatternData; @@ -79,11 +84,14 @@ public class InternalPathPatternParser { * Create a PatternParser that will use the specified separator instead of * the default. * - * @param separator the path separator to look for when parsing. + * @param separator the path separator to look for when parsing + * @param caseSensitive true if PathPatterns should be sensitive to case + * @param matchOptionalTrailingSlash true if patterns without a trailing slash can match paths that do have a trailing slash */ - public InternalPathPatternParser(char separator, boolean caseSensitive) { + public InternalPathPatternParser(char separator, boolean caseSensitive, boolean matchOptionalTrailingSlash) { this.separator = separator; this.caseSensitive = caseSensitive; + this.matchOptionalTrailingSlash = matchOptionalTrailingSlash; } /** @@ -99,10 +107,6 @@ public class InternalPathPatternParser { if (pathPattern == null) { pathPattern = ""; } -// int starstar = pathPattern.indexOf("**"); -// if (starstar!=-1 && starstar!=pathPattern.length()-2) { -// throw new IllegalStateException("Not allowed ** unless at end of pattern: "+pathPattern); -// } pathPatternData = pathPattern.toCharArray(); pathPatternLength = pathPatternData.length; headPE = null; @@ -117,10 +121,6 @@ public class InternalPathPatternParser { if (pathElementStart != -1) { pushPathElement(createPathElement()); } - // Skip over multiple separators - while ((pos + 1) < pathPatternLength && pathPatternData[pos + 1] == separator) { - pos++; - } if (peekDoubleWildcard()) { pushPathElement(new WildcardTheRestPathElement(pos, separator)); pos += 2; @@ -195,7 +195,7 @@ public class InternalPathPatternParser { if (pathElementStart != -1) { pushPathElement(createPathElement()); } - return new PathPattern(pathPattern, headPE, separator, caseSensitive); + return new PathPattern(pathPattern, headPE, separator, caseSensitive, matchOptionalTrailingSlash); } /** diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/LiteralPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/LiteralPathElement.java index 1bc9e6feaa4..5a07ce13f7f 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/LiteralPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/LiteralPathElement.java @@ -69,12 +69,19 @@ class LiteralPathElement extends PathElement { } } if (next == null) { - if (matchingContext.determineRemaining && nextIfExistsIsSeparator(candidateIndex, matchingContext)) { + if (matchingContext.determineRemainingPath && nextIfExistsIsSeparator(candidateIndex, matchingContext)) { matchingContext.remainingPathIndex = candidateIndex; return true; } else { - return candidateIndex == matchingContext.candidateLength; + if (candidateIndex == matchingContext.candidateLength) { + return true; + } + else { + return matchingContext.isAllowOptionalTrailingSlash() && + (candidateIndex + 1) == matchingContext.candidateLength && + matchingContext.candidate[candidateIndex] == separator; + } } } else { diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPattern.java b/spring-web/src/main/java/org/springframework/web/util/patterns/PathPattern.java index 4a7b1308809..d31868057b3 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPattern.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/PathPattern.java @@ -76,6 +76,9 @@ public class PathPattern implements Comparable { /** Will this match candidates in a case sensitive way? (case sensitivity at parse time) */ private boolean caseSensitive; + /** If this pattern has no trailing slash, allow candidates to include one and still match successfully */ + boolean allowOptionalTrailingSlash; + /** How many variables are captured in this pattern */ private int capturedVariableCount; @@ -105,11 +108,12 @@ public class PathPattern implements Comparable { /** Does the pattern end with {*...} */ private boolean isCatchAll = false; - public PathPattern(String patternText, PathElement head, char separator, boolean caseSensitive) { + public PathPattern(String patternText, PathElement head, char separator, boolean caseSensitive, boolean allowOptionalTrailingSlash) { this.head = head; this.patternString = patternText; this.separator = separator; this.caseSensitive = caseSensitive; + this.allowOptionalTrailingSlash = allowOptionalTrailingSlash; // Compute fields for fast comparison PathElement s = head; while (s != null) { @@ -251,11 +255,15 @@ public class PathPattern implements Comparable { // assert this.matches(path) PathElement s = head; int separatorCount = 0; + boolean matchTheRest = false; // Find first path element that is pattern based while (s != null) { if (s instanceof SeparatorPathElement || s instanceof CaptureTheRestPathElement || s instanceof WildcardTheRestPathElement) { separatorCount++; + if (s instanceof WildcardTheRestPathElement || s instanceof CaptureTheRestPathElement) { + matchTheRest = true; + } } if (s.getWildcardCount() != 0 || s.getCaptureCount() != 0) { break; @@ -271,17 +279,15 @@ public class PathPattern implements Comparable { int pos = 0; while (separatorCount > 0 && pos < len) { if (path.charAt(pos++) == separator) { - // Skip any adjacent separators - while (pos < len && path.charAt(pos) == separator) { - pos++; - } separatorCount--; } } int end = len; // Trim trailing separators - while (end > 0 && path.charAt(end - 1) == separator) { - end--; + if (!matchTheRest) { + while (end > 0 && path.charAt(end - 1) == separator) { + end--; + } } // Check if multiple separators embedded in the resulting path, if so trim them out. // Example: aaa////bbb//ccc/d -> aaa/bbb/ccc/d @@ -425,7 +431,7 @@ public class PathPattern implements Comparable { boolean extractingVariables; - boolean determineRemaining = false; + boolean determineRemainingPath = false; // if determineRemaining is true, this is set to the position in // the candidate where the pattern finished matching - i.e. it @@ -438,11 +444,12 @@ public class PathPattern implements Comparable { this.extractingVariables = extractVariables; } - /** - * - */ public void setMatchAllowExtraPath() { - determineRemaining = true; + determineRemainingPath = true; + } + + public boolean isAllowOptionalTrailingSlash() { + return allowOptionalTrailingSlash; } public void setMatchStartMatching(boolean b) { diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternParser.java index 5c56a62f6f5..4f0970a39ac 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternParser.java @@ -32,6 +32,11 @@ public class PathPatternParser { // The expected path separator to split path elements during parsing, default '/' private char separator = DEFAULT_SEPARATOR; + + // If true the PathPatterns produced by the parser will allow patterns + // that don't have a trailing slash to match paths that may or may not + // have a trailing slash + private boolean matchOptionalTrailingSlash = false; /** * Create a path pattern parser that will use the default separator '/' when @@ -40,6 +45,18 @@ public class PathPatternParser { public PathPatternParser() { } + /** + * Control behavior of the path patterns produced by this parser. The default + * value for matchOptionalTrailingSlash is true but here it can be set to false. + * If true then PathPatterns without a trailing slash will match paths with or + * without a trailing slash. + * + * @param matchOptionalTrailingSlash boolean value to override the default value of true + */ + public void setMatchOptionalTrailingSlash(boolean matchOptionalTrailingSlash) { + this.matchOptionalTrailingSlash = matchOptionalTrailingSlash; + } + /** * Create a path pattern parser that will use the supplied separator when * parsing patterns. @@ -64,7 +81,7 @@ public class PathPatternParser { * @return a PathPattern for quickly matching paths against the specified path pattern */ public PathPattern parse(String pathPattern) { - InternalPathPatternParser ippp = new InternalPathPatternParser(separator, caseSensitive); + InternalPathPatternParser ippp = new InternalPathPatternParser(separator, caseSensitive, matchOptionalTrailingSlash); return ippp.parse(pathPattern); } diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/RegexPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/RegexPathElement.java index e1c35d71bc5..0c0b9ae0acd 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/RegexPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/RegexPathElement.java @@ -124,7 +124,7 @@ class RegexPathElement extends PathElement { boolean matches = m.matches(); if (matches) { if (next == null) { - if (matchingContext.determineRemaining && + if (matchingContext.determineRemainingPath && ((this.variableNames.size() == 0) ? true : p > candidateIndex)) { matchingContext.remainingPathIndex = p; matches = true; @@ -134,6 +134,11 @@ class RegexPathElement extends PathElement { // If pattern is capturing variables there must be some actual data to bind to them matches = (p == matchingContext.candidateLength && ((this.variableNames.size() == 0) ? true : p > candidateIndex)); + if (!matches && matchingContext.isAllowOptionalTrailingSlash()) { + matches = ((this.variableNames.size() == 0) ? true : p > candidateIndex) && + (p + 1) == matchingContext.candidateLength && + matchingContext.candidate[p] == separator; + } } } else { diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/SeparatorPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/SeparatorPathElement.java index f981cd1604a..ce972b95dc4 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/SeparatorPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/SeparatorPathElement.java @@ -39,30 +39,23 @@ class SeparatorPathElement extends PathElement { @Override public boolean matches(int candidateIndex, MatchingContext matchingContext) { boolean matched = false; - if (candidateIndex < matchingContext.candidateLength) { - if (matchingContext.candidate[candidateIndex] == separator) { - // Skip further separators in the path (they are all 'matched' - // by a single SeparatorPathElement) - while ((candidateIndex + 1) < matchingContext.candidateLength && - matchingContext.candidate[candidateIndex + 1] == separator) { - candidateIndex++; - } - if (next == null) { - if (matchingContext.determineRemaining) { - matchingContext.remainingPathIndex = candidateIndex + 1; - matched = true; - } - else { - matched = ((candidateIndex + 1) == matchingContext.candidateLength); - } + if (candidateIndex < matchingContext.candidateLength && + matchingContext.candidate[candidateIndex] == separator) { + if (next == null) { + if (matchingContext.determineRemainingPath) { + matchingContext.remainingPathIndex = candidateIndex + 1; + matched = true; } else { - candidateIndex++; - if (matchingContext.isMatchStartMatching && candidateIndex == matchingContext.candidateLength) { - return true; // no more data but matches up to this point - } - matched = next.matches(candidateIndex, matchingContext); + matched = ((candidateIndex + 1) == matchingContext.candidateLength); + } + } + else { + candidateIndex++; + if (matchingContext.isMatchStartMatching && candidateIndex == matchingContext.candidateLength) { + return true; // no more data but matches up to this point } + matched = next.matches(candidateIndex, matchingContext); } } return matched; diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/SingleCharWildcardedPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/SingleCharWildcardedPathElement.java index aff8b74cbad..4ca41b9b237 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/SingleCharWildcardedPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/SingleCharWildcardedPathElement.java @@ -76,12 +76,19 @@ class SingleCharWildcardedPathElement extends PathElement { } } if (next == null) { - if (matchingContext.determineRemaining && nextIfExistsIsSeparator(candidateIndex, matchingContext)) { + if (matchingContext.determineRemainingPath && nextIfExistsIsSeparator(candidateIndex, matchingContext)) { matchingContext.remainingPathIndex = candidateIndex; return true; } else { - return candidateIndex == matchingContext.candidateLength; + if (candidateIndex == matchingContext.candidateLength) { + return true; + } + else { + return matchingContext.isAllowOptionalTrailingSlash() && + (candidateIndex + 1) == matchingContext.candidateLength && + matchingContext.candidate[candidateIndex] == separator; + } } } else { diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardPathElement.java index a4cf8838cc4..826e89c3d71 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardPathElement.java @@ -20,7 +20,8 @@ import org.springframework.web.util.patterns.PathPattern.MatchingContext; /** * A wildcard path element. In the pattern '/foo/*/goo' the * is - * represented by a WildcardPathElement. + * represented by a WildcardPathElement. Within a path it matches at least + * one character but at the end of a path it can match zero characters. * * @author Andy Clement * @since 5.0 @@ -32,26 +33,38 @@ class WildcardPathElement extends PathElement { } /** - * Matching on a WildcardPathElement is quite straight forward. Just scan the - * candidate from the candidateIndex for the next separator or the end of the + * Matching on a WildcardPathElement is quite straight forward. Scan the + * candidate from the candidateIndex onwards for the next separator or the end of the * candidate. */ @Override public boolean matches(int candidateIndex, MatchingContext matchingContext) { int nextPos = matchingContext.scanAhead(candidateIndex); if (next == null) { - if (matchingContext.determineRemaining) { + if (matchingContext.determineRemainingPath) { matchingContext.remainingPathIndex = nextPos; return true; } else { - return (nextPos == matchingContext.candidateLength); + if (nextPos == matchingContext.candidateLength) { + return true; + } + else { + return matchingContext.isAllowOptionalTrailingSlash() && // if optional slash is on... + nextPos > candidateIndex && // and there is at least one character to match the *... + (nextPos + 1) == matchingContext.candidateLength && // and the nextPos is the end of the candidate... + matchingContext.candidate[nextPos] == separator; // and the final character is a separator + } } } - else { + else { if (matchingContext.isMatchStartMatching && nextPos == matchingContext.candidateLength) { return true; // no more data but matches up to this point } + // Within a path (e.g. /aa/*/bb) there must be at least one character to match the wildcard + if (nextPos == candidateIndex) { + return false; + } return next.matches(nextPos, matchingContext); } } diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardTheRestPathElement.java b/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardTheRestPathElement.java index 36df46349b1..ab64fae4546 100644 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardTheRestPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/patterns/WildcardTheRestPathElement.java @@ -38,7 +38,7 @@ class WildcardTheRestPathElement extends PathElement { matchingContext.candidate[candidateIndex] != separator) { return false; } - if (matchingContext.determineRemaining) { + if (matchingContext.determineRemainingPath) { matchingContext.remainingPathIndex = matchingContext.candidateLength; } return true; diff --git a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternMatcherTests.java b/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternMatcherTests.java index c55739a280c..6073ca2f1a5 100644 --- a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternMatcherTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternMatcherTests.java @@ -38,6 +38,198 @@ import static org.junit.Assert.*; */ public class PathPatternMatcherTests { + @Test + public void basicMatching() { + checkMatches(null, null); + checkMatches("", ""); + checkMatches("", null); + checkNoMatch("/abc", null); + checkMatches(null, ""); + checkNoMatch(null, "/abc"); + checkMatches("/", "/"); + checkNoMatch("/", "/a"); + checkMatches("f", "f"); + checkMatches("/foo", "/foo"); + checkMatches("/foo/", "/foo/"); + checkMatches("/foo/bar", "/foo/bar"); + checkMatches("foo/bar", "foo/bar"); + checkMatches("/foo/bar/", "/foo/bar/"); + checkMatches("foo/bar/", "foo/bar/"); + checkMatches("/foo/bar/woo", "/foo/bar/woo"); + checkNoMatch("foo", "foobar"); + checkMatches("/foo/bar", "/foo/bar"); + checkNoMatch("/foo/bar", "/foo/baz"); + // TODO Need more tests for escaped separators in path patterns and paths? + checkMatches("/foo\\/bar", "/foo\\/bar"); // chain string is Separator(/) Literal(foo\) Separator(/) Literal(bar) + } + + @Test + public void optionalTrailingSeparators() { + // LiteralPathElement + PathPattern pp = parse("/resource"); + assertTrue(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parse("/resource/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // SingleCharWildcardPathElement + pp = parse("/res?urce"); + assertTrue(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parse("/res?urce/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // CaptureVariablePathElement + pp = parse("/{var}"); + assertTrue(pp.matches("/resource")); + assertEquals("resource",pp.matchAndExtract("/resource").get("var")); + assertTrue(pp.matches("/resource/")); + assertEquals("resource",pp.matchAndExtract("/resource/").get("var")); + assertFalse(pp.matches("/resource//")); + pp = parse("/{var}/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertEquals("resource",pp.matchAndExtract("/resource/").get("var")); + assertFalse(pp.matches("/resource//")); + + // CaptureTheRestPathElement + pp = parse("/{*var}"); + assertTrue(pp.matches("/resource")); + assertEquals("/resource",pp.matchAndExtract("/resource").get("var")); + assertTrue(pp.matches("/resource/")); + assertEquals("/resource/",pp.matchAndExtract("/resource/").get("var")); + assertTrue(pp.matches("/resource//")); + assertEquals("/resource//",pp.matchAndExtract("/resource//").get("var")); + assertTrue(pp.matches("//resource//")); + assertEquals("//resource//",pp.matchAndExtract("//resource//").get("var")); + + // WildcardTheRestPathElement + pp = parse("/**"); + assertTrue(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertTrue(pp.matches("/resource//")); + assertTrue(pp.matches("//resource//")); + + // WildcardPathElement + pp = parse("/*"); + assertTrue(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parse("/*/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // RegexPathElement + pp = parse("/{var1}_{var2}"); + assertTrue(pp.matches("/res1_res2")); + assertEquals("res1",pp.matchAndExtract("/res1_res2").get("var1")); + assertEquals("res2",pp.matchAndExtract("/res1_res2").get("var2")); + assertTrue(pp.matches("/res1_res2/")); + assertEquals("res1",pp.matchAndExtract("/res1_res2/").get("var1")); + assertEquals("res2",pp.matchAndExtract("/res1_res2/").get("var2")); + assertFalse(pp.matches("/res1_res2//")); + pp = parse("/{var1}_{var2}/"); + assertFalse(pp.matches("/res1_res2")); + assertTrue(pp.matches("/res1_res2/")); + assertEquals("res1",pp.matchAndExtract("/res1_res2/").get("var1")); + assertEquals("res2",pp.matchAndExtract("/res1_res2/").get("var2")); + assertFalse(pp.matches("/res1_res2//")); + pp = parse("/{var1}*"); + assertTrue(pp.matches("/a")); + assertTrue(pp.matches("/a/")); + assertFalse(pp.matches("/")); // no characters for var1 + assertFalse(pp.matches("//")); // no characters for var1 + + // Now with trailing matching turned OFF + PathPatternParser parser = new PathPatternParser(); + parser.setMatchOptionalTrailingSlash(false); + // LiteralPathElement + pp = parser.parse("/resource"); + assertTrue(pp.matches("/resource")); + assertFalse(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parser.parse("/resource/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // SingleCharWildcardPathElement + pp = parser.parse("/res?urce"); + assertTrue(pp.matches("/resource")); + assertFalse(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parser.parse("/res?urce/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // CaptureVariablePathElement + pp = parser.parse("/{var}"); + assertTrue(pp.matches("/resource")); + assertEquals("resource",pp.matchAndExtract("/resource").get("var")); + assertFalse(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parser.parse("/{var}/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertEquals("resource",pp.matchAndExtract("/resource/").get("var")); + assertFalse(pp.matches("/resource//")); + + // CaptureTheRestPathElement + pp = parser.parse("/{*var}"); + assertTrue(pp.matches("/resource")); + assertEquals("/resource",pp.matchAndExtract("/resource").get("var")); + assertTrue(pp.matches("/resource/")); + assertEquals("/resource/",pp.matchAndExtract("/resource/").get("var")); + assertTrue(pp.matches("/resource//")); + assertEquals("/resource//",pp.matchAndExtract("/resource//").get("var")); + assertTrue(pp.matches("//resource//")); + assertEquals("//resource//",pp.matchAndExtract("//resource//").get("var")); + + // WildcardTheRestPathElement + pp = parser.parse("/**"); + assertTrue(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertTrue(pp.matches("/resource//")); + assertTrue(pp.matches("//resource//")); + + // WildcardPathElement + pp = parser.parse("/*"); + assertTrue(pp.matches("/resource")); + assertFalse(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + pp = parser.parse("/*/"); + assertFalse(pp.matches("/resource")); + assertTrue(pp.matches("/resource/")); + assertFalse(pp.matches("/resource//")); + + // RegexPathElement + pp = parser.parse("/{var1}_{var2}"); + assertTrue(pp.matches("/res1_res2")); + assertEquals("res1",pp.matchAndExtract("/res1_res2").get("var1")); + assertEquals("res2",pp.matchAndExtract("/res1_res2").get("var2")); + assertFalse(pp.matches("/res1_res2/")); + assertFalse(pp.matches("/res1_res2//")); + pp = parser.parse("/{var1}_{var2}/"); + assertFalse(pp.matches("/res1_res2")); + assertTrue(pp.matches("/res1_res2/")); + assertEquals("res1",pp.matchAndExtract("/res1_res2/").get("var1")); + assertEquals("res2",pp.matchAndExtract("/res1_res2/").get("var2")); + assertFalse(pp.matches("/res1_res2//")); + pp = parser.parse("/{var1}*"); + assertTrue(pp.matches("/a")); + assertFalse(pp.matches("/a/")); + assertFalse(pp.matches("/")); // no characters for var1 + assertFalse(pp.matches("//")); // no characters for var1 + } + @Test public void pathRemainderBasicCases_spr15336() { // Cover all PathElement kinds @@ -92,31 +284,6 @@ public class PathPatternMatcherTests { assertNull(parse("").getPathRemaining(null).getPathRemaining()); } - @Test - public void basicMatching() { - checkMatches(null, null); - checkMatches("", ""); - checkMatches("", null); - checkNoMatch("/abc", null); - checkMatches(null, ""); - checkNoMatch(null, "/abc"); - checkMatches("/", "/"); - checkNoMatch("/", "/a"); - checkMatches("f", "f"); - checkMatches("/foo", "/foo"); - checkMatches("/foo/", "/foo/"); - checkMatches("/foo/bar", "/foo/bar"); - checkMatches("foo/bar", "foo/bar"); - checkMatches("/foo/bar/", "/foo/bar/"); - checkMatches("foo/bar/", "foo/bar/"); - checkMatches("/foo/bar/woo", "/foo/bar/woo"); - checkNoMatch("foo", "foobar"); - checkMatches("/foo/bar", "/foo/bar"); - checkNoMatch("/foo/bar", "/foo/baz"); - // TODO Need more tests for escaped separators in path patterns and paths? - checkMatches("/foo\\/bar", "/foo\\/bar"); // chain string is Separator(/) Literal(foo\) Separator(/) Literal(bar) - } - @Test public void questionMarks() { checkNoMatch("a", "ab"); @@ -144,38 +311,62 @@ public class PathPatternMatcherTests { checkCapture("/customer/{*something}", "/customer/aa/bb/cc", "something", "/aa/bb/cc"); checkCapture("/customer/{*something}", "/customer/", "something", "/"); - checkCapture("/customer/////{*something}", "/customer/", "something", "/"); - checkCapture("/customer/{*something}", "/customer//////99", "something", "/99"); - checkCapture("/customer///{*something}", "/customer//////99", "something", "/99"); + checkCapture("/customer/////{*something}", "/customer/////", "something", "/"); + checkCapture("/customer/////{*something}", "/customer//////", "something", "//"); + checkCapture("/customer//////{*something}", "/customer//////99", "something", "/99"); + checkCapture("/customer//////{*something}", "/customer//////99", "something", "/99"); checkCapture("/customer/{*something}", "/customer", "something", ""); checkCapture("/{*something}", "", "something", ""); + checkCapture("/customer/{*something}", "/customer//////99", "something", "//////99"); } @Test - public void multipleSelectorsInPattern() { - checkMatches("///abc", "/abc"); - checkMatches("//", "/"); - checkMatches("abc", "abc"); - checkMatches("///abc//d/e", "/abc/d/e"); - checkMatches("///abc//{def}//////xyz", "/abc/foo/xyz"); + public void multipleSeparatorsInPattern() { + PathPattern pp = parse("a//b//c"); + assertEquals("Literal(a) Separator(/) Separator(/) Literal(b) Separator(/) Separator(/) Literal(c)",pp.toChainString()); + assertTrue(pp.matches("a//b//c")); + assertEquals("Literal(a) Separator(/) WildcardTheRest(/**)",parse("a//**").toChainString()); + checkMatches("///abc", "///abc"); + checkNoMatch("///abc", "/abc"); + checkNoMatch("//", "/"); + checkMatches("//", "//"); + checkNoMatch("///abc//d/e", "/abc/d/e"); + checkMatches("///abc//d/e", "///abc//d/e"); + checkNoMatch("///abc//{def}//////xyz", "/abc/foo/xyz"); + checkMatches("///abc//{def}//////xyz", "///abc//p//////xyz"); } @Test public void multipleSelectorsInPath() { - checkMatches("/abc", "////abc"); - checkMatches("/", "//"); - checkMatches("/abc//def///ghi", "/abc/def/ghi"); + checkNoMatch("/abc", "////abc"); + checkNoMatch("/", "//"); + checkNoMatch("/abc/def/ghi", "/abc//def///ghi"); + checkNoMatch("/abc", "////abc"); + checkMatches("////abc", "////abc"); + checkNoMatch("/", "//"); + checkNoMatch("/abc//def///ghi", "/abc/def/ghi"); + checkMatches("/abc//def///ghi", "/abc//def///ghi"); } @Test - public void multipleSelectorsInPatternAndPath() { - checkMatches("///one///two///three", "//one/////two///////three"); - checkMatches("//one//two//three", "/one/////two/three"); - checkCapture("///{foo}///bar", "/one/bar", "foo", "one"); + public void multipleSeparatorsInPatternAndPath() { + checkNoMatch("///one///two///three", "//one/////two///////three"); + checkMatches("//one/////two///////three", "//one/////two///////three"); + checkNoMatch("//one//two//three", "/one/////two/three"); + checkMatches("/one/////two/three", "/one/////two/three"); + checkCapture("///{foo}///bar", "///one///bar", "foo", "one"); } - + @Test public void wildcards() { + checkMatches("/*/bar", "/foo/bar"); + checkNoMatch("/*/bar", "/foo/baz"); + checkNoMatch("/*/bar", "//bar"); + checkMatches("/f*/bar", "/foo/bar"); + checkMatches("/*/bar", "/foo/bar"); + checkMatches("a/*","a/"); + + checkMatches("/*","/"); checkMatches("/*/bar", "/foo/bar"); checkNoMatch("/*/bar", "/foo/baz"); checkMatches("/f*/bar", "/foo/bar"); @@ -183,20 +374,19 @@ public class PathPatternMatcherTests { checkMatches("/a*b*c*d/bar", "/abcd/bar"); checkMatches("*a*", "testa"); checkMatches("a/*", "a/"); + checkNoMatch("a/*", "a//"); // trailing slash, so is allowed + checkMatches("a/*", "a/a/"); // trailing slash, so is allowed + PathPatternParser ppp = new PathPatternParser(); + ppp.setMatchOptionalTrailingSlash(false); + assertFalse(ppp.parse("a/*").matches("a//")); checkMatches("a/*", "a/a"); - checkNoMatch("a/*", "a/a/"); - + checkMatches("a/*", "a/a/"); // trailing slash is optional checkMatches("/resource/**", "/resource"); checkNoMatch("/resource/**", "/resourceX"); checkNoMatch("/resource/**", "/resourceX/foobar"); checkMatches("/resource/**", "/resource/foobar"); } - @Test - public void trailingSeparators() { - checkNoMatch("aaa/", "aaa"); - } - @Test public void constrainedMatches() { checkCapture("{foo:[0-9]*}", "123", "foo", "123"); @@ -246,7 +436,8 @@ public class PathPatternMatcherTests { checkMatches("test*aaa", "testblaaaa"); checkNoMatch("test*", "tst"); checkNoMatch("test*", "tsttest"); - checkNoMatch("test*", "test/"); + checkMatches("test*", "test/"); // trailing slash is optional + checkMatches("test*", "test"); // trailing slash is optional checkNoMatch("test*", "test/t"); checkNoMatch("test/*", "test"); checkNoMatch("*test*", "tsttst"); @@ -265,8 +456,6 @@ public class PathPatternMatcherTests { checkMatches("/**", ""); checkMatches("/books/**", "/books"); - checkMatches("/books////**", "/books"); - checkMatches("/books////**", "/books////"); checkMatches("/**", "/testing/testing"); checkMatches("/*/**", "/testing/testing"); checkMatches("/bla*bla/test", "/blaXXXbla/test"); @@ -314,6 +503,12 @@ public class PathPatternMatcherTests { @Test public void matchStart() { + checkStartNoMatch("test/*/","test//"); + checkStartMatches("test/*","test/abc"); + checkStartMatches("test/*/def","test/abc/def"); + checkStartNoMatch("test/*/def","test//"); + checkStartNoMatch("test/*/def","test//def"); + checkStartMatches("test/{a}_{b}/foo", "test/a_b"); checkStartMatches("test/?/abc", "test/a"); checkStartMatches("test/{*foobar}", "test/"); @@ -357,7 +552,8 @@ public class PathPatternMatcherTests { checkStartMatches("*.*", "test.test.test"); checkStartMatches("test*aaa", "testblaaaa"); checkStartNoMatch("test*", "tst"); - checkStartNoMatch("test*", "test/"); + checkStartMatches("test*", "test/"); // trailing slash is optional + checkStartMatches("test*", "test"); checkStartNoMatch("test*", "tsttest"); checkStartNoMatch("test*", "test/t"); checkStartMatches("test/*", "test"); @@ -398,7 +594,7 @@ public class PathPatternMatcherTests { "/XXXblaXXXX/testing/bla/testing/testing.jpg"); checkStartMatches("/abc/{foo}", "/abc/def"); - checkStartNoMatch("/abc/{foo}", "/abc/def/"); + checkStartMatches("/abc/{foo}", "/abc/def/"); // trailing slash is optional checkStartMatches("/abc/{foo}/", "/abc/def/"); checkStartNoMatch("/abc/{foo}/", "/abc/def/ghi"); checkStartMatches("/abc/{foo}/", "/abc/def"); @@ -556,8 +752,8 @@ public class PathPatternMatcherTests { @Test public void extractPathWithinPattern_spr15259() { + checkExtractPathWithinPattern("/**","//","/"); checkExtractPathWithinPattern("/**","/",""); - checkExtractPathWithinPattern("/**","//",""); checkExtractPathWithinPattern("/**","",""); checkExtractPathWithinPattern("/**","/foobar","foobar"); } @@ -579,10 +775,8 @@ public class PathPatternMatcherTests { checkExtractPathWithinPattern("/a/b/c*d*/*.html", "/a/b/cod/foo.html", "cod/foo.html"); checkExtractPathWithinPattern("a/{foo}/b/{bar}", "a/c/b/d", "c/b/d"); checkExtractPathWithinPattern("a/{foo}_{bar}/d/e", "a/b_c/d/e", "b_c/d/e"); - checkExtractPathWithinPattern("aaa//*///ccc///ddd", "aaa/bbb/ccc/ddd", "bbb/ccc/ddd"); - checkExtractPathWithinPattern("aaa/*/ccc/ddd", "aaa//bbb//ccc/ddd", "bbb/ccc/ddd"); + checkExtractPathWithinPattern("aaa//*///ccc///ddd", "aaa//bbb///ccc///ddd", "bbb/ccc/ddd"); checkExtractPathWithinPattern("aaa//*///ccc///ddd", "aaa//bbb//ccc/ddd", "bbb/ccc/ddd"); - checkExtractPathWithinPattern("aaa//*///ccc///ddd", "aaa/////bbb//ccc/ddd", "bbb/ccc/ddd"); checkExtractPathWithinPattern("aaa/c*/ddd/", "aaa/ccc///ddd///", "ccc/ddd"); checkExtractPathWithinPattern("", "", ""); checkExtractPathWithinPattern("/", "", ""); @@ -604,13 +798,18 @@ public class PathPatternMatcherTests { pp = new PathPatternParser().parse("/{foo}/{bar}"); assertTrue(pp.matches("/abc/def")); + assertFalse(pp.matches("/def")); + assertFalse(pp.matches("/")); assertFalse(pp.matches("//def")); assertFalse(pp.matches("//")); + pp = parse("/{foo}/boo"); assertTrue(pp.matches("/abc/boo")); assertTrue(pp.matches("/a/boo")); + assertFalse(pp.matches("/boo")); assertFalse(pp.matches("//boo")); + pp = parse("/{foo}*"); assertTrue(pp.matches("/abc")); @@ -630,11 +829,14 @@ public class PathPatternMatcherTests { checkCapture("/{foo:[a-z][a-z]}{bar:[a-z]}", "/abc", "foo", "ab", "bar", "c"); // Only patterns not capturing variables cannot match against just / - pp = new PathPatternParser().parse("/****"); + PathPatternParser ppp = new PathPatternParser(); + ppp.setMatchOptionalTrailingSlash(true); + pp = ppp.parse("/****"); assertTrue(pp.matches("/abcdef")); assertTrue(pp.matches("/")); + assertTrue(pp.matches("/")); assertTrue(pp.matches("//")); - + // Confirming AntPathMatcher behaviour: assertFalse(new AntPathMatcher().match("/{foo}", "/")); assertTrue(new AntPathMatcher().match("/{foo}", "/a")); @@ -682,6 +884,7 @@ public class PathPatternMatcherTests { checkCapture("/{bla}.*", "/testing.html", "bla", "testing"); Map extracted = checkCapture("/abc", "/abc"); assertEquals(0, extracted.size()); + checkCapture("/{bla}/foo","/a/foo"); } @Test @@ -990,6 +1193,7 @@ public class PathPatternMatcherTests { private PathPattern parse(String path) { PathPatternParser pp = new PathPatternParser(); + pp.setMatchOptionalTrailingSlash(true); return pp.parse(path); } @@ -998,18 +1202,21 @@ public class PathPatternMatcherTests { private void checkMatches(String uriTemplate, String path) { PathPatternParser parser = (separator == PathPatternParser.DEFAULT_SEPARATOR ? new PathPatternParser() : new PathPatternParser(separator)); + parser.setMatchOptionalTrailingSlash(true); PathPattern p = parser.parse(uriTemplate); assertTrue(p.matches(path)); } private void checkStartNoMatch(String uriTemplate, String path) { PathPatternParser p = new PathPatternParser(); + p.setMatchOptionalTrailingSlash(true); PathPattern pattern = p.parse(uriTemplate); assertFalse(pattern.matchStart(path)); } private void checkStartMatches(String uriTemplate, String path) { PathPatternParser p = new PathPatternParser(); + p.setMatchOptionalTrailingSlash(true); PathPattern pattern = p.parse(uriTemplate); assertTrue(pattern.matchStart(path)); } diff --git a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternParserTests.java b/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternParserTests.java index 9729648dc06..116479e2aba 100644 --- a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternParserTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternParserTests.java @@ -41,7 +41,7 @@ public class PathPatternParserTests { checkStructure("foo"); checkStructure("foo/"); checkStructure("/foo/"); - checkStructure("//"); + checkStructure(""); } @Test @@ -49,7 +49,7 @@ public class PathPatternParserTests { p = checkStructure("?"); assertPathElements(p, SingleCharWildcardedPathElement.class); checkStructure("/?/"); - checkStructure("//?abc?/"); + checkStructure("/?abc?/"); } @Test @@ -175,7 +175,7 @@ public class PathPatternParserTests { p = checkStructure("{foo}"); assertEquals(CaptureVariablePathElement.class.getName(), p.getHeadSection().getClass().getName()); checkStructure("/{foo}"); - checkStructure("//{f}/"); + checkStructure("/{f}/"); checkStructure("/{foo}/{bar}/{wibble}"); } @@ -208,13 +208,13 @@ public class PathPatternParserTests { checkError("{", 1, PatternMessage.MISSING_CLOSE_CAPTURE); checkError("{abc", 4, PatternMessage.MISSING_CLOSE_CAPTURE); checkError("{/}", 1, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("//{", 3, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("/{", 2, PatternMessage.MISSING_CLOSE_CAPTURE); checkError("}", 0, PatternMessage.MISSING_OPEN_CAPTURE); checkError("/}", 1, PatternMessage.MISSING_OPEN_CAPTURE); checkError("def}", 3, PatternMessage.MISSING_OPEN_CAPTURE); - checkError("//{/}", 3, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("//{{/}", 3, PatternMessage.ILLEGAL_NESTED_CAPTURE); - checkError("//{abc{/}", 6, PatternMessage.ILLEGAL_NESTED_CAPTURE); + checkError("/{/}", 2, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("/{{/}", 2, PatternMessage.ILLEGAL_NESTED_CAPTURE); + checkError("/{abc{/}", 5, PatternMessage.ILLEGAL_NESTED_CAPTURE); checkError("/{0abc}/abc", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR); checkError("/{a?bc}/abc", 3, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); checkError("/{abc}_{abc}", 1, PatternMessage.ILLEGAL_DOUBLE_CAPTURE); @@ -294,15 +294,19 @@ public class PathPatternParserTests { @Test public void multipleSeparatorPatterns() { p = checkStructure("///aaa"); - assertEquals(4, p.getNormalizedLength()); - assertPathElements(p, SeparatorPathElement.class, LiteralPathElement.class); + assertEquals(6, p.getNormalizedLength()); + assertPathElements(p, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, LiteralPathElement.class); p = checkStructure("///aaa////aaa/b"); - assertEquals(10, p.getNormalizedLength()); - assertPathElements(p, SeparatorPathElement.class, LiteralPathElement.class, - SeparatorPathElement.class, LiteralPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); + assertEquals(15, p.getNormalizedLength()); + assertPathElements(p, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, LiteralPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, SeparatorPathElement.class, SeparatorPathElement.class, + LiteralPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); p = checkStructure("/////**"); - assertEquals(1, p.getNormalizedLength()); - assertPathElements(p, WildcardTheRestPathElement.class); + assertEquals(5, p.getNormalizedLength()); + assertPathElements(p, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, SeparatorPathElement.class, WildcardTheRestPathElement.class); } @Test