From 62fa20fd6f4b3ebbc2eaf47ebeb5d9f2d91f21c2 Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Wed, 2 Aug 2017 16:31:06 +0200
Subject: [PATCH] PathPattern#matchAndExtract minor refactoring

Consistent behavior with matches(PathContainer), the two had slightly
different logic for handling of empty paths.

Make matchAndExtract independantly usable without the need to call
matches(PathContainer) first. Essentially no longer raising ISE if the
pattern doesn't match but simply returning null.
---
 .../web/util/pattern/PathPattern.java         | 24 ++++++++++---------
 .../web/util/pattern/PathPatternTests.java    | 18 ++++----------
 .../function/server/RequestPredicates.java    |  8 +++----
 3 files changed, 21 insertions(+), 29 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 2af53b1cb8..ef608e503f 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
@@ -175,26 +175,28 @@ public class PathPattern implements Comparable<PathPattern> {
 		MatchingContext matchingContext = new MatchingContext(pathContainer, false);
 		return this.head.matches(0, matchingContext);
 	}
-	
+
 	/**
 	 * Match this pattern to the given URI path and return extracted URI template
 	 * variables as well as path parameters (matrix variables).
 	 * @param pathContainer the candidate path to attempt to match against
-	 * @return info object with the extracted variables
-	 * @throws IllegalStateException if the path does not match the pattern
+	 * @return info object with the extracted variables, or {@code null} for no match
 	 */
+	@Nullable
 	public PathMatchInfo matchAndExtract(PathContainer pathContainer) {
-		MatchingContext matchingContext = new MatchingContext(pathContainer, true);
-		if (this.head != null && this.head.matches(0, matchingContext)) {
-			return matchingContext.getPathMatchResult();
+		if (this.head == null) {
+			return hasLength(pathContainer) ? null : PathMatchInfo.EMPTY;
 		}
 		else if (!hasLength(pathContainer)) {
-			return PathMatchInfo.EMPTY;
-		}
-		else {
-			throw new IllegalStateException(
-					"Pattern \"" + this + "\" is not a match for \"" + pathContainer.value() + "\"");
+			if (this.head instanceof WildcardTheRestPathElement || this.head instanceof CaptureTheRestPathElement) {
+				pathContainer = EMPTY_PATH; // Will allow CaptureTheRest to bind the variable to empty
+			}
+			else {
+				return null;
+			}
 		}
+		MatchingContext matchingContext = new MatchingContext(pathContainer, true);
+		return this.head.matches(0, matchingContext) ? matchingContext.getPathMatchResult() : null;
 	}
 
 	/**
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 181bea804b..0ff812d1cb 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
@@ -768,20 +768,10 @@ public class PathPatternTests {
 		checkCapture("/{page}.*", "/42.html", "page", "42");
 		checkCapture("/A-{B}-C", "/A-b-C", "B", "b");
 		checkCapture("/{name}.{extension}", "/test.html", "name", "test", "extension", "html");
-		try {
-			checkCapture("/{one}/", "//", "one", "");
-			fail("Expected exception");
-		}
-		catch (IllegalStateException e) {
-			assertEquals("Pattern \"/{one}/\" is not a match for \"//\"", e.getMessage());
-		}
-		try {
-			checkCapture("", "/abc");
-			fail("Expected exception");
-		}
-		catch (IllegalStateException e) {
-			assertEquals("Pattern \"\" is not a match for \"/abc\"", e.getMessage());
-		}
+
+		assertNull(checkCapture("/{one}/", "//"));
+		assertNull(checkCapture("", "/abc"));
+
 		assertEquals(0, checkCapture("", "").getUriVariables().size());
 		checkCapture("{id}", "99", "id", "99");
 		checkCapture("/customer/{customerId}", "/customer/78", "customerId", "78");
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
index dbf210ad24..108b1b6a34 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
@@ -343,10 +343,10 @@ public abstract class RequestPredicates {
 		@Override
 		public boolean test(ServerRequest request) {
 			PathContainer pathContainer = request.pathContainer();
-			boolean match = this.pattern.matches(pathContainer);
-			traceMatch("Pattern", this.pattern.getPatternString(), request.path(), match);
-			if (match) {
-				mergeTemplateVariables(request, this.pattern.matchAndExtract(pathContainer).getUriVariables());
+			PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(pathContainer);
+			traceMatch("Pattern", this.pattern.getPatternString(), request.path(), info != null);
+			if (info != null) {
+				mergeTemplateVariables(request, info.getUriVariables());
 				return true;
 			}
 			else {