Browse Source

Relax multiple segment matching constraints in PathPattern

Prior to this commit, gh-35213 allowed wildcard path elments at the
start of path patterns. This came with an additional constraint that
rejected such patterns if the pattern segment following the wildcard one
was not a literal:

* `/**/{name}` was rejected
* `/**/something/{name}` was accepted

The motivation here was to make the performance impact of wildard
patterns as small as possible at runtime.

This commit relaxes this constraint because `/**/*.js` patterns are very
popular in the security space for request matchers.

Closes gh-35686
pull/35814/head
Brian Clozel 1 month ago
parent
commit
285182be27
  1. 6
      framework-docs/modules/ROOT/partials/web/uri-patterns.adoc
  2. 19
      spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java
  3. 1
      spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java
  4. 1
      spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java
  5. 10
      spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java

6
framework-docs/modules/ROOT/partials/web/uri-patterns.adoc

@ -26,9 +26,6 @@
`+"/resources/**/file.png"+` is invalid as `+**+` is not allowed in the middle of the path. `+"/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. `+"/**/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern.
| `+{name}+` | `+{name}+`
@ -49,9 +46,6 @@
`+"/resources/{*path}/file.png"+` is invalid as `{*path}` is not allowed in the middle of the path. `+"/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. `+"/{*path}/spring/**"+` is not allowed, as only a single `+**+`/`+{*path}+` instance is allowed per pattern.
|=== |===

19
spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java

@ -185,7 +185,6 @@ class InternalPathPatternParser {
if (this.pathElementStart != -1) { if (this.pathElementStart != -1) {
pushPathElement(createPathElement()); pushPathElement(createPathElement());
} }
verifyPatternElements(this.headPE);
return new PathPattern(pathPattern, this.parser, this.headPE); return new PathPattern(pathPattern, this.parser, this.headPE);
} }
@ -441,22 +440,4 @@ class InternalPathPatternParser {
this.capturedVariableNames.add(variableName); 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;
}
}
} }

1
spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java

@ -101,7 +101,6 @@ public class PatternParseException extends IllegalArgumentException {
ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR("Char ''{0}'' is not allowed in a captured variable name"), ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR("Char ''{0}'' is not allowed in a captured variable name"),
CANNOT_HAVE_MANY_MULTISEGMENT_PATHELEMENTS("Multiple '{*...}' or '**' pattern elements are not allowed"), 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"), 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 '{*...}'"), BADLY_FORMED_CAPTURE_THE_REST("Expected form when capturing the rest of the path is simply '{*...}'"),
MISSING_REGEX_CONSTRAINT("Missing regex constraint on capture"), MISSING_REGEX_CONSTRAINT("Missing regex constraint on capture"),
ILLEGAL_DOUBLE_CAPTURE("Not allowed to capture ''{0}'' twice in the same pattern"), ILLEGAL_DOUBLE_CAPTURE("Not allowed to capture ''{0}'' twice in the same pattern"),

1
spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java

@ -232,7 +232,6 @@ class PathPatternParserTests {
checkError("/{abc}{*foobar}", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); checkError("/{abc}{*foobar}", 1, PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT);
checkError("/{abc}{*foobar}{foo}", 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}/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:}", 5, PatternMessage.MISSING_REGEX_CONSTRAINT);
checkError("{foo}_{foo}", 0, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "foo"); checkError("{foo}_{foo}", 0, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "foo");
checkError("/{bar}/{bar}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar"); checkError("/{bar}/{bar}", 7, PatternMessage.ILLEGAL_DOUBLE_CAPTURE, "bar");

10
spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java

@ -185,6 +185,16 @@ class PathPatternTests {
checkMatches("/resource/**", "/resource/foobar"); checkMatches("/resource/**", "/resource/foobar");
} }
@Test
void wildcardSegmentsThenNonLiteral() {
checkMatches("/**/*.js", "/script.js");
checkMatches("/**/*.js", "/js/script.js");
checkMatches("/**/*.js", "/files/js/script.js");
checkMatches("/**/{type}/*.js", "/files/js/script.js");
checkNoMatch("/**/*.js", "/files/style.css");
}
@Test @Test
void antPathMatcherTests() { void antPathMatcherTests() {
// test exact matching // test exact matching

Loading…
Cancel
Save