From dc4cda1b131acefa25869691efeac116fe377d32 Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@pivotal.io>
Date: Thu, 23 Apr 2020 12:48:07 +0200
Subject: [PATCH] WARN against invalid patterns with PathPatternParser

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
---
 .../web/util/pattern/PathPattern.java         |  4 ++
 .../web/util/pattern/PathPatternParser.java   | 14 ++++++-
 .../util/pattern/PathPatternParserTests.java  | 10 ++++-
 src/docs/asciidoc/web/webflux.adoc            | 42 ++++++++++++++++---
 src/docs/asciidoc/web/webmvc.adoc             | 38 ++++++++++++++---
 5 files changed, 94 insertions(+), 14 deletions(-)

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 9c1b6cfe73..84b409b168 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
@@ -47,6 +47,10 @@ import org.springframework.util.StringUtils;
  * 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
diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java
index 2a5152fea4..d083514f09 100644
--- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java
+++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java
@@ -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.
@@ -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;
 
 /**
@@ -34,6 +37,8 @@ import org.springframework.http.server.PathContainer;
  */
 public class PathPatternParser {
 
+	private static final Log logger = LogFactory.getLog(PathPatternParser.class);
+
 	private boolean matchOptionalTrailingSeparator = true;
 
 	private boolean caseSensitive = true;
@@ -104,11 +109,16 @@ public class PathPatternParser {
 	 * 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);
 	}
 
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 918af190af..8b6c204c4f 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
@@ -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.
@@ -414,6 +414,14 @@ public class PathPatternParserTests {
 		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();
diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc
index 43535a2d0a..b8e8e9be02 100644
--- a/src/docs/asciidoc/web/webflux.adoc
+++ b/src/docs/asciidoc/web/webflux.adoc
@@ -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
diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc
index d492c2524f..584cecdc1b 100644
--- a/src/docs/asciidoc/web/webmvc.adoc
+++ b/src/docs/asciidoc/web/webmvc.adoc
@@ -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