Support optional trailing separator on path pattern matching

This commit adds the ability for path patterns to automatically
match a trailing separator (so there is no need to add two
variants of a pattern, one with and one without). This behaviour
is currently turned off but a simple tweak in PathPatternParser
can make it the default. If made default other parts of Spring
may need altering (simplifying hopefully) to cope with this.

Issue: SPR-15260
This commit is contained in:
Andy Clement
2017-04-19 11:59:21 -07:00
parent e93e49f268
commit cd86558811
13 changed files with 404 additions and 139 deletions

View File

@@ -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,

View File

@@ -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 {

View File

@@ -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);
}
/**

View File

@@ -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 {

View File

@@ -76,6 +76,9 @@ public class PathPattern implements Comparable<PathPattern> {
/** 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<PathPattern> {
/** 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<PathPattern> {
// 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<PathPattern> {
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<PathPattern> {
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<PathPattern> {
this.extractingVariables = extractVariables;
}
/**
*
*/
public void setMatchAllowExtraPath() {
determineRemaining = true;
determineRemainingPath = true;
}
public boolean isAllowOptionalTrailingSlash() {
return allowOptionalTrailingSlash;
}
public void setMatchStartMatching(boolean b) {

View File

@@ -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);
}

View File

@@ -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 {

View File

@@ -39,31 +39,24 @@ 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;
}

View File

@@ -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 {

View File

@@ -20,7 +20,8 @@ import org.springframework.web.util.patterns.PathPattern.MatchingContext;
/**
* A wildcard path element. In the pattern '/foo/&ast;/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);
}
}

View File

@@ -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;