diff --git a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-requestmapping.adoc b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-requestmapping.adoc index 5b4e756899a..67874085f08 100644 --- a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-requestmapping.adoc +++ b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-requestmapping.adoc @@ -93,6 +93,10 @@ You can map requests by using glob patterns and wildcards: |=== |Pattern |Description |Example +| `spring` +| Literal pattern +| `+"/spring"+` matches `+"/spring"+` + | `+?+` | Matches one character | `+"/pages/t?st.html"+` matches `+"/pages/test.html"+` and `+"/pages/t3st.html"+` @@ -104,23 +108,41 @@ You can map requests by using glob patterns and wildcards: `+"/projects/*/versions"+` matches `+"/projects/spring/versions"+` but does not match `+"/projects/spring/boot/versions"+` | `+**+` -| Matches zero or more path segments until the end of the path +| Matches zero or more path segments | `+"/resources/**"+` matches `+"/resources/file.png"+` and `+"/resources/images/file.png"+` -`+"/resources/**/file.png"+` is invalid as `+**+` is only allowed at the end of the path. +`+"/**/resources"+` matches `+"/spring/resources"+` and `+"/spring/framework/resources"+` + +`+"/resources/**/file.png"+` is invalid as `+**+` is not allowed in the middle of the path. + +`+"/**/{name}/resources"+` is invalid as only a literal pattern is allowed right after `+**+`. +`+"/**/project/{project}/resources"+` is allowed. + +`+"/**/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern. | `+{name}+` | Matches a path segment and captures it as a variable named "name" | `+"/projects/{project}/versions"+` matches `+"/projects/spring/versions"+` and captures `+project=spring+` +`+"/projects/{project}/versions"+` does not match `+"/projects/spring/framework/versions"+` as it captures a single path segment. + | `+{name:[a-z]+}+` | Matches the regexp `+"[a-z]+"+` as a path variable named "name" | `+"/projects/{project:[a-z]+}/versions"+` matches `+"/projects/spring/versions"+` but not `+"/projects/spring1/versions"+` | `+{*path}+` -| Matches zero or more path segments until the end of the path and captures it as a variable named "path" +| Matches zero or more path segments and captures it as a variable named "path" | `+"/resources/{*file}"+` matches `+"/resources/images/file.png"+` and captures `+file=/images/file.png+` +`+"{*path}/resources"+` matches `+"/spring/framework/resources"+` and captures `+path=/spring/framework+` + +`+"/resources/{*path}/file.png"+` is invalid as `{*path}` is not allowed in the middle of the path. + +`+"/{*path}/{name}/resources"+` is invalid as only a literal pattern is allowed right after `{*path}`. +`+"/{*path}/project/{project}/resources"+` is allowed. + +`+"/{*path}/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern. + |=== Captured URI variables can be accessed with `@PathVariable`, as the following example shows: diff --git a/framework-docs/modules/ROOT/pages/web/webmvc/mvc-controller/ann-requestmapping.adoc b/framework-docs/modules/ROOT/pages/web/webmvc/mvc-controller/ann-requestmapping.adoc index 94ff1b4f420..cd15384c34e 100644 --- a/framework-docs/modules/ROOT/pages/web/webmvc/mvc-controller/ann-requestmapping.adoc +++ b/framework-docs/modules/ROOT/pages/web/webmvc/mvc-controller/ann-requestmapping.adoc @@ -88,37 +88,71 @@ Kotlin:: == URI patterns [.small]#xref:web/webflux/controller/ann-requestmapping.adoc#webflux-ann-requestmapping-uri-templates[See equivalent in the Reactive stack]# -`@RequestMapping` methods can be mapped using URL patterns. There are two alternatives: - -* `PathPattern` -- a pre-parsed pattern matched against the URL path also pre-parsed as -`PathContainer`. Designed for web use, this solution deals effectively with encoding and -path parameters, and matches efficiently. -* `AntPathMatcher` -- match String patterns against a String path. This is the original -solution also used in Spring configuration to select resources on the classpath, on the -filesystem, and other locations. It is less efficient and the String path input is a +`@RequestMapping` methods can be mapped using URL patterns. +Spring MVC is using `PathPattern` -- a pre-parsed pattern matched against the URL path also pre-parsed as `PathContainer`. +Designed for web use, this solution deals effectively with encoding and path parameters, and matches efficiently. +See xref:web/webmvc/mvc-config/path-matching.adoc[MVC config] for customizations of path matching options. + +NOTE: the `AntPathMatcher` variant is now deprecated because it is less efficient and the String path input is a challenge for dealing effectively with encoding and other issues with URLs. -`PathPattern` is the recommended solution for web applications and it is the only choice in -Spring WebFlux. It was enabled for use in Spring MVC from version 5.3 and is enabled by -default from version 6.0. See xref:web/webmvc/mvc-config/path-matching.adoc[MVC config] for -customizations of path matching options. - -`PathPattern` supports the same pattern syntax as `AntPathMatcher`. In addition, it also -supports the capturing pattern, for example, `+{*spring}+`, for matching 0 or more path segments -at the end of a path. `PathPattern` also restricts the use of `+**+` for matching multiple -path segments such that it's only allowed at the end of a pattern. This eliminates many -cases of ambiguity when choosing the best matching pattern for a given request. -For full pattern syntax please refer to -{spring-framework-api}/web/util/pattern/PathPattern.html[PathPattern] and -{spring-framework-api}/util/AntPathMatcher.html[AntPathMatcher]. - -Some example patterns: - -* `+"/resources/ima?e.png"+` - match one character in a path segment -* `+"/resources/*.png"+` - match zero or more characters in a path segment -* `+"/resources/**"+` - match multiple path segments -* `+"/projects/{project}/versions"+` - match a path segment and capture it as a variable -* `++"/projects/{project:[a-z]+}/versions"++` - match and capture a variable with a regex +You can map requests by using glob patterns and wildcards: + +[cols="2,3,5"] +|=== +|Pattern |Description |Example + +| `spring` +| Literal pattern +| `+"/spring"+` matches `+"/spring"+` + +| `+?+` +| Matches one character +| `+"/pages/t?st.html"+` matches `+"/pages/test.html"+` and `+"/pages/t3st.html"+` + +| `+*+` +| Matches zero or more characters within a path segment +| `+"/resources/*.png"+` matches `+"/resources/file.png"+` + +`+"/projects/*/versions"+` matches `+"/projects/spring/versions"+` but does not match `+"/projects/spring/boot/versions"+` + +| `+**+` +| Matches zero or more path segments +| `+"/resources/**"+` matches `+"/resources/file.png"+` and `+"/resources/images/file.png"+` + +`+"/**/resources"+` matches `+"/spring/resources"+` and `+"/spring/framework/resources"+` + +`+"/resources/**/file.png"+` is invalid as `+**+` is not allowed in the middle of the path. + +`+"/**/{name}/resources"+` is invalid as only a literal pattern is allowed right after `+**+`. +`+"/**/project/{project}/resources"+` is allowed. + +`+"/**/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern. + +| `+{name}+` +| Matches a path segment and captures it as a variable named "name" +| `+"/projects/{project}/versions"+` matches `+"/projects/spring/versions"+` and captures `+project=spring+` + +`+"/projects/{project}/versions"+` does not match `+"/projects/spring/framework/versions"+` as it captures a single path segment. + +| `+{name:[a-z]+}+` +| Matches the regexp `+"[a-z]+"+` as a path variable named "name" +| `+"/projects/{project:[a-z]+}/versions"+` matches `+"/projects/spring/versions"+` but not `+"/projects/spring1/versions"+` + +| `+{*path}+` +| Matches zero or more path segments and captures it as a variable named "path" +| `+"/resources/{*file}"+` matches `+"/resources/images/file.png"+` and captures `+file=/images/file.png+` + +`+"{*path}/resources"+` matches `+"/spring/framework/resources"+` and captures `+path=/spring/framework+` + +`+"/resources/{*path}/file.png"+` is invalid as `{*path}` is not allowed in the middle of the path. + +`+"/{*path}/{name}/resources"+` is invalid as only a literal pattern is allowed right after `{*path}`. +`+"/{*path}/project/{project}/resources"+` is allowed. + +`+"/{*path}/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern. + +|=== Captured URI variables can be accessed with `@PathVariable`. For example: diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureSegmentsPathElement.java similarity index 59% rename from spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java rename to spring-web/src/main/java/org/springframework/web/util/pattern/CaptureSegmentsPathElement.java index 11cefc0c249..7133cebd8e7 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureSegmentsPathElement.java @@ -25,24 +25,31 @@ import org.springframework.util.MultiValueMap; import org.springframework.web.util.pattern.PathPattern.MatchingContext; /** - * A path element representing capturing the rest of a path. In the pattern - * '/foo/{*foobar}' the /{*foobar} is represented as a {@link CaptureTheRestPathElement}. + * A path element that captures multiple path segments. + * This element is only allowed in two situations: + *
    + *
  1. At the start of a path, immediately followed by a {@link LiteralPathElement} like '/{*foobar}/foo/{bar}' + *
  2. At the end of a path, like '/foo/{*foobar}' + *
+ *

Only a single {@link WildcardSegmentsPathElement} or {@link CaptureSegmentsPathElement} element is allowed + * * in a pattern. In the pattern '/foo/{*foobar}' the /{*foobar} is represented as a {@link CaptureSegmentsPathElement}. * * @author Andy Clement + * @author Brian Clozel * @since 5.0 */ -class CaptureTheRestPathElement extends PathElement { +class CaptureSegmentsPathElement extends PathElement { private final String variableName; /** - * Create a new {@link CaptureTheRestPathElement} instance. + * Create a new {@link CaptureSegmentsPathElement} instance. * @param pos position of the path element within the path pattern text * @param captureDescriptor a character array containing contents like '{' '*' 'a' 'b' '}' * @param separator the separator used in the path pattern */ - CaptureTheRestPathElement(int pos, char[] captureDescriptor, char separator) { + CaptureSegmentsPathElement(int pos, char[] captureDescriptor, char separator) { super(pos, separator); this.variableName = new String(captureDescriptor, 2, captureDescriptor.length - 3); } @@ -50,41 +57,53 @@ class CaptureTheRestPathElement extends PathElement { @Override public boolean matches(int pathIndex, MatchingContext matchingContext) { - // No need to handle 'match start' checking as this captures everything - // anyway and cannot be followed by anything else - // assert next == null - - // If there is more data, it must start with the separator - if (pathIndex < matchingContext.pathLength && !matchingContext.isSeparator(pathIndex)) { + // wildcard segments at the start of the pattern + if (pathIndex == 0 && this.next != null) { + int endPathIndex = pathIndex; + while (endPathIndex < matchingContext.pathLength) { + if (this.next.matches(endPathIndex, matchingContext)) { + collectParameters(matchingContext, pathIndex, endPathIndex); + return true; + } + endPathIndex++; + } + return false; + } + // match until the end of the path + else if (pathIndex < matchingContext.pathLength && !matchingContext.isSeparator(pathIndex)) { return false; } if (matchingContext.determineRemainingPath) { matchingContext.remainingPathIndex = matchingContext.pathLength; } + collectParameters(matchingContext, pathIndex, matchingContext.pathLength); + return true; + } + + private void collectParameters(MatchingContext matchingContext, int pathIndex, int endPathIndex) { if (matchingContext.extractingVariables) { // Collect the parameters from all the remaining segments - MultiValueMap parametersCollector = null; - for (int i = pathIndex; i < matchingContext.pathLength; i++) { + MultiValueMap parametersCollector = NO_PARAMETERS; + for (int i = pathIndex; i < endPathIndex; i++) { Element element = matchingContext.pathElements.get(i); if (element instanceof PathSegment pathSegment) { MultiValueMap parameters = pathSegment.parameters(); if (!parameters.isEmpty()) { - if (parametersCollector == null) { + if (parametersCollector == NO_PARAMETERS) { parametersCollector = new LinkedMultiValueMap<>(); } parametersCollector.addAll(parameters); } } } - matchingContext.set(this.variableName, pathToString(pathIndex, matchingContext.pathElements), - parametersCollector == null?NO_PARAMETERS:parametersCollector); + matchingContext.set(this.variableName, pathToString(pathIndex, endPathIndex, matchingContext.pathElements), + parametersCollector); } - return true; } - private String pathToString(int fromSegment, List pathElements) { + private String pathToString(int fromSegment, int toSegment, List pathElements) { StringBuilder sb = new StringBuilder(); - for (int i = fromSegment, max = pathElements.size(); i < max; i++) { + for (int i = fromSegment, max = toSegment; i < max; i++) { Element element = pathElements.get(i); if (element instanceof PathSegment pathSegment) { sb.append(pathSegment.valueToMatch()); @@ -119,7 +138,7 @@ class CaptureTheRestPathElement extends PathElement { @Override public String toString() { - return "CaptureTheRest(/{*" + this.variableName + "})"; + return "CaptureSegments(/{*" + this.variableName + "})"; } } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java index 0f9579a0289..7011545ed3f 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java @@ -30,6 +30,7 @@ import org.springframework.web.util.pattern.PatternParseException.PatternMessage * {@link PathElement PathElements} in a linked list. Instances are reusable but are not thread-safe. * * @author Andy Clement + * @author Brian Clozel * @since 5.0 */ class InternalPathPatternParser { @@ -52,7 +53,7 @@ class InternalPathPatternParser { private boolean wildcard = false; // Is the construct {*...} being used in a particular path element - private boolean isCaptureTheRestVariable = false; + private boolean isCaptureSegmentsVariable = false; // Has the parser entered a {...} variable capture block in a particular // path element @@ -67,6 +68,9 @@ class InternalPathPatternParser { // Start of the most recent variable capture in a particular path element private int variableCaptureStart; + // Did we parse a WildcardSegments(**) or CaptureSegments({*foo}) PathElement already? + private boolean hasMultipleSegmentsElement = false; + // Variables captures in this path pattern private @Nullable List capturedVariableNames; @@ -108,13 +112,7 @@ class InternalPathPatternParser { if (this.pathElementStart != -1) { pushPathElement(createPathElement()); } - if (peekDoubleWildcard()) { - pushPathElement(new WildcardTheRestPathElement(this.pos, separator)); - this.pos += 2; - } - else { - pushPathElement(new SeparatorPathElement(this.pos, separator)); - } + pushPathElement(new SeparatorPathElement(this.pos, separator)); } else { if (this.pathElementStart == -1) { @@ -142,35 +140,37 @@ class InternalPathPatternParser { PatternMessage.MISSING_OPEN_CAPTURE); } this.insideVariableCapture = false; - if (this.isCaptureTheRestVariable && (this.pos + 1) < this.pathPatternLength) { - throw new PatternParseException(this.pos + 1, this.pathPatternData, - PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - } this.variableCaptureCount++; } else if (ch == ':') { - if (this.insideVariableCapture && !this.isCaptureTheRestVariable) { + if (this.insideVariableCapture && !this.isCaptureSegmentsVariable) { skipCaptureRegex(); this.insideVariableCapture = false; this.variableCaptureCount++; } } + else if (isDoubleWildcard(separator)) { + checkValidMultipleSegmentsElements(this.pos, this.pos + 1); + pushPathElement(new WildcardSegmentsPathElement(this.pos, separator)); + this.hasMultipleSegmentsElement = true; + this.pos++; + } else if (ch == '*') { if (this.insideVariableCapture && this.variableCaptureStart == this.pos - 1) { - this.isCaptureTheRestVariable = true; + this.isCaptureSegmentsVariable = true; } this.wildcard = true; } // Check that the characters used for captured variable names are like java identifiers if (this.insideVariableCapture) { - if ((this.variableCaptureStart + 1 + (this.isCaptureTheRestVariable ? 1 : 0)) == this.pos && + if ((this.variableCaptureStart + 1 + (this.isCaptureSegmentsVariable ? 1 : 0)) == this.pos && !Character.isJavaIdentifierStart(ch)) { throw new PatternParseException(this.pos, this.pathPatternData, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, Character.toString(ch)); } - else if ((this.pos > (this.variableCaptureStart + 1 + (this.isCaptureTheRestVariable ? 1 : 0)) && + else if ((this.pos > (this.variableCaptureStart + 1 + (this.isCaptureSegmentsVariable ? 1 : 0)) && !Character.isJavaIdentifierPart(ch) && ch != '-')) { throw new PatternParseException(this.pos, this.pathPatternData, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR, @@ -183,6 +183,7 @@ class InternalPathPatternParser { if (this.pathElementStart != -1) { pushPathElement(createPathElement()); } + verifyPatternElements(this.headPE); return new PathPattern(pathPattern, this.parser, this.headPE); } @@ -232,23 +233,28 @@ class InternalPathPatternParser { PatternMessage.MISSING_CLOSE_CAPTURE); } - /** - * After processing a separator, a quick peek whether it is followed by - * a double wildcard (and only as the last path element). - */ - private boolean peekDoubleWildcard() { - if ((this.pos + 2) >= this.pathPatternLength) { + private boolean isDoubleWildcard(char separator) { + if ((this.pos + 1) >= this.pathPatternLength) { return false; } - if (this.pathPatternData[this.pos + 1] != '*' || this.pathPatternData[this.pos + 2] != '*') { + if (this.pathPatternData[this.pos] != '*' || this.pathPatternData[this.pos + 1] != '*') { return false; } - char separator = this.parser.getPathOptions().separator(); - if ((this.pos + 3) < this.pathPatternLength && this.pathPatternData[this.pos + 3] == separator) { + if ((this.pos + 2) < this.pathPatternLength) { + return this.pathPatternData[this.pos + 2] == separator; + } + return true; + } + + private void checkValidMultipleSegmentsElements(int startPosition, int endPosition) { + if (this.hasMultipleSegmentsElement) { throw new PatternParseException(this.pos, this.pathPatternData, - PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); + PatternMessage.CANNOT_HAVE_MANY_MULTISEGMENT_PATHELEMENTS); + } + if (startPosition > 1 && endPosition != this.pathPatternLength - 1) { + throw new PatternParseException(this.pos, this.pathPatternData, + PatternMessage.INVALID_LOCATION_FOR_MULTISEGMENT_PATHELEMENT); } - return (this.pos + 3 == this.pathPatternLength); } /** @@ -256,7 +262,8 @@ class InternalPathPatternParser { * @param newPathElement the new path element to add */ private void pushPathElement(PathElement newPathElement) { - if (newPathElement instanceof CaptureTheRestPathElement) { + if (newPathElement instanceof CaptureSegmentsPathElement || + newPathElement instanceof WildcardSegmentsPathElement) { // There must be a separator ahead of this thing // currentPE SHOULD be a SeparatorPathElement if (this.currentPE == null) { @@ -277,7 +284,8 @@ class InternalPathPatternParser { this.currentPE = newPathElement; } else { - throw new IllegalStateException("Expected SeparatorPathElement but was " + this.currentPE); + throw new IllegalStateException("Expected SeparatorPathElement before " + + newPathElement.getClass().getName() +" but was " + this.currentPE); } } else { @@ -318,9 +326,11 @@ class InternalPathPatternParser { if (this.variableCaptureCount > 0) { if (this.variableCaptureCount == 1 && this.pathElementStart == this.variableCaptureStart && this.pathPatternData[this.pos - 1] == '}') { - if (this.isCaptureTheRestVariable) { + if (this.isCaptureSegmentsVariable) { // It is {*....} - newPE = new CaptureTheRestPathElement( + checkValidMultipleSegmentsElements(this.pathElementStart, this.pos -1); + this.hasMultipleSegmentsElement = true; + newPE = new CaptureSegmentsPathElement( this.pathElementStart, getPathElementText(), separator); } else { @@ -339,7 +349,7 @@ class InternalPathPatternParser { } } else { - if (this.isCaptureTheRestVariable) { + if (this.isCaptureSegmentsVariable) { throw new PatternParseException(this.pathElementStart, this.pathPatternData, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); } @@ -403,7 +413,7 @@ class InternalPathPatternParser { this.insideVariableCapture = false; this.variableCaptureCount = 0; this.wildcard = false; - this.isCaptureTheRestVariable = false; + this.isCaptureSegmentsVariable = false; this.variableCaptureStart = -1; } @@ -421,4 +431,22 @@ class InternalPathPatternParser { this.capturedVariableNames.add(variableName); } + private void verifyPatternElements(@Nullable PathElement headPE) { + PathElement currentElement = headPE; + while (currentElement != null) { + if (currentElement instanceof CaptureSegmentsPathElement || + currentElement instanceof WildcardSegmentsPathElement) { + PathElement nextElement = currentElement.next; + while (nextElement instanceof SeparatorPathElement) { + nextElement = nextElement.next; + } + if (nextElement != null && !(nextElement instanceof LiteralPathElement)) { + throw new PatternParseException(nextElement.pos, this.pathPatternData, + PatternMessage.MULTISEGMENT_PATHELEMENT_NOT_FOLLOWED_BY_LITERAL); + } + } + currentElement = currentElement.next; + } + } + } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathElement.java index 2cc9c24ff5a..a154cd59628 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathElement.java @@ -108,7 +108,7 @@ abstract class PathElement { } /** - * Return if the there are no more PathElements in the pattern. + * Return if there are no more PathElements in the pattern. * @return {@code true} if the there are no more elements */ protected final boolean isNoMorePattern() { diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java index e3aa9d28ee1..ddd8ce9fc8a 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java @@ -162,7 +162,7 @@ public class PathPattern implements Comparable { this.capturedVariableCount += elem.getCaptureCount(); this.normalizedLength += elem.getNormalizedLength(); this.score += elem.getScore(); - if (elem instanceof CaptureTheRestPathElement || elem instanceof WildcardTheRestPathElement) { + if (elem instanceof CaptureSegmentsPathElement || elem instanceof WildcardSegmentsPathElement) { this.catchAll = true; } if (elem instanceof SeparatorPathElement && elem.next instanceof WildcardPathElement && elem.next.next == null) { @@ -200,7 +200,7 @@ public class PathPattern implements Comparable { return !hasLength(pathContainer); } else if (!hasLength(pathContainer)) { - if (this.head instanceof WildcardTheRestPathElement || this.head instanceof CaptureTheRestPathElement) { + if (this.head instanceof WildcardSegmentsPathElement || this.head instanceof CaptureSegmentsPathElement) { pathContainer = EMPTY_PATH; // Will allow CaptureTheRest to bind the variable to empty } else { @@ -222,7 +222,7 @@ public class PathPattern implements Comparable { return (hasLength(pathContainer) && !pathContainerIsJustSeparator(pathContainer) ? null : PathMatchInfo.EMPTY); } else if (!hasLength(pathContainer)) { - if (this.head instanceof WildcardTheRestPathElement || this.head instanceof CaptureTheRestPathElement) { + if (this.head instanceof WildcardSegmentsPathElement || this.head instanceof CaptureSegmentsPathElement) { pathContainer = EMPTY_PATH; // Will allow CaptureTheRest to bind the variable to empty } else { diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java index 9cd3583092d..d9dee2780d4 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java @@ -22,6 +22,7 @@ import java.text.MessageFormat; * Exception that is thrown when there is a problem with the pattern being parsed. * * @author Andy Clement + * @author Brian Clozel * @since 5.0 */ @SuppressWarnings("serial") @@ -98,12 +99,14 @@ public class PatternParseException extends IllegalArgumentException { CANNOT_HAVE_ADJACENT_CAPTURES("Adjacent captures are not allowed"), ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR("Char ''{0}'' not allowed at start of captured variable name"), ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR("Char ''{0}'' is not allowed in a captured variable name"), - NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST("No more pattern data allowed after '{*...}' or '**' pattern element"), + CANNOT_HAVE_MANY_MULTISEGMENT_PATHELEMENTS("Multiple '{*...}' or '**' pattern elements are not allowed"), + INVALID_LOCATION_FOR_MULTISEGMENT_PATHELEMENT("'{*...}' or '**' pattern elements should be placed at the start or end of the pattern"), + MULTISEGMENT_PATHELEMENT_NOT_FOLLOWED_BY_LITERAL("'{*...}' or '**' pattern elements should be followed by a literal path element"), BADLY_FORMED_CAPTURE_THE_REST("Expected form when capturing the rest of the path is simply '{*...}'"), MISSING_REGEX_CONSTRAINT("Missing regex constraint on capture"), ILLEGAL_DOUBLE_CAPTURE("Not allowed to capture ''{0}'' twice in the same pattern"), REGEX_PATTERN_SYNTAX_EXCEPTION("Exception occurred in regex pattern compilation"), - CAPTURE_ALL_IS_STANDALONE_CONSTRUCT("'{*...}' can only be preceded by a path separator"); + CAPTURE_ALL_IS_STANDALONE_CONSTRUCT("'{*...}' cannot be mixed with other path elements in the same path segment"); private final String message; diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardTheRestPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardSegmentsPathElement.java similarity index 51% rename from spring-web/src/main/java/org/springframework/web/util/pattern/WildcardTheRestPathElement.java rename to spring-web/src/main/java/org/springframework/web/util/pattern/WildcardSegmentsPathElement.java index 65ae88dee5e..80c3413ed71 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardTheRestPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardSegmentsPathElement.java @@ -17,23 +17,41 @@ package org.springframework.web.util.pattern; /** - * A path element representing wildcarding the rest of a path. In the pattern - * '/foo/**' the /** is represented as a {@link WildcardTheRestPathElement}. + * A path element representing wildcarding multiple segments in a path. + * This element is only allowed in two situations: + *

    + *
  1. At the start of a path, immediately followed by a {@link LiteralPathElement} like '/**/foo/{bar}' + *
  2. At the end of a path, like '/foo/**' + *
+ *

Only a single {@link WildcardSegmentsPathElement} or {@link CaptureSegmentsPathElement} element is allowed + * in a pattern. In the pattern '/foo/**' the '/**' is represented as a {@link WildcardSegmentsPathElement}. * * @author Andy Clement + * @author Brian Clozel * @since 5.0 */ -class WildcardTheRestPathElement extends PathElement { +class WildcardSegmentsPathElement extends PathElement { - WildcardTheRestPathElement(int pos, char separator) { + WildcardSegmentsPathElement(int pos, char separator) { super(pos, separator); } @Override public boolean matches(int pathIndex, PathPattern.MatchingContext matchingContext) { - // If there is more data, it must start with the separator - if (pathIndex < matchingContext.pathLength && !matchingContext.isSeparator(pathIndex)) { + // wildcard segments at the start of the pattern + if (pathIndex == 0 && this.next != null) { + int endPathIndex = pathIndex; + while (endPathIndex < matchingContext.pathLength) { + if (this.next.matches(endPathIndex, matchingContext)) { + return true; + } + endPathIndex++; + } + return false; + } + // match until the end of the path + else if (pathIndex < matchingContext.pathLength && !matchingContext.isSeparator(pathIndex)) { return false; } if (matchingContext.determineRemainingPath) { @@ -60,7 +78,7 @@ class WildcardTheRestPathElement extends PathElement { @Override public String toString() { - return "WildcardTheRest(" + this.separator + "**)"; + return "WildcardSegments(" + this.separator + "**)"; } } diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java index 4abc53cd176..99c3db01549 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.http.server.PathContainer; @@ -36,66 +37,303 @@ import static org.assertj.core.api.Assertions.fail; * * @author Andy Clement * @author Sam Brannen + * @author Brian Clozel */ class PathPatternParserTests { private PathPattern pathPattern; - @Test - void basicPatterns() { - checkStructure("/"); - checkStructure("/foo"); - checkStructure("foo"); - checkStructure("foo/"); - checkStructure("/foo/"); - checkStructure(""); - } + /** + * Verify that the parsed pattern matches + * the text and path elements of the original pattern. + */ + @Nested + class StructureTests { + + @Test + void literalPatterns() { + checkStructure("/"); + checkStructure("/foo"); + checkStructure("foo"); + checkStructure("foo/"); + checkStructure("/foo/"); + checkStructure(""); + } - @Test - void singleCharWildcardPatterns() { - pathPattern = checkStructure("?"); - assertPathElements(pathPattern, SingleCharWildcardedPathElement.class); - checkStructure("/?/"); - checkStructure("/?abc?/"); - } + @Test + void singleCharWildcardPatterns() { + pathPattern = checkStructure("?"); + assertPathElements(pathPattern, SingleCharWildcardedPathElement.class); + checkStructure("/?/"); + checkStructure("/?abc?/"); + } + + @Test + void wildcardSegmentsStartOfPathPatterns() { + pathPattern = checkStructure("/**/foo"); + assertPathElements(pathPattern, WildcardSegmentsPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); + } + + @Test + void wildcardSegmentEndOfPathPatterns() { + pathPattern = checkStructure("/**"); + assertPathElements(pathPattern, WildcardSegmentsPathElement.class); + pathPattern = checkStructure("/foo/**"); + assertPathElements(pathPattern, SeparatorPathElement.class, LiteralPathElement.class, WildcardSegmentsPathElement.class); + + } + + @Test + void regexpSegmentIsNotWildcardSegment() { + // this is not double wildcard, it's / then **acb (an odd, unnecessary use of double *) + pathPattern = checkStructure("/**acb"); + assertPathElements(pathPattern, SeparatorPathElement.class, RegexPathElement.class); + } + + @Test + void partialCapturingPatterns() { + pathPattern = checkStructure("{foo}abc"); + assertPathElements(pathPattern, RegexPathElement.class); + checkStructure("abc{foo}"); + checkStructure("/abc{foo}"); + checkStructure("{foo}def/"); + checkStructure("/abc{foo}def/"); + checkStructure("{foo}abc{bar}"); + checkStructure("{foo}abc{bar}/"); + checkStructure("/{foo}abc{bar}/"); + } + + @Test + void completeCapturingPatterns() { + pathPattern = checkStructure("{foo}"); + assertPathElements(pathPattern, CaptureVariablePathElement.class); + checkStructure("/{foo}"); + checkStructure("/{f}/"); + checkStructure("/{foo}/{bar}/{wibble}"); + checkStructure("/{mobile-number}"); // gh-23101 + } + + @Test + void completeCaptureWithConstraints() { + pathPattern = checkStructure("{foo:...}"); + assertPathElements(pathPattern, CaptureVariablePathElement.class); + pathPattern = checkStructure("{foo:[0-9]*}"); + assertPathElements(pathPattern, CaptureVariablePathElement.class); + } + + @Test + void captureSegmentsStartOfPathPatterns() { + pathPattern = checkStructure("/{*foobar}"); + assertPathElements(pathPattern, CaptureSegmentsPathElement.class); + pathPattern = checkStructure("/{*foobar}/foo"); + assertPathElements(pathPattern, CaptureSegmentsPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); + } + + @Test + void captureSegmentsEndOfPathPatterns() { + pathPattern = parse("{*foobar}"); + assertThat(pathPattern.computePatternString()).isEqualTo("/{*foobar}"); + assertPathElements(pathPattern, CaptureSegmentsPathElement.class); + pathPattern = checkStructure("/{*foobar}"); + assertPathElements(pathPattern, CaptureSegmentsPathElement.class); + pathPattern = checkStructure("/foo/{*foobar}"); + assertPathElements(pathPattern, SeparatorPathElement.class, LiteralPathElement.class, CaptureSegmentsPathElement.class); + } + + @Test + void multipleSeparatorPatterns() { + pathPattern = checkStructure("///aaa"); + assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, LiteralPathElement.class); + pathPattern = checkStructure("///aaa////aaa/b"); + assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, LiteralPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, SeparatorPathElement.class, SeparatorPathElement.class, + LiteralPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); + pathPattern = checkStructure("/////**"); + assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, + SeparatorPathElement.class, SeparatorPathElement.class, WildcardSegmentsPathElement.class); + } + + @Test + void regexPathElementPatterns() { + pathPattern = checkStructure("/{var:\\\\}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:\\/}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:a{1,2}}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:[^\\/]*}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:\\[*}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:[\\{]*}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("/{var:[\\}]*}"); + assertPathElements(pathPattern, SeparatorPathElement.class, CaptureVariablePathElement.class); + + pathPattern = checkStructure("*"); + assertPathElements(pathPattern, WildcardPathElement.class); + checkStructure("/*"); + checkStructure("/*/"); + checkStructure("*/"); + checkStructure("/*/"); + pathPattern = checkStructure("/*a*/"); + assertPathElements(pathPattern, SeparatorPathElement.class, RegexPathElement.class, SeparatorPathElement.class); + pathPattern = checkStructure("*/"); + assertPathElements(pathPattern, WildcardPathElement.class, SeparatorPathElement.class); + + pathPattern = checkStructure("{symbolicName:[\\p{L}\\.]+}-sources-{version:[\\p{N}\\.]+}.jar"); + assertPathElements(pathPattern, RegexPathElement.class); + } + + private PathPattern checkStructure(String pattern) { + PathPatternParser patternParser = new PathPatternParser(); + PathPattern pp = patternParser.parse(pattern); + assertThat(pp.computePatternString()).isEqualTo(pattern); + return pp; + } + + @SafeVarargs + final void assertPathElements(PathPattern p, Class... sectionClasses) { + PathElement head = p.getHeadSection(); + for (Class sectionClass : sectionClasses) { + if (head == null) { + fail("Ran out of data in parsed pattern. Pattern is: " + p.toChainString()); + } + assertThat(head.getClass().getSimpleName()).as("Not expected section type. Pattern is: " + p.toChainString()).isEqualTo(sectionClass.getSimpleName()); + head = head.next; + } + } - @Test - void multiwildcardPattern() { - pathPattern = checkStructure("/**"); - assertPathElements(pathPattern, WildcardTheRestPathElement.class); - // this is not double wildcard, it's / then **acb (an odd, unnecessary use of double *) - pathPattern = checkStructure("/**acb"); - assertPathElements(pathPattern, SeparatorPathElement.class, RegexPathElement.class); } - @Test - void toStringTests() { - assertThat(checkStructure("/{*foobar}").toChainString()).isEqualTo("CaptureTheRest(/{*foobar})"); - assertThat(checkStructure("{foobar}").toChainString()).isEqualTo("CaptureVariable({foobar})"); - assertThat(checkStructure("abc").toChainString()).isEqualTo("Literal(abc)"); - assertThat(checkStructure("{a}_*_{b}").toChainString()).isEqualTo("Regex({a}_*_{b})"); - assertThat(checkStructure("/").toChainString()).isEqualTo("Separator(/)"); - assertThat(checkStructure("?a?b?c").toChainString()).isEqualTo("SingleCharWildcarded(?a?b?c)"); - assertThat(checkStructure("*").toChainString()).isEqualTo("Wildcard(*)"); - assertThat(checkStructure("/**").toChainString()).isEqualTo("WildcardTheRest(/**)"); + @Nested + class ParsingErrorTests { + + @Test + void captureSegmentsIllegalSyntax() { + checkError("/{*foobar}abc", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); + checkError("/{*f%obar}", 4, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); + checkError("/{*foobar}abc", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); + checkError("/{f*oobar}", 3, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); + checkError("/{*foobar:.*}/abc", 9, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); + checkError("/{abc}{*foobar}", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); + checkError("/{abc}{*foobar}{foo}", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); + checkError("/{*foo}/foo/{*bar}", 18, PatternMessage.CANNOT_HAVE_MANY_MULTISEGMENT_PATHELEMENTS); + checkError("/{*foo}/{bar}", 8, PatternMessage.MULTISEGMENT_PATHELEMENT_NOT_FOLLOWED_BY_LITERAL); + checkError("{foo:}", 5, PatternMessage.MISSING_REGEX_CONSTRAINT); + checkError("{foo}_{foo}", 0, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "foo"); + checkError("/{bar}/{bar}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar"); + checkError("/{bar}/{bar}_{foo}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar"); + } + + @Test + void regexpSegmentsIllegalSyntax() { + checkError("/{var:[^/]*}", 8, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("/{var:abc", 8, PatternMessage.MISSING_CLOSE_CAPTURE); + // Do not check the expected position due a change in RegEx parsing in JDK 13. + // See https://github.com/spring-projects/spring-framework/issues/23669 + checkError("/{var:a{{1,2}}}", PatternMessage.REGEX_PATTERN_SYNTAX_EXCEPTION); + } + + @Test + void illegalCapturePatterns() { + checkError("{abc/", 4, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("{abc:}/", 5, PatternMessage.MISSING_REGEX_CONSTRAINT); + checkError("{", 1, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("{abc", 4, PatternMessage.MISSING_CLOSE_CAPTURE); + checkError("{/}", 1, 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("/{/}", 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); + checkError("/foobar/{abc}_{abc}", 8, PatternMessage.ILLEGAL_DOUBLE_CAPTURE); + checkError("/foobar/{abc:..}_{abc:..}", 8, PatternMessage.ILLEGAL_DOUBLE_CAPTURE); + } + + @Test + void captureGroupInRegexpNotAllowed() { + PathPattern pp = parse("/{abc:foo(bar)}"); + assertThatIllegalArgumentException().isThrownBy(() -> + pp.matchAndExtract(PathContainer.parsePath("/foo"))) + .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); + assertThatIllegalArgumentException().isThrownBy(() -> + pp.matchAndExtract(PathContainer.parsePath("/foobar"))) + .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); + } + + @Test + void badPatterns() { + //checkError("/{foo}{bar}/",6,PatternMessage.CANNOT_HAVE_ADJACENT_CAPTURES); + checkError("/{?}/", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "?"); + checkError("/{a?b}/", 3, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR, "?"); + checkError("/{%%$}", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "%"); + checkError("/{ }", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, " "); + checkError("/{%:[0-9]*}", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "%"); + } + + @Test + void captureTheRestWithinPatternNotSupported() { + PathPatternParser parser = new PathPatternParser(); + assertThatThrownBy(() -> parser.parse("/resources/**/details")) + .isInstanceOf(PatternParseException.class) + .extracting("messageType").isEqualTo(PatternMessage.INVALID_LOCATION_FOR_MULTISEGMENT_PATHELEMENT); + } + + /** + * Delegates to {@link #checkError(String, int, PatternMessage, String...)}, + * passing {@code -1} as the {@code expectedPos}. + * @since 5.2 + */ + private void checkError(String pattern, PatternMessage expectedMessage, String... expectedInserts) { + checkError(pattern, -1, expectedMessage, expectedInserts); + } + + /** + * @param expectedPos the expected position, or {@code -1} if the position should not be checked + */ + private void checkError(String pattern, int expectedPos, PatternMessage expectedMessage, + String... expectedInserts) { + + assertThatExceptionOfType(PatternParseException.class) + .isThrownBy(() -> pathPattern = parse(pattern)) + .satisfies(ex -> { + if (expectedPos >= 0) { + assertThat(ex.getPosition()).as(ex.toDetailedString()).isEqualTo(expectedPos); + } + assertThat(ex.getMessageType()).as(ex.toDetailedString()).isEqualTo(expectedMessage); + if (expectedInserts.length != 0) { + assertThat(ex.getInserts()).isEqualTo(expectedInserts); + } + }); + } + } + @Test - void captureTheRestPatterns() { - pathPattern = parse("{*foobar}"); - assertThat(pathPattern.computePatternString()).isEqualTo("/{*foobar}"); - assertPathElements(pathPattern, CaptureTheRestPathElement.class); - pathPattern = checkStructure("/{*foobar}"); - assertPathElements(pathPattern, CaptureTheRestPathElement.class); - checkError("/{*foobar}/", 10, PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - checkError("/{*foobar}abc", 10, PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - checkError("/{*f%obar}", 4, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); - checkError("/{*foobar}abc", 10, PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - checkError("/{f*oobar}", 3, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); - checkError("/{*foobar}/abc", 10, PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - checkError("/{*foobar:.*}/abc", 9, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR); - checkError("/{abc}{*foobar}", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); - checkError("/{abc}{*foobar}{foo}", 15, PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); + void toStringTests() { + assertThat(parse("/{*foobar}").toChainString()).isEqualTo("CaptureSegments(/{*foobar})"); + assertThat(parse("{foobar}").toChainString()).isEqualTo("CaptureVariable({foobar})"); + assertThat(parse("abc").toChainString()).isEqualTo("Literal(abc)"); + assertThat(parse("{a}_*_{b}").toChainString()).isEqualTo("Regex({a}_*_{b})"); + assertThat(parse("/").toChainString()).isEqualTo("Separator(/)"); + assertThat(parse("?a?b?c").toChainString()).isEqualTo("SingleCharWildcarded(?a?b?c)"); + assertThat(parse("*").toChainString()).isEqualTo("Wildcard(*)"); + assertThat(parse("/**").toChainString()).isEqualTo("WildcardSegments(/**)"); } @Test @@ -116,82 +354,6 @@ class PathPatternParserTests { assertThat(pp2.hashCode()).isNotEqualTo(pp1.hashCode()); } - @Test - void regexPathElementPatterns() { - checkError("/{var:[^/]*}", 8, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("/{var:abc", 8, PatternMessage.MISSING_CLOSE_CAPTURE); - - // Do not check the expected position due a change in RegEx parsing in JDK 13. - // See https://github.com/spring-projects/spring-framework/issues/23669 - checkError("/{var:a{{1,2}}}", PatternMessage.REGEX_PATTERN_SYNTAX_EXCEPTION); - - pathPattern = checkStructure("/{var:\\\\}"); - PathElement next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - assertMatches(pathPattern, "/\\"); - - pathPattern = checkStructure("/{var:\\/}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - assertNoMatch(pathPattern, "/aaa"); - - pathPattern = checkStructure("/{var:a{1,2}}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - - pathPattern = checkStructure("/{var:[^\\/]*}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - PathPattern.PathMatchInfo result = matchAndExtract(pathPattern, "/foo"); - assertThat(result.getUriVariables().get("var")).isEqualTo("foo"); - - pathPattern = checkStructure("/{var:\\[*}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern, "/[[["); - assertThat(result.getUriVariables().get("var")).isEqualTo("[[["); - - pathPattern = checkStructure("/{var:[\\{]*}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern, "/{{{"); - assertThat(result.getUriVariables().get("var")).isEqualTo("{{{"); - - pathPattern = checkStructure("/{var:[\\}]*}"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern, "/}}}"); - assertThat(result.getUriVariables().get("var")).isEqualTo("}}}"); - - pathPattern = checkStructure("*"); - assertThat(pathPattern.getHeadSection().getClass().getName()).isEqualTo(WildcardPathElement.class.getName()); - checkStructure("/*"); - checkStructure("/*/"); - checkStructure("*/"); - checkStructure("/*/"); - pathPattern = checkStructure("/*a*/"); - next = pathPattern.getHeadSection().next; - assertThat(next.getClass().getName()).isEqualTo(RegexPathElement.class.getName()); - pathPattern = checkStructure("*/"); - assertThat(pathPattern.getHeadSection().getClass().getName()).isEqualTo(WildcardPathElement.class.getName()); - checkError("{foo}_{foo}", 0, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "foo"); - checkError("/{bar}/{bar}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar"); - checkError("/{bar}/{bar}_{foo}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar"); - - pathPattern = checkStructure("{symbolicName:[\\p{L}\\.]+}-sources-{version:[\\p{N}\\.]+}.jar"); - assertThat(pathPattern.getHeadSection().getClass().getName()).isEqualTo(RegexPathElement.class.getName()); - } - - @Test - void completeCapturingPatterns() { - pathPattern = checkStructure("{foo}"); - assertThat(pathPattern.getHeadSection().getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - checkStructure("/{foo}"); - checkStructure("/{f}/"); - checkStructure("/{foo}/{bar}/{wibble}"); - checkStructure("/{mobile-number}"); // gh-23101 - } - @Test void noEncoding() { // Check no encoding of expressions or constraints @@ -205,66 +367,6 @@ class PathPatternParserTests { assertThat(pp.toChainString()).isEqualTo("Regex({foo:f o}_ _{bar:b\\|o})"); } - @Test - void completeCaptureWithConstraints() { - pathPattern = checkStructure("{foo:...}"); - assertPathElements(pathPattern, CaptureVariablePathElement.class); - pathPattern = checkStructure("{foo:[0-9]*}"); - assertPathElements(pathPattern, CaptureVariablePathElement.class); - checkError("{foo:}", 5, PatternMessage.MISSING_REGEX_CONSTRAINT); - } - - @Test - void partialCapturingPatterns() { - pathPattern = checkStructure("{foo}abc"); - assertThat(pathPattern.getHeadSection().getClass().getName()).isEqualTo(RegexPathElement.class.getName()); - checkStructure("abc{foo}"); - checkStructure("/abc{foo}"); - checkStructure("{foo}def/"); - checkStructure("/abc{foo}def/"); - checkStructure("{foo}abc{bar}"); - checkStructure("{foo}abc{bar}/"); - checkStructure("/{foo}abc{bar}/"); - } - - @Test - void illegalCapturePatterns() { - checkError("{abc/", 4, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("{abc:}/", 5, PatternMessage.MISSING_REGEX_CONSTRAINT); - checkError("{", 1, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("{abc", 4, PatternMessage.MISSING_CLOSE_CAPTURE); - checkError("{/}", 1, 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("/{/}", 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); - checkError("/foobar/{abc}_{abc}", 8, PatternMessage.ILLEGAL_DOUBLE_CAPTURE); - checkError("/foobar/{abc:..}_{abc:..}", 8, PatternMessage.ILLEGAL_DOUBLE_CAPTURE); - PathPattern pp = parse("/{abc:foo(bar)}"); - assertThatIllegalArgumentException().isThrownBy(() -> - pp.matchAndExtract(toPSC("/foo"))) - .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); - assertThatIllegalArgumentException().isThrownBy(() -> - pp.matchAndExtract(toPSC("/foobar"))) - .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); - } - - @Test - void badPatterns() { -// checkError("/{foo}{bar}/",6,PatternMessage.CANNOT_HAVE_ADJACENT_CAPTURES); - checkError("/{?}/", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "?"); - checkError("/{a?b}/", 3, PatternMessage.ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR, "?"); - checkError("/{%%$}", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "%"); - checkError("/{ }", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, " "); - checkError("/{%:[0-9]*}", 2, PatternMessage.ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR, "%"); - } - @Test void patternPropertyGetCaptureCountTests() { // Test all basic section types @@ -311,30 +413,23 @@ class PathPatternParserTests { } @Test - void multipleSeparatorPatterns() { - pathPattern = checkStructure("///aaa"); + void normalizedLengthWhenMultipleSeparator() { + pathPattern = parse("///aaa"); assertThat(pathPattern.getNormalizedLength()).isEqualTo(6); - assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, - SeparatorPathElement.class, LiteralPathElement.class); - pathPattern = checkStructure("///aaa////aaa/b"); + pathPattern = parse("///aaa////aaa/b"); assertThat(pathPattern.getNormalizedLength()).isEqualTo(15); - assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, - SeparatorPathElement.class, LiteralPathElement.class, SeparatorPathElement.class, - SeparatorPathElement.class, SeparatorPathElement.class, SeparatorPathElement.class, - LiteralPathElement.class, SeparatorPathElement.class, LiteralPathElement.class); - pathPattern = checkStructure("/////**"); + pathPattern = parse("/////**"); assertThat(pathPattern.getNormalizedLength()).isEqualTo(5); - assertPathElements(pathPattern, SeparatorPathElement.class, SeparatorPathElement.class, - SeparatorPathElement.class, SeparatorPathElement.class, WildcardTheRestPathElement.class); } @Test - void patternPropertyGetLengthTests() { + void normalizedLengthWhenVariable() { // Test all basic section types assertThat(parse("{foo}").getNormalizedLength()).isEqualTo(1); assertThat(parse("foo").getNormalizedLength()).isEqualTo(3); assertThat(parse("{*foobar}").getNormalizedLength()).isEqualTo(1); assertThat(parse("/{*foobar}").getNormalizedLength()).isEqualTo(1); + assertThat(parse("**").getNormalizedLength()).isEqualTo(1); assertThat(parse("/**").getNormalizedLength()).isEqualTo(1); assertThat(parse("{abc}asdf").getNormalizedLength()).isEqualTo(5); assertThat(parse("{abc}_*").getNormalizedLength()).isEqualTo(3); @@ -350,6 +445,15 @@ class PathPatternParserTests { assertThat(parse("/{foo}/{bar}_{goo}_{wibble}/abc/bar").getNormalizedLength()).isEqualTo(16); } + @Test + void separatorTests() { + PathPatternParser parser = new PathPatternParser(); + parser.setPathOptions(PathContainer.Options.create('.', false)); + String rawPattern = "first.second.{last}"; + PathPattern pattern = parser.parse(rawPattern); + assertThat(pattern.computePatternString()).isEqualTo(rawPattern); + } + @Test void compareTests() { PathPattern p1, p2, p3; @@ -414,96 +518,14 @@ class PathPatternParserTests { assertThat(patterns).element(1).isEqualTo(p2); } - @Test - void captureTheRestWithinPatternNotSupported() { - PathPatternParser parser = new PathPatternParser(); - assertThatThrownBy(() -> parser.parse("/resources/**/details")) - .isInstanceOf(PatternParseException.class) - .extracting("messageType").isEqualTo(PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); - } - - @Test - void separatorTests() { - PathPatternParser parser = new PathPatternParser(); - parser.setPathOptions(PathContainer.Options.create('.', false)); - String rawPattern = "first.second.{last}"; - PathPattern pattern = parser.parse(rawPattern); - assertThat(pattern.computePatternString()).isEqualTo(rawPattern); - } - private PathPattern parse(String pattern) { PathPatternParser patternParser = new PathPatternParser(); return patternParser.parse(pattern); } - /** - * Verify the pattern string computed for a parsed pattern matches the original pattern text - */ - private PathPattern checkStructure(String pattern) { - PathPattern pp = parse(pattern); - assertThat(pp.computePatternString()).isEqualTo(pattern); - return pp; - } - - /** - * Delegates to {@link #checkError(String, int, PatternMessage, String...)}, - * passing {@code -1} as the {@code expectedPos}. - * @since 5.2 - */ - private void checkError(String pattern, PatternMessage expectedMessage, String... expectedInserts) { - checkError(pattern, -1, expectedMessage, expectedInserts); - } - - /** - * @param expectedPos the expected position, or {@code -1} if the position should not be checked - */ - private void checkError(String pattern, int expectedPos, PatternMessage expectedMessage, - String... expectedInserts) { - - assertThatExceptionOfType(PatternParseException.class) - .isThrownBy(() -> pathPattern = parse(pattern)) - .satisfies(ex -> { - if (expectedPos >= 0) { - assertThat(ex.getPosition()).as(ex.toDetailedString()).isEqualTo(expectedPos); - } - assertThat(ex.getMessageType()).as(ex.toDetailedString()).isEqualTo(expectedMessage); - if (expectedInserts.length != 0) { - assertThat(ex.getInserts()).isEqualTo(expectedInserts); - } - }); - } - - @SafeVarargs - private void assertPathElements(PathPattern p, Class... sectionClasses) { - PathElement head = p.getHeadSection(); - for (Class sectionClass : sectionClasses) { - if (head == null) { - fail("Ran out of data in parsed pattern. Pattern is: " + p.toChainString()); - } - assertThat(head.getClass().getSimpleName()).as("Not expected section type. Pattern is: " + p.toChainString()).isEqualTo(sectionClass.getSimpleName()); - head = head.next; - } - } - // Mirrors the score computation logic in PathPattern private int computeScore(int capturedVariableCount, int wildcardCount) { return capturedVariableCount + wildcardCount * 100; } - private void assertMatches(PathPattern pp, String path) { - assertThat(pp.matches(PathPatternTests.toPathContainer(path))).isTrue(); - } - - private void assertNoMatch(PathPattern pp, String path) { - assertThat(pp.matches(PathPatternTests.toPathContainer(path))).isFalse(); - } - - private PathPattern.PathMatchInfo matchAndExtract(PathPattern pp, String path) { - return pp.matchAndExtract(PathPatternTests.toPathContainer(path)); - } - - private PathContainer toPSC(String path) { - return PathPatternTests.toPathContainer(path); - } - } diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index aca89c26878..9238fcd77d1 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.http.server.PathContainer; @@ -34,12 +35,303 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** - * Exercise matching of {@link PathPattern} objects. + * Tests for {@link PathPattern}. * * @author Andy Clement + * @author Brian Clozel */ class PathPatternTests { + @Nested + class MatchingTests { + + @Test + void basicMatching() { + checkMatches("", ""); + checkMatches("", null); + checkNoMatch("/abc", "/"); + checkMatches("/", "/"); + checkNoMatch("/", "/a"); + checkMatches("foo/bar/", "foo/bar/"); + checkNoMatch("foo", "foobar"); + checkMatches("/foo/bar", "/foo/bar"); + checkNoMatch("/foo/bar", "/foo/baz"); + } + + @Test + void literalPathElements() { + checkMatches("foo", "foo"); + checkNoMatch("foo", "bar"); + checkNoMatch("foo", "/foo"); + checkNoMatch("/foo", "foo"); + checkMatches("/f", "/f"); + checkMatches("/foo", "/foo"); + checkNoMatch("/foo", "/food"); + checkNoMatch("/food", "/foo"); + checkMatches("/foo/", "/foo/"); + checkMatches("/foo/bar/woo", "/foo/bar/woo"); + checkMatches("foo/bar/woo", "foo/bar/woo"); + } + + @Test + void questionMarks() { + checkNoMatch("a", "ab"); + checkMatches("/f?o/bar", "/foo/bar"); + checkNoMatch("/foo/b2r", "/foo/bar"); + checkNoMatch("?", "te"); + checkMatches("?", "a"); + checkMatches("???", "abc"); + checkNoMatch("tes?", "te"); + checkNoMatch("tes?", "tes"); + checkNoMatch("tes?", "testt"); + checkNoMatch("tes?", "tsst"); + checkMatches(".?.a", ".a.a"); + checkNoMatch(".?.a", ".aba"); + checkMatches("/f?o/bar","/f%20o/bar"); + } + + @Test + void multipleSeparatorsInPattern() { + PathPattern pp = parse("a//b//c"); + assertThat(pp.toChainString()).isEqualTo("Literal(a) Separator(/) Separator(/) Literal(b) Separator(/) Separator(/) Literal(c)"); + assertMatches(pp,"a//b//c"); + assertThat(parse("a//**").toChainString()).isEqualTo("Literal(a) Separator(/) WildcardSegments(/**)"); + 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 + void multipleSelectorsInPath() { + checkNoMatch("/abc", "////abc"); + checkNoMatch("/", "//"); + checkNoMatch("/abc/def/ghi", "/abc//def///ghi"); + checkNoMatch("/abc", "////abc"); + checkMatches("////abc", "////abc"); + checkNoMatch("/", "//"); + checkNoMatch("/abc//def", "/abc/def"); + checkNoMatch("/abc//def///ghi", "/abc/def/ghi"); + checkMatches("/abc//def///ghi", "/abc//def///ghi"); + } + + @Test + 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 + void captureSegmentsAtStart() { + checkMatches("/{*foobar}/resource", "/resource"); + checkNoMatch("/{*foobar}/resource", "/resourceX"); + checkNoMatch("/{*foobar}/resource", "/foobar/resourceX"); + checkMatches("/{*foobar}/resource", "/foobar/resource"); + } + + @Test + void captureSegmentsAtEnd() { + checkMatches("/resource/{*foobar}", "/resource"); + checkNoMatch("/resource/{*foobar}", "/resourceX"); + checkNoMatch("/resource/{*foobar}", "/resourceX/foobar"); + checkMatches("/resource/{*foobar}", "/resource/foobar"); + } + + @Test + 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"); + checkMatches("/*/bar", "/foo/bar"); + checkMatches("/a*b*c*d/bar", "/abcd/bar"); + checkMatches("*a*", "testa"); + checkMatches("a/*", "a/"); + checkNoMatch("a/*", "a//"); // no data for * + PathPatternParser ppp = new PathPatternParser(); + assertThat(ppp.parse("a/*").matches(toPathContainer("a//"))).isFalse(); + checkMatches("a/*", "a/a"); + } + + @Test + void wildcardSegmentsStart() { + checkMatches("/**/resource", "/resource"); + checkNoMatch("/**/resource", "/Xresource"); + checkNoMatch("/**/resource", "/foobar/resourceX"); + checkMatches("/**/resource", "/foobar/resource"); + + checkMatches("/**/resource/test", "/foo/bar/resource/test"); + checkNoMatch("/**/resource/test", "/foo/bar/resource/t"); + } + + @Test + void wildcardSegmentsEnd() { + checkMatches("/resource/**", "/resource"); + checkNoMatch("/resource/**", "/resourceX"); + checkNoMatch("/resource/**", "/resourceX/foobar"); + checkMatches("/resource/**", "/resource/foobar"); + } + + @Test + void antPathMatcherTests() { + // test exact matching + checkMatches("test", "test"); + checkMatches("/test", "/test"); + checkMatches("https://example.org", "https://example.org"); + checkNoMatch("/test.jpg", "test.jpg"); + checkNoMatch("test", "/test"); + checkNoMatch("/test", "test"); + + // test matching with ?'s + checkMatches("t?st", "test"); + checkMatches("??st", "test"); + checkMatches("tes?", "test"); + checkMatches("te??", "test"); + checkMatches("?es?", "test"); + checkNoMatch("tes?", "tes"); + checkNoMatch("tes?", "testt"); + checkNoMatch("tes?", "tsst"); + + // test matching with *'s + checkMatches("*", "test"); + checkMatches("test*", "test"); + checkMatches("test*", "testTest"); + checkMatches("test/*", "test/Test"); + checkMatches("test/*", "test/t"); + checkMatches("test/*", "test/"); + checkMatches("*test*", "AnothertestTest"); + checkMatches("*test", "Anothertest"); + checkMatches("*.*", "test."); + checkMatches("*.*", "test.test"); + checkMatches("*.*", "test.test.test"); + checkMatches("test*aaa", "testblaaaa"); + checkNoMatch("test*", "tst"); + checkNoMatch("test*", "tsttest"); + checkMatches("test*", "test"); // trailing slash is optional + checkNoMatch("test*", "test/t"); + checkNoMatch("test/*", "test"); + checkNoMatch("*test*", "tsttst"); + checkNoMatch("*test", "tsttst"); + checkNoMatch("*.*", "tsttst"); + checkNoMatch("test*aaa", "test"); + checkNoMatch("test*aaa", "testblaaab"); + + // test matching with ?'s and /'s + checkMatches("/?", "/a"); + checkMatches("/?/a", "/a/a"); + checkMatches("/a/?", "/a/b"); + checkMatches("/??/a", "/aa/a"); + checkMatches("/a/??", "/a/bb"); + checkMatches("/?", "/a"); + + checkMatches("/**", ""); + checkMatches("/books/**", "/books"); + checkMatches("/**", "/testing/testing"); + checkMatches("/*/**", "/testing/testing"); + checkMatches("/bla*bla/test", "/blaXXXbla/test"); + checkMatches("/*bla/test", "/XXXbla/test"); + checkNoMatch("/bla*bla/test", "/blaXXXbl/test"); + checkNoMatch("/*bla/test", "XXXblab/test"); + checkNoMatch("/*bla/test", "XXXbl/test"); + checkNoMatch("/????", "/bala/bla"); + checkMatches("/foo/bar/**", "/foo/bar/"); + checkMatches("/{bla}.html", "/testing.html"); + checkCapture("/{bla}.*", "/testing.html", "bla", "testing"); + } + + } + + @Nested + class VariableCaptureTests { + + @Test + void constrainedMatches() { + checkCapture("{foo:[0-9]*}", "123", "foo", "123"); + checkNoMatch("{foo:[0-9]*}", "abc"); + checkNoMatch("/{foo:[0-9]*}", "abc"); + checkCapture("/*/{foo:....}/**", "/foo/barg/foo", "foo", "barg"); + checkCapture("/*/{foo:....}/**", "/foo/barg/abc/def/ghi", "foo", "barg"); + checkNoMatch("{foo:....}", "99"); + checkMatches("{foo:..}", "99"); + checkCapture("/{abc:\\{\\}}", "/{}", "abc", "{}"); + checkCapture("/{abc:\\[\\]}", "/[]", "abc", "[]"); + checkCapture("/{abc:\\\\\\\\}", "/\\\\"); // this is fun... + } + + @Test + void captureSegmentsAtStart() { + checkCapture("/{*foobar}/resource", "/foobar/resource", "foobar", "/foobar"); + checkCapture("/{*something}/customer", "/99/customer", "something", "/99"); + checkCapture("/{*something}/customer", "/aa/bb/cc/customer", "something", "/aa/bb/cc"); + checkCapture("/{*something}/customer", "/customer", "something", ""); + checkCapture("/{*something}/customer", "//////99/customer", "something", "//////99"); + } + + @Test + void captureSegmentsAtEnd() { + checkCapture("/resource/{*foobar}", "/resource/foobar", "foobar", "/foobar"); + checkCapture("/customer/{*something}", "/customer/99", "something", "/99"); + 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//////", "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 + void encodingAndBoundVariablesCapturePathElement() { + checkCapture("{var}","f%20o","var","f o"); + checkCapture("{var1}/{var2}","f%20o/f%7Co","var1","f o","var2","f|o"); + checkCapture("{var1}/{var2}","f%20o/f%7co","var1","f o","var2","f|o"); // lower case encoding + checkCapture("{var:foo}","foo","var","foo"); + checkCapture("{var:f o}","f%20o","var","f o"); // constraint is expressed in non encoded form + checkCapture("{var:f.o}","f%20o","var","f o"); + checkCapture("{var:f\\|o}","f%7co","var","f|o"); + checkCapture("{var:.*}","x\ny","var","x\ny"); + } + + @Test + void encodingAndBoundVariablesCaptureTheRestPathElement() { + checkCapture("/{*var}","/f%20o","var","/f o"); + checkCapture("{var1}/{*var2}","f%20o/f%7Co","var1","f o","var2","/f|o"); + checkCapture("/{*var}","/foo","var","/foo"); + checkCapture("/{*var}","/f%20o","var","/f o"); + checkCapture("/{*var}","/f%20o","var","/f o"); + checkCapture("/{*var}","/f%7co","var","/f|o"); + } + + @Test + void encodingAndBoundVariablesRegexPathElement() { + checkCapture("/{var1:f o}_ _{var2}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); + checkCapture("/{var1}_{var2}","/f%20o_foo","var1","f o","var2","foo"); + checkCapture("/{var1}_ _{var2}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); + checkCapture("/{var1}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); + checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); + checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); + checkCapture("/{var1}_{var2}","/f\noo_foo","var1","f\noo","var2","foo"); + } + + } + + @Test void pathContainer() { assertThat(elementsToString(toPathContainer("/abc/def").elements())).isEqualTo("[/][abc][/][def]"); @@ -62,44 +354,8 @@ class PathPatternTests { assertThat(parser.parse("/foo/bar").hasPatternSyntax()).isFalse(); } - @Test - void matching_LiteralPathElement() { - checkMatches("foo", "foo"); - checkNoMatch("foo", "bar"); - checkNoMatch("foo", "/foo"); - checkNoMatch("/foo", "foo"); - checkMatches("/f", "/f"); - checkMatches("/foo", "/foo"); - checkNoMatch("/foo", "/food"); - checkNoMatch("/food", "/foo"); - checkMatches("/foo/", "/foo/"); - checkMatches("/foo/bar/woo", "/foo/bar/woo"); - checkMatches("foo/bar/woo", "foo/bar/woo"); - } - - @Test - void basicMatching() { - checkMatches("", ""); - checkMatches("", null); - checkNoMatch("/abc", "/"); - checkMatches("/", "/"); - checkNoMatch("/", "/a"); - checkMatches("foo/bar/", "foo/bar/"); - checkNoMatch("foo", "foobar"); - checkMatches("/foo/bar", "/foo/bar"); - checkNoMatch("/foo/bar", "/foo/baz"); - } - - private void assertMatches(PathPattern pp, String path) { - assertThat(pp.matches(toPathContainer(path))).isTrue(); - } - - private void assertNoMatch(PathPattern pp, String path) { - assertThat(pp.matches(toPathContainer(path))).isFalse(); - } - - @Test - void pathRemainderBasicCases_spr15336() { + @Test // SPR-15336 + void pathRemainderBasicCases() { // Cover all PathElement kinds assertThat(getPathRemaining("/foo", "/foo/bar").getPathRemaining().value()).isEqualTo("/bar"); assertThat(getPathRemaining("/foo", "/foo/").getPathRemaining().value()).isEqualTo("/"); @@ -119,41 +375,8 @@ class PathPatternTests { assertThat(getPathRemaining("/foo//", "/foo///bar").getPathRemaining().value()).isEqualTo("/bar"); } - @Test - void encodingAndBoundVariablesCapturePathElement() { - checkCapture("{var}","f%20o","var","f o"); - checkCapture("{var1}/{var2}","f%20o/f%7Co","var1","f o","var2","f|o"); - checkCapture("{var1}/{var2}","f%20o/f%7co","var1","f o","var2","f|o"); // lower case encoding - checkCapture("{var:foo}","foo","var","foo"); - checkCapture("{var:f o}","f%20o","var","f o"); // constraint is expressed in non encoded form - checkCapture("{var:f.o}","f%20o","var","f o"); - checkCapture("{var:f\\|o}","f%7co","var","f|o"); - checkCapture("{var:.*}","x\ny","var","x\ny"); - } - - @Test - void encodingAndBoundVariablesCaptureTheRestPathElement() { - checkCapture("/{*var}","/f%20o","var","/f o"); - checkCapture("{var1}/{*var2}","f%20o/f%7Co","var1","f o","var2","/f|o"); - checkCapture("/{*var}","/foo","var","/foo"); - checkCapture("/{*var}","/f%20o","var","/f o"); - checkCapture("/{*var}","/f%20o","var","/f o"); - checkCapture("/{*var}","/f%7co","var","/f|o"); - } - - @Test - void encodingAndBoundVariablesRegexPathElement() { - checkCapture("/{var1:f o}_ _{var2}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); - checkCapture("/{var1}_{var2}","/f%20o_foo","var1","f o","var2","foo"); - checkCapture("/{var1}_ _{var2}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); - checkCapture("/{var1}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); - checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); - checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o"); - checkCapture("/{var1}_{var2}","/f\noo_foo","var1","f\noo","var2","foo"); - } - - @Test - void pathRemainingCornerCases_spr15336() { + @Test // SPR-15336 + void pathRemainingCornerCases() { // No match when the literal path element is a longer form of the segment in the pattern assertThat(parse("/foo").matchStartOfPath(toPathContainer("/footastic/bar"))).isNull(); assertThat(parse("/f?o").matchStartOfPath(toPathContainer("/footastic/bar"))).isNull(); @@ -166,6 +389,11 @@ class PathPatternTests { assertThat(parse("/resource/**") .matchStartOfPath(toPathContainer("/resource")).getPathRemaining().value()).isEmpty(); + assertThat(parse("/**/resource") + .matchStartOfPath(toPathContainer("/test/resource")).getPathRemaining().value()).isEmpty(); + assertThat(parse("/**/resource") + .matchStartOfPath(toPathContainer("/test/resource/other")).getPathRemaining().value()).isEqualTo("/other"); + // Similar to above for the capture-the-rest variant assertThat(parse("/resource/{*foo}").matchStartOfPath(toPathContainer("/resourceX"))).isNull(); assertThat(parse("/resource/{*foo}") @@ -194,191 +422,8 @@ class PathPatternTests { assertThat(parse("").matchStartOfPath(toPathContainer("")).getPathRemaining().value()).isEmpty(); } - @Test - void questionMarks() { - checkNoMatch("a", "ab"); - checkMatches("/f?o/bar", "/foo/bar"); - checkNoMatch("/foo/b2r", "/foo/bar"); - checkNoMatch("?", "te"); - checkMatches("?", "a"); - checkMatches("???", "abc"); - checkNoMatch("tes?", "te"); - checkNoMatch("tes?", "tes"); - checkNoMatch("tes?", "testt"); - checkNoMatch("tes?", "tsst"); - checkMatches(".?.a", ".a.a"); - checkNoMatch(".?.a", ".aba"); - checkMatches("/f?o/bar","/f%20o/bar"); - } - - @Test - void captureTheRest() { - checkMatches("/resource/{*foobar}", "/resource"); - checkNoMatch("/resource/{*foobar}", "/resourceX"); - checkNoMatch("/resource/{*foobar}", "/resourceX/foobar"); - checkMatches("/resource/{*foobar}", "/resource/foobar"); - checkCapture("/resource/{*foobar}", "/resource/foobar", "foobar", "/foobar"); - checkCapture("/customer/{*something}", "/customer/99", "something", "/99"); - 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//////", "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 - void multipleSeparatorsInPattern() { - PathPattern pp = parse("a//b//c"); - assertThat(pp.toChainString()).isEqualTo("Literal(a) Separator(/) Separator(/) Literal(b) Separator(/) Separator(/) Literal(c)"); - assertMatches(pp,"a//b//c"); - assertThat(parse("a//**").toChainString()).isEqualTo("Literal(a) Separator(/) WildcardTheRest(/**)"); - 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 - void multipleSelectorsInPath() { - checkNoMatch("/abc", "////abc"); - checkNoMatch("/", "//"); - checkNoMatch("/abc/def/ghi", "/abc//def///ghi"); - checkNoMatch("/abc", "////abc"); - checkMatches("////abc", "////abc"); - checkNoMatch("/", "//"); - checkNoMatch("/abc//def", "/abc/def"); - checkNoMatch("/abc//def///ghi", "/abc/def/ghi"); - checkMatches("/abc//def///ghi", "/abc//def///ghi"); - } - - @Test - 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"); - } - - @SuppressWarnings("deprecation") - @Test - 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"); - checkMatches("/*/bar", "/foo/bar"); - checkMatches("/a*b*c*d/bar", "/abcd/bar"); - checkMatches("*a*", "testa"); - checkMatches("a/*", "a/"); - checkNoMatch("a/*", "a//"); // no data for * - PathPatternParser ppp = new PathPatternParser(); - assertThat(ppp.parse("a/*").matches(toPathContainer("a//"))).isFalse(); - checkMatches("a/*", "a/a"); - checkMatches("/resource/**", "/resource"); - checkNoMatch("/resource/**", "/resourceX"); - checkNoMatch("/resource/**", "/resourceX/foobar"); - checkMatches("/resource/**", "/resource/foobar"); - } - - @Test - void constrainedMatches() { - checkCapture("{foo:[0-9]*}", "123", "foo", "123"); - checkNoMatch("{foo:[0-9]*}", "abc"); - checkNoMatch("/{foo:[0-9]*}", "abc"); - checkCapture("/*/{foo:....}/**", "/foo/barg/foo", "foo", "barg"); - checkCapture("/*/{foo:....}/**", "/foo/barg/abc/def/ghi", "foo", "barg"); - checkNoMatch("{foo:....}", "99"); - checkMatches("{foo:..}", "99"); - checkCapture("/{abc:\\{\\}}", "/{}", "abc", "{}"); - checkCapture("/{abc:\\[\\]}", "/[]", "abc", "[]"); - checkCapture("/{abc:\\\\\\\\}", "/\\\\"); // this is fun... - } - - @Test - void antPathMatcherTests() { - // test exact matching - checkMatches("test", "test"); - checkMatches("/test", "/test"); - checkMatches("https://example.org", "https://example.org"); - checkNoMatch("/test.jpg", "test.jpg"); - checkNoMatch("test", "/test"); - checkNoMatch("/test", "test"); - - // test matching with ?'s - checkMatches("t?st", "test"); - checkMatches("??st", "test"); - checkMatches("tes?", "test"); - checkMatches("te??", "test"); - checkMatches("?es?", "test"); - checkNoMatch("tes?", "tes"); - checkNoMatch("tes?", "testt"); - checkNoMatch("tes?", "tsst"); - - // test matching with *'s - checkMatches("*", "test"); - checkMatches("test*", "test"); - checkMatches("test*", "testTest"); - checkMatches("test/*", "test/Test"); - checkMatches("test/*", "test/t"); - checkMatches("test/*", "test/"); - checkMatches("*test*", "AnothertestTest"); - checkMatches("*test", "Anothertest"); - checkMatches("*.*", "test."); - checkMatches("*.*", "test.test"); - checkMatches("*.*", "test.test.test"); - checkMatches("test*aaa", "testblaaaa"); - checkNoMatch("test*", "tst"); - checkNoMatch("test*", "tsttest"); - checkMatches("test*", "test"); // trailing slash is optional - checkNoMatch("test*", "test/t"); - checkNoMatch("test/*", "test"); - checkNoMatch("*test*", "tsttst"); - checkNoMatch("*test", "tsttst"); - checkNoMatch("*.*", "tsttst"); - checkNoMatch("test*aaa", "test"); - checkNoMatch("test*aaa", "testblaaab"); - - // test matching with ?'s and /'s - checkMatches("/?", "/a"); - checkMatches("/?/a", "/a/a"); - checkMatches("/a/?", "/a/b"); - checkMatches("/??/a", "/aa/a"); - checkMatches("/a/??", "/a/bb"); - checkMatches("/?", "/a"); - - checkMatches("/**", ""); - checkMatches("/books/**", "/books"); - checkMatches("/**", "/testing/testing"); - checkMatches("/*/**", "/testing/testing"); - checkMatches("/bla*bla/test", "/blaXXXbla/test"); - checkMatches("/*bla/test", "/XXXbla/test"); - checkNoMatch("/bla*bla/test", "/blaXXXbl/test"); - checkNoMatch("/*bla/test", "XXXblab/test"); - checkNoMatch("/*bla/test", "XXXbl/test"); - checkNoMatch("/????", "/bala/bla"); - checkMatches("/foo/bar/**", "/foo/bar/"); - checkMatches("/{bla}.html", "/testing.html"); - checkCapture("/{bla}.*", "/testing.html", "bla", "testing"); - } - - @Test - void pathRemainingEnhancements_spr15419() { + @Test // SPR-15149 + void pathRemainingEnhancements() { PathPattern pp; PathPattern.PathRemainingMatchInfo pri; // It would be nice to partially match a path and get any bound variables in one step @@ -495,8 +540,8 @@ class PathPatternTests { assertMatches(p,"bAb"); } - @Test - void extractPathWithinPattern_spr15259() { + @Test // SPR-15259 + void extractPathWithinPatternWildards() { checkExtractPathWithinPattern("/**","//",""); checkExtractPathWithinPattern("/**","/",""); checkExtractPathWithinPattern("/**","",""); @@ -553,9 +598,8 @@ class PathPatternTests { assertThat(result.elements()).hasSize(3); } - @Test - @SuppressWarnings("deprecation") - public void extractUriTemplateVariables_spr15264() { + @Test // SPR-15264 + public void extractUriTemplateVariables() { PathPattern pp; pp = new PathPatternParser().parse("/{foo}"); assertMatches(pp,"/abc"); @@ -611,10 +655,7 @@ class PathPatternTests { Map vars = new AntPathMatcher().extractUriTemplateVariables("/{foo}{bar}", "/a"); assertThat(vars).containsEntry("foo", "a"); assertThat(vars.get("bar")).isEmpty(); - } - @Test - void extractUriTemplateVariables() { assertMatches(parse("{hotel}"),"1"); assertMatches(parse("/hotels/{hotel}"),"/hotels/1"); checkCapture("/hotels/{hotel}", "/hotels/1", "hotel", "1"); @@ -1003,6 +1044,43 @@ class PathPatternTests { assertThat(result).isNotNull(); } + @Test + void regexPathElementPatterns() { + PathPatternParser pp = new PathPatternParser(); + + PathPattern pattern = pp.parse("/{var:\\\\}"); + assertMatches(pattern, "/\\"); + + pattern = pp.parse("/{var:\\/}"); + assertNoMatch(pattern, "/aaa"); + + pattern = pp.parse("/{var:[^\\/]*}"); + PathPattern.PathMatchInfo result = matchAndExtract(pattern, "/foo"); + assertThat(result.getUriVariables().get("var")).isEqualTo("foo"); + + pattern = pp.parse("/{var:\\[*}"); + result = matchAndExtract(pattern, "/[[["); + assertThat(result.getUriVariables().get("var")).isEqualTo("[[["); + + pattern = pp.parse("/{var:[\\{]*}"); + result = matchAndExtract(pattern, "/{{{"); + assertThat(result.getUriVariables().get("var")).isEqualTo("{{{"); + + pattern = pp.parse("/{var:[\\}]*}"); + result = matchAndExtract(pattern, "/}}}"); + assertThat(result.getUriVariables().get("var")).isEqualTo("}}}"); + } + + private void assertMatches(PathPattern pp, String path) { + assertThat(pp.matches(toPathContainer(path))).isTrue(); + } + + private void assertNoMatch(PathPattern pp, String path) { + assertThat(pp.matches(toPathContainer(path))).isFalse(); + } + + + private PathPattern.PathMatchInfo matchAndExtract(String pattern, String path) { return parse(pattern).matchAndExtract(PathPatternTests.toPathContainer(path)); }