Skip to content

Commit

Permalink
WARN against invalid patterns with PathPatternParser
Browse files Browse the repository at this point in the history
As of gh-24952, `PathPatternParser` will strictly reject patterns with
`"**"` in the middle of them. `"**"` is only allowed at the end of the
pattern for matching multiple path segments until the end of the path.

Currently, if `"**"` is used in the middle of a pattern it will be
considered as a single `"*"` instead. Rejecting such cases should
clarify the situation.

This commit prepares for that upcoming change and:

* logs a warning message if such a case is used by an application
* expands the MVC and WebFlux documentation about URI matching in
general

Closes gh-24958
  • Loading branch information
bclozel committed Apr 23, 2020
1 parent 5eba1ae commit dc4cda1
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
* and captures it as a variable named "spring"</li>
* </ul>
*
* Notable behavior difference with {@code AntPathMatcher}:<br>
* {@code **} and its capturing variant <code>{*spring}</code> cannot be used in the middle of a pattern
* string, only at the end: {@code /pages/{**}} is valid, but {@code /pages/{**}/details} is not.
*
* <h3>Examples</h3>
* <ul>
* <li>{@code /pages/t?st.html} &mdash; matches {@code /pages/test.html} as well as
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,9 @@

package org.springframework.web.util.pattern;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.http.server.PathContainer;

/**
Expand All @@ -34,6 +37,8 @@
*/
public class PathPatternParser {

private static final Log logger = LogFactory.getLog(PathPatternParser.class);

private boolean matchOptionalTrailingSeparator = true;

private boolean caseSensitive = true;
Expand Down Expand Up @@ -104,11 +109,16 @@ public PathContainer.Options getPathOptions() {
* stage. Produces a PathPattern object that can be used for fast matching
* against paths. Each invocation of this method delegates to a new instance of
* the {@link InternalPathPatternParser} because that class is not thread-safe.
* @param pathPattern the input path pattern, e.g. /foo/{bar}
* @param pathPattern the input path pattern, e.g. /project/{name}
* @return a PathPattern for quickly matching paths against request paths
* @throws PatternParseException in case of parse errors
*/
public PathPattern parse(String pathPattern) throws PatternParseException {
int wildcardIndex = pathPattern.indexOf("**" + this.pathOptions.separator());
if (wildcardIndex != -1 && wildcardIndex != pathPattern.length() - 3) {
logger.warn("'**' patterns are not supported in the middle of patterns and will be rejected in the future. " +
"Consider using '*' instead for matching a single path segment.");
}
return new InternalPathPatternParser(this).parse(pathPattern);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -414,6 +414,14 @@ public void compareTests() {
assertThat(patterns.get(1)).isEqualTo(p2);
}

@Test // Should be updated with gh-24952
public void doubleWildcardWithinPatternNotSupported() {
PathPatternParser parser = new PathPatternParser();
PathPattern pattern = parser.parse("/resources/**/details");
assertThat(pattern.matches(PathContainer.parsePath("/resources/test/details"))).isTrue();
assertThat(pattern.matches(PathContainer.parsePath("/resources/projects/spring/details"))).isFalse();
}

@Test
public void separatorTests() {
PathPatternParser parser = new PathPatternParser();
Expand Down
42 changes: 37 additions & 5 deletions src/docs/asciidoc/web/webflux.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1402,12 +1402,44 @@ The following example uses type and method level mappings:

You can map requests by using glob patterns and wildcards:

* `?` matches one character
* `*` matches zero or more characters within a path segment
* `**` match zero or more path segments
[cols="2,3,5"]
|===
|Pattern |Description |Example

You can also declare URI variables and access their values with `@PathVariable`,
as the following example shows:
| `+?+`
| 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 until the end of the path
| `+"/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.

| `+{name}+`
| Matches a path segment and captures it as a variable named "name"
| `+"/projects/{project}/versions"+` matches `+"/projects/spring/versions"+` and captures `+project=spring+`

| `+{name:[a-z]+}+`
| Matches the regexp `+"[a-z]+"+` as a path variable named "name"
| `+"/projects/{project:[a-z]+}/versions"+` matches `+"/projects/spring/versions"+` but not `+"/projects/spring1/versions"+`

| `+{*path}+`
| Matches zero or more path segments until the end of the path and captures it as a variable named "path"
| `+"/resources/{*file}"+` matches `+"/resources/images/file.png"+` and captures `+file=resources/file.png+`

|===

Captured URI variables can be accessed with `@PathVariable`, as the following example shows:

[source,java,indent=0,subs="verbatim,quotes",role="primary"]
.Java
Expand Down
38 changes: 32 additions & 6 deletions src/docs/asciidoc/web/webmvc.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1535,14 +1535,40 @@ The following example has type and method level mappings:
==== URI patterns
[.small]#<<web-reactive.adoc#webflux-ann-requestmapping-uri-templates, WebFlux>>#

You can map requests by using the following global patterns and wildcards:
You can map requests by using glob patterns and wildcards:

* `?` matches one character
* `*` matches zero or more characters within a path segment
* `**` match zero or more path segments
[cols="2,3,5"]
|===
|Pattern |Description |Example

You can also declare URI variables and access their values with `@PathVariable`,
as the following example shows:
| `+?+`
| 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 until the end of the path
| `+"/resources/**"+` matches `+"/resources/file.png"+` and `+"/resources/images/file.png"+`

| `+{name}+`
| Matches a path segment and captures it as a variable named "name"
| `+"/projects/{project}/versions"+` matches `+"/projects/spring/versions"+` and captures `+project=spring+`

| `+{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"+`

|===

Captured URI variables can be accessed with `@PathVariable`, as the following example shows:

[source,java,indent=0,subs="verbatim,quotes",role="primary"]
.Java
Expand Down

0 comments on commit dc4cda1

Please sign in to comment.