Rework implementation of PathPattern.extractPathWithinPattern

This commit changes the implementation of the PathPattern
extractPathWithinPattern method that used an old AntPathMatcher
derivative to a new version that integrates more closely with
PathContainer.

It also introduces consistency in a couple of areas. The javadoc
is updated to specify this but basically:
- the response from the extra method will have all leading
  and trailing separators removed.
- the response will have multiple adjacent separators within the
  reponse reduced to just one.  (For example response would be
  aaa/bb/cc and not aaa///bbb//cc)
If your response would start or finish with multiple separators, they
are all removed.

Issue: SPR-16120

Author: aclement

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

@@ -246,99 +246,79 @@ else if (!hasLength(pathContainer)) {
 	 * <li>'{@code /docs/cvs/*.html}' and '{@code /docs/cvs/commit.html} -> '{@code commit.html}'</li>
 	 * <li>'{@code /docs/**}' and '{@code /docs/cvs/commit} -> '{@code cvs/commit}'</li>
 	 * </ul>
-	 * <p><b>Note:</b> Assumes that {@link #matches} returns {@code true} for
+	 * <p><b>Notes:</b>
+	 * <ul>
+	 * <li>Assumes that {@link #matches} returns {@code true} for
 	 * the same path but does <strong>not</strong> enforce this.
+	 * <li>Duplicate occurrences of separators within the returned result are removed
+	 * <li>Leading and trailing separators are removed from the returned result
+	 * </ul>
 	 * @param path a path that matches this pattern
 	 * @return the subset of the path that is matched by pattern or "" if none
 	 * of it is matched by pattern elements
 	 */
 	public PathContainer extractPathWithinPattern(PathContainer path) {
+		List<Element> pathElements = path.elements();
+		int pathElementsCount = pathElements.size();
 
-		// TODO: implement extractPathWithinPattern for PathContainer
-
-		// TODO: align behavior with matchStartOfPath with regards to this:
-		// As per the contract on {@link PathMatcher}, this method will trim leading/trailing
-		// separators. It will also remove duplicate separators in the returned path.
-
-		String result = extractPathWithinPattern(path.value());
-		return PathContainer.parsePath(result);
-	}
-
-	private String extractPathWithinPattern(String path) {
-		// assert this.matches(path)
+		int startIndex = 0;
+		// Find first path element that is not a separator or a literal (i.e. the first pattern based element)
 		PathElement elem = head;
-		int separatorCount = 0;
-		boolean matchTheRest = false;
-
-		// Find first path element that is pattern based
 		while (elem != null) {
-			if (elem instanceof SeparatorPathElement || elem instanceof CaptureTheRestPathElement ||
-					elem instanceof WildcardTheRestPathElement) {
-				separatorCount++;
-				if (elem instanceof WildcardTheRestPathElement || elem instanceof CaptureTheRestPathElement) {
-					matchTheRest = true;
-				}
-			}
 			if (elem.getWildcardCount() != 0 || elem.getCaptureCount() != 0) {
 				break;
 			}
 			elem = elem.next;
+			startIndex++;
 		}
-
 		if (elem == null) {
-			return "";  // there is no pattern mapped section
-		}
-
-		// Now separatorCount indicates how many sections of the path to skip
-		char[] pathChars = path.toCharArray();
-		int len = pathChars.length;
-		int pos = 0;
-		while (separatorCount > 0 && pos < len) {
-			if (path.charAt(pos++) == separator) {
-				separatorCount--;
-			}
-		}
-		int end = len;
-		// Trim trailing separators
-		if (!matchTheRest) {
-			while (end > 0 && path.charAt(end - 1) == separator) {
-				end--;
+			// There is no pattern piece
+			return PathContainer.parsePath("");
+		}
+		
+		// Skip leading separators that would be in the result
+		while (startIndex < pathElementsCount && (pathElements.get(startIndex) instanceof Separator)) {
+			startIndex++;
+		}
+		
+		int endIndex = pathElements.size();
+		// Skip trailing separators that would be in the result
+		while (endIndex > 0 && (pathElements.get(endIndex - 1) instanceof Separator)) {
+			endIndex--;
+		}
+		
+		boolean multipleAdjacentSeparators = false;
+		for (int i = startIndex; i < (endIndex - 1); i++) {
+			if ((pathElements.get(i) instanceof Separator) && (pathElements.get(i+1) instanceof Separator)) {
+				multipleAdjacentSeparators=true;
+				break;
 			}
 		}
-
-		// Check if multiple separators embedded in the resulting path, if so trim them out.
-		// Example: aaa////bbb//ccc/d -> aaa/bbb/ccc/d
-		// The stringWithDuplicateSeparatorsRemoved is only computed if necessary
-		int c = pos;
-		StringBuilder stringWithDuplicateSeparatorsRemoved = null;
-		while (c < end) {
-			char ch = path.charAt(c);
-			if (ch == separator) {
-				if ((c + 1) < end && path.charAt(c + 1) == separator) {
-					// multiple separators
-					if (stringWithDuplicateSeparatorsRemoved == null) {
-						// first time seen, need to capture all data up to this point
-						stringWithDuplicateSeparatorsRemoved = new StringBuilder();
-						stringWithDuplicateSeparatorsRemoved.append(path.substring(pos, c));
-					}
-					do {
-						c++;
-					} while ((c + 1) < end && path.charAt(c + 1) == separator);
+		
+		PathContainer resultPath = null;
+		if (multipleAdjacentSeparators) {
+			// Need to rebuild the path without the duplicate adjacent separators
+			StringBuilder buf = new StringBuilder();
+			int i = startIndex;
+			while (i < endIndex) {
+				Element e = pathElements.get(i++);
+				buf.append(e.value());
+				if (e instanceof Separator) {
+					while (i < endIndex && (pathElements.get(i) instanceof Separator)) {
+						i++;
+					} 
 				}
 			}
-			if (stringWithDuplicateSeparatorsRemoved != null) {
-				stringWithDuplicateSeparatorsRemoved.append(ch);
-			}
-			c++;
+			resultPath = PathContainer.parsePath(buf.toString());
+		} else if (startIndex >= endIndex) {
+			resultPath = PathContainer.parsePath("");
 		}
-
-		if (stringWithDuplicateSeparatorsRemoved != null) {
-			return stringWithDuplicateSeparatorsRemoved.toString();
+		else {
+			resultPath = path.subPath(startIndex, endIndex);
 		}
-		return (pos == len ? "" : path.substring(pos, end));
+		return resultPath;
 	}
 
-
 	/**
 	 * Compare this pattern with a supplied pattern: return -1,0,+1 if this pattern
 	 * is more specific, the same or less specific than the supplied pattern.

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

@@ -664,7 +664,7 @@ public void caseSensitivity() {
 
 	@Test
 	public void extractPathWithinPattern_spr15259() { 
-		checkExtractPathWithinPattern("/**","//","/");
+		checkExtractPathWithinPattern("/**","//","");
 		checkExtractPathWithinPattern("/**","/","");
 		checkExtractPathWithinPattern("/**","","");
 		checkExtractPathWithinPattern("/**","/foobar","foobar");
@@ -682,6 +682,13 @@ public void extractPathWithinPattern() throws Exception {
 		checkExtractPathWithinPattern("/*.html", "/commit.html", "commit.html");
 		checkExtractPathWithinPattern("/docs/*/*/*/*", "/docs/cvs/other/commit.html", "cvs/other/commit.html");
 		checkExtractPathWithinPattern("/d?cs/**", "/docs/cvs/commit", "docs/cvs/commit");
+		checkExtractPathWithinPattern("/*/**", "/docs/cvs/commit///", "docs/cvs/commit");
+		checkExtractPathWithinPattern("/*/**", "/docs/cvs/commit/", "docs/cvs/commit");
+		checkExtractPathWithinPattern("/aaa/bbb/**", "/aaa///","");
+		checkExtractPathWithinPattern("/aaa/bbb/**", "/aaa//","");
+		checkExtractPathWithinPattern("/aaa/bbb/**", "/aaa/","");
+		checkExtractPathWithinPattern("/docs/**", "/docs/cvs/commit///", "cvs/commit");
+		checkExtractPathWithinPattern("/docs/**", "/docs/cvs/commit/", "cvs/commit");
 		checkExtractPathWithinPattern("/docs/c?s/*.html", "/docs/cvs/commit.html", "cvs/commit.html");
 		checkExtractPathWithinPattern("/d?cs/*/*.html", "/docs/cvs/commit.html", "docs/cvs/commit.html");
 		checkExtractPathWithinPattern("/a/b/c*d*/*.html", "/a/b/cod/foo.html", "cod/foo.html");