From 508aea8a47b69ac11426a6cedd9ddd9f93117ad2 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Fri, 17 Nov 2017 12:03:17 -0800 Subject: [PATCH] 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 --- .../web/util/pattern/PathPattern.java | 120 ++++++++---------- .../web/util/pattern/PathPatternTests.java | 9 +- 2 files changed, 58 insertions(+), 71 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 45b8776f99..0d1605826e 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 @@ -246,99 +246,79 @@ public class PathPattern implements Comparable { *
  • '{@code /docs/cvs/*.html}' and '{@code /docs/cvs/commit.html} -> '{@code commit.html}'
    • *
  • '{@code /docs/**}' and '{@code /docs/cvs/commit} -> '{@code cvs/commit}'
    • * - *

    Note: Assumes that {@link #matches} returns {@code true} for + *

    Notes: + *

    * @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 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. 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 e9d7d3cd49..a2c3b86f80 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 @@ -664,7 +664,7 @@ public class PathPatternTests { @Test public void extractPathWithinPattern_spr15259() { - checkExtractPathWithinPattern("/**","//","/"); + checkExtractPathWithinPattern("/**","//",""); checkExtractPathWithinPattern("/**","/",""); checkExtractPathWithinPattern("/**","",""); checkExtractPathWithinPattern("/**","/foobar","foobar"); @@ -682,6 +682,13 @@ public class PathPatternTests { 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");