From d3c1e678c20337b800a08847c3f334fcdce73d70 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 25 Jul 2025 12:34:40 +0200 Subject: [PATCH 1/2] Support wildcard path elements at the start of path patterns Prior to this commit, the `PathPattern` and `PathPatternParser` would allow multiple-segments matching and capturing with the following: * "/files/**" (matching 0-N segments until the end) * "/files/{*path}" (matching 0-N segments until the end and capturing the value as the "path" variable) This would be only allowed as the last path element in the pattern and the parser would reject other combinations. This commit expands the support and allows multiple segments matching at the beginning of the path: * "/**/index.html" (matching 0-N segments from the start) * "/{*path}/index.html" (matching 0-N segments until the end and capturing the value as the "path" variable) This does come with additional restrictions: 1. "/files/**/file.txt" and "/files/{*path}/file.txt" are invalid, as multiple segment matching is not allowed in the middle of the pattern. 2. "/{*path}/files/**" is not allowed, as a single "{*path}" or "/**" element is allowed in a pattern 3. "/{*path}/{folder}/file.txt" "/**/{folder:[a-z]+}/file.txt" are invalid because only a literal pattern is allowed right after multiple segments path elements. Closes gh-35679 --- .../controller/ann-requestmapping.adoc | 26 +- .../mvc-controller/ann-requestmapping.adoc | 73 +- ...t.java => CaptureSegmentsPathElement.java} | 59 +- .../pattern/InternalPathPatternParser.java | 94 +- .../web/util/pattern/PathElement.java | 2 +- .../web/util/pattern/PathPattern.java | 6 +- .../util/pattern/PatternParseException.java | 7 +- ....java => WildcardSegmentsPathElement.java} | 32 +- .../util/pattern/PathPatternParserTests.java | 580 ++++++------ .../web/util/pattern/PathPatternTests.java | 835 ++++++++---------- 10 files changed, 866 insertions(+), 848 deletions(-) rename spring-web/src/main/java/org/springframework/web/util/pattern/{CaptureTheRestPathElement.java => CaptureSegmentsPathElement.java} (59%) rename spring-web/src/main/java/org/springframework/web/util/pattern/{WildcardTheRestPathElement.java => WildcardSegmentsPathElement.java} (51%) 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 8f572a42112..4cd8a601bf0 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,10 +108,17 @@ 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" @@ -119,9 +130,18 @@ You can map requests by using glob patterns and wildcards: | `/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 64f5abec6c4..534cc947a10 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 @@ -103,22 +103,63 @@ Spring WebFlux. It was enabled for use in Spring MVC from version 5.3 and is ena 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 a0ccb7cfe98..21b0ce3f066 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 @@ -29,6 +29,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 { @@ -51,7 +52,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 @@ -66,6 +67,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 @Nullable private List capturedVariableNames; @@ -110,13 +114,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) { @@ -144,35 +142,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, @@ -185,6 +185,7 @@ class InternalPathPatternParser { if (this.pathElementStart != -1) { pushPathElement(createPathElement()); } + verifyPatternElements(this.headPE); return new PathPattern(pathPattern, this.parser, this.headPE); } @@ -234,23 +235,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); } /** @@ -258,7 +264,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) { @@ -279,7 +286,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 { @@ -320,9 +328,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 { @@ -341,7 +351,7 @@ class InternalPathPatternParser { } } else { - if (this.isCaptureTheRestVariable) { + if (this.isCaptureSegmentsVariable) { throw new PatternParseException(this.pathElementStart, this.pathPatternData, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); } @@ -405,7 +415,7 @@ class InternalPathPatternParser { this.insideVariableCapture = false; this.variableCaptureCount = 0; this.wildcard = false; - this.isCaptureTheRestVariable = false; + this.isCaptureSegmentsVariable = false; this.variableCaptureStart = -1; } @@ -423,4 +433,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 2dbab9e6764..499936ce82f 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 @@ -109,7 +109,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 6ef856fb9da..b7a27d4b94d 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 @@ -167,7 +167,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) { @@ -206,7 +206,7 @@ public class PathPattern implements Comparable { (this.matchOptionalTrailingSeparator && pathContainerIsJustSeparator(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 { @@ -231,7 +231,7 @@ public class PathPattern implements Comparable { 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 cf790dcbb84..7f9df03242f 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,250 +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("", "/"); - 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(); - } - - @SuppressWarnings("deprecation") - @Test - void optionalTrailingSeparators() { - PathPattern pp; - // LiteralPathElement - pp = parse("/resource"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parse("/resource/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - pp = parse("res?urce"); - assertNoMatch(pp,"resource//"); - // SingleCharWildcardPathElement - pp = parse("/res?urce"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parse("/res?urce/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - // CaptureVariablePathElement - pp = parse("/{var}"); - assertMatches(pp,"/resource"); - assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry("var", "resource"); - assertMatches(pp,"/resource/"); - assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry( - "var", - "resource" - ); - assertNoMatch(pp,"/resource//"); - pp = parse("/{var}/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry( - "var", - "resource" - ); - assertNoMatch(pp,"/resource//"); - - // CaptureTheRestPathElement - pp = parse("/{*var}"); - assertMatches(pp,"/resource"); - assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry( - "var", - "/resource" - ); - assertMatches(pp,"/resource/"); - assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry( - "var", - "/resource/" - ); - assertMatches(pp,"/resource//"); - assertThat(pp.matchAndExtract(toPathContainer("/resource//")).getUriVariables()).containsEntry( - "var", - "/resource//" - ); - assertMatches(pp,"//resource//"); - assertThat(pp.matchAndExtract(toPathContainer("//resource//")).getUriVariables()).containsEntry( - "var", - "//resource//" - ); - - // WildcardTheRestPathElement - pp = parse("/**"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertMatches(pp,"/resource//"); - assertMatches(pp,"//resource//"); - - // WildcardPathElement - pp = parse("/*"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parse("/*/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - // RegexPathElement - pp = parse("/{var1}_{var2}"); - assertMatches(pp,"/res1_res2"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var1", "res1"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var2", "res2"); - assertMatches(pp,"/res1_res2/"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2"); - assertNoMatch(pp,"/res1_res2//"); - pp = parse("/{var1}_{var2}/"); - assertNoMatch(pp,"/res1_res2"); - assertMatches(pp,"/res1_res2/"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2"); - assertNoMatch(pp,"/res1_res2//"); - pp = parse("/{var1}*"); - assertMatches(pp,"/a"); - assertMatches(pp,"/a/"); - assertNoMatch(pp,"/"); // no characters for var1 - assertNoMatch(pp,"//"); // no characters for var1 - - // Now with trailing matching turned OFF - PathPatternParser parser = new PathPatternParser(); - parser.setMatchOptionalTrailingSeparator(false); - // LiteralPathElement - pp = parser.parse("/resource"); - assertMatches(pp,"/resource"); - assertNoMatch(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parser.parse("/resource/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - // SingleCharWildcardPathElement - pp = parser.parse("/res?urce"); - assertMatches(pp,"/resource"); - assertNoMatch(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parser.parse("/res?urce/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - // CaptureVariablePathElement - pp = parser.parse("/{var}"); - assertMatches(pp,"/resource"); - assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry("var", "resource"); - assertNoMatch(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parser.parse("/{var}/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry( - "var", - "resource" - ); - assertNoMatch(pp,"/resource//"); - - // CaptureTheRestPathElement - pp = parser.parse("/{*var}"); - assertMatches(pp,"/resource"); - assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry( - "var", - "/resource" - ); - assertMatches(pp,"/resource/"); - assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry( - "var", - "/resource/" - ); - assertMatches(pp,"/resource//"); - assertThat(pp.matchAndExtract(toPathContainer("/resource//")).getUriVariables()).containsEntry( - "var", - "/resource//" - ); - assertMatches(pp,"//resource//"); - assertThat(pp.matchAndExtract(toPathContainer("//resource//")).getUriVariables()).containsEntry( - "var", - "//resource//" - ); - - // WildcardTheRestPathElement - pp = parser.parse("/**"); - assertMatches(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertMatches(pp,"/resource//"); - assertMatches(pp,"//resource//"); - - // WildcardPathElement - pp = parser.parse("/*"); - assertMatches(pp,"/resource"); - assertNoMatch(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - pp = parser.parse("/*/"); - assertNoMatch(pp,"/resource"); - assertMatches(pp,"/resource/"); - assertNoMatch(pp,"/resource//"); - - // RegexPathElement - pp = parser.parse("/{var1}_{var2}"); - assertMatches(pp,"/res1_res2"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var1", "res1"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var2", "res2"); - assertNoMatch(pp,"/res1_res2/"); - assertNoMatch(pp,"/res1_res2//"); - pp = parser.parse("/{var1}_{var2}/"); - assertNoMatch(pp,"/res1_res2"); - assertMatches(pp,"/res1_res2/"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1"); - assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2"); - assertNoMatch(pp,"/res1_res2//"); - pp = parser.parse("/{var1}*"); - assertMatches(pp,"/a"); - assertNoMatch(pp,"/a/"); - assertNoMatch(pp,"/"); // no characters for var1 - assertNoMatch(pp,"//"); // no characters for var1 - } - - @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("/"); @@ -325,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(); @@ -372,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}") @@ -400,195 +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 * - checkMatches("a/*", "a/a/"); // trailing slash, so is allowed - PathPatternParser ppp = new PathPatternParser(); - ppp.setMatchOptionalTrailingSeparator(false); - assertThat(ppp.parse("a/*").matches(toPathContainer("a//"))).isFalse(); - checkMatches("a/*", "a/a"); - checkMatches("a/*", "a/a/"); // trailing slash is optional - checkMatches("/resource/**", "/resource"); - checkNoMatch("/resource/**", "/resourceX"); - checkNoMatch("/resource/**", "/resourceX/foobar"); - checkMatches("/resource/**", "/resource/foobar"); - } - - @Test - 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 - 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 @@ -705,8 +540,8 @@ class PathPatternTests { assertMatches(p,"bAb"); } - @Test - void extractPathWithinPattern_spr15259() { + @Test // SPR-15259 + void extractPathWithinPatternWildards() { checkExtractPathWithinPattern("/**","//",""); checkExtractPathWithinPattern("/**","/",""); checkExtractPathWithinPattern("/**","",""); @@ -763,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"); @@ -805,7 +639,6 @@ class PathPatternTests { // Only patterns not capturing variables cannot match against just / PathPatternParser ppp = new PathPatternParser(); - ppp.setMatchOptionalTrailingSeparator(true); pp = ppp.parse("/****"); assertMatches(pp,"/abcdef"); assertMatches(pp,"/"); @@ -822,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"); @@ -900,8 +730,8 @@ class PathPatternTests { PathPatternParser ppp = new PathPatternParser(); PathPattern pathPattern = ppp.parse("/web/{id:foo(bar)?}_{goo}"); assertThatIllegalArgumentException().isThrownBy(() -> - matchAndExtract(pathPattern,"/web/foobar_goo")) - .withMessageContaining("The number of capturing groups in the pattern"); + matchAndExtract(pathPattern,"/web/foobar_goo")) + .withMessageContaining("The number of capturing groups in the pattern"); } @Test @@ -1003,8 +833,8 @@ class PathPatternTests { parse("/hotels/{hotel}/booking"))).isEqualTo(1); assertThat(comparator.compare( - parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"), - parse("/**"))).isEqualTo(-1); + parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"), + parse("/**"))).isEqualTo(-1); assertThat(comparator.compare(parse("/**"), parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"))).isEqualTo(1); assertThat(comparator.compare(parse("/**"), parse("/**"))).isEqualTo(0); @@ -1021,8 +851,8 @@ class PathPatternTests { // SPR-6741 assertThat(comparator.compare( - parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"), - parse("/hotels/**"))).isEqualTo(-1); + parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"), + parse("/hotels/**"))).isEqualTo(-1); assertThat(comparator.compare(parse("/hotels/**"), parse("/hotels/{hotel}/bookings/{booking}/cutomers/{customer}"))).isEqualTo(1); assertThat(comparator.compare(parse("/hotels/foo/bar/**"), @@ -1214,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)); } From a5141b187afb2fa3353065ca79bf51811dbae7ed Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 18 Aug 2025 10:44:15 +0200 Subject: [PATCH 2/2] Fix '**' parsing within a PathPattern segment Prior to this commit, a regexp path segment ending with a double wilcard (like "/path**") would be incorrectly parsed as a double wildcard segment ("/**"). This commit fixes the incorrect parsing. See gh-35679 --- .../web/util/pattern/InternalPathPatternParser.java | 12 ++++++++++-- .../web/util/pattern/PathPatternParserTests.java | 7 ++++++- 2 files changed, 16 insertions(+), 3 deletions(-) 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 21b0ce3f066..2a17dab8abf 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 @@ -236,14 +236,22 @@ class InternalPathPatternParser { } private boolean isDoubleWildcard(char separator) { + // next char is present if ((this.pos + 1) >= this.pathPatternLength) { return false; } + // current char and next char are '*' if (this.pathPatternData[this.pos] != '*' || this.pathPatternData[this.pos + 1] != '*') { return false; } - if ((this.pos + 2) < this.pathPatternLength) { - return this.pathPatternData[this.pos + 2] == separator; + // previous char is a separator, if any + if ((this.pos - 1 >= 0) && (this.pathPatternData[this.pos - 1] != separator)) { + return false; + } + // next char is a separator, if any + if (((this.pos + 2) < this.pathPatternLength) && + this.pathPatternData[this.pos + 2] != separator) { + return false; } return true; } 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 99c3db01549..e70fe5e8328 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 @@ -85,9 +85,14 @@ class PathPatternParserTests { @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); + + pathPattern = checkStructure("/a**bc"); + assertPathElements(pathPattern, SeparatorPathElement.class, RegexPathElement.class); + + pathPattern = checkStructure("/abc**"); + assertPathElements(pathPattern, SeparatorPathElement.class, RegexPathElement.class); } @Test