From 1d201a55dbea44f6a7499c9203c3f598eeb1ffa2 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Jul 2017 06:35:59 +0200 Subject: [PATCH] Refine PathContainer.Segment value representation Segment.value() now returns the actual original path segment value including path parameters while semicolonContent() is removed. valueDecoded() is renamed to valueToMatch() to reflect it is the value for pattern matching which is not only decoded but also has path parameters removed. --- .../server/reactive/DefaultPathContainer.java | 59 +++++++------------ .../server/reactive/DefaultRequestPath.java | 3 - .../http/server/reactive/PathContainer.java | 17 ++---- .../pattern/CaptureTheRestPathElement.java | 2 +- .../web/util/pattern/LiteralPathElement.java | 4 +- .../web/util/pattern/PathPattern.java | 2 +- .../SingleCharWildcardedPathElement.java | 4 +- .../web/util/pattern/WildcardPathElement.java | 2 +- .../reactive/DefaultPathContainerTests.java | 28 ++++----- 9 files changed, 46 insertions(+), 75 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathContainer.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathContainer.java index bc9b6748025..a6b2cce48ce 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathContainer.java @@ -119,14 +119,15 @@ class DefaultPathContainer implements PathContainer { private static PathContainer.Segment parsePathSegment(String input, Charset charset) { int index = input.indexOf(';'); if (index == -1) { - String inputDecoded = StringUtils.uriDecode(input, charset); - return new DefaultPathSegment(input, inputDecoded, "", EMPTY_MAP); + String valueToMatch = StringUtils.uriDecode(input, charset); + return new DefaultPathSegment(input, valueToMatch, EMPTY_MAP); + } + else { + String valueToMatch = StringUtils.uriDecode(input.substring(0, index), charset); + String pathParameterContent = input.substring(index); + MultiValueMap parameters = parseParams(pathParameterContent, charset); + return new DefaultPathSegment(input, valueToMatch, parameters); } - String value = input.substring(0, index); - String valueDecoded = StringUtils.uriDecode(value, charset); - String semicolonContent = input.substring(index); - MultiValueMap parameters = parseParams(semicolonContent, charset); - return new DefaultPathSegment(value, valueDecoded, semicolonContent, parameters); } private static MultiValueMap parseParams(String input, Charset charset) { @@ -189,22 +190,17 @@ class DefaultPathContainer implements PathContainer { private final String value; - private final String valueDecoded; - - private final char[] valueDecodedChars; + private final String valueToMatch; - private final String semicolonContent; + private final char[] valueToMatchAsChars; private final MultiValueMap parameters; - DefaultPathSegment(String value, String valueDecoded, String semicolonContent, - MultiValueMap params) { - + DefaultPathSegment(String value, String valueToMatch, MultiValueMap params) { Assert.isTrue(!value.contains("/"), () -> "Invalid path segment value: " + value); this.value = value; - this.valueDecoded = valueDecoded; - this.valueDecodedChars = valueDecoded.toCharArray(); - this.semicolonContent = semicolonContent; + this.valueToMatch = valueToMatch; + this.valueToMatchAsChars = valueToMatch.toCharArray(); this.parameters = CollectionUtils.unmodifiableMultiValueMap(params); } @@ -214,18 +210,13 @@ class DefaultPathContainer implements PathContainer { } @Override - public String valueDecoded() { - return this.valueDecoded; + public String valueToMatch() { + return this.valueToMatch; } @Override - public char[] valueDecodedChars() { - return this.valueDecodedChars; - } - - @Override - public String semicolonContent() { - return this.semicolonContent; + public char[] valueToMatchAsChars() { + return this.valueToMatchAsChars; } @Override @@ -241,26 +232,16 @@ class DefaultPathContainer implements PathContainer { if (other == null || getClass() != other.getClass()) { return false; } - - DefaultPathSegment segment = (DefaultPathSegment) other; - return (this.value.equals(segment.value) && - this.semicolonContent.equals(segment.semicolonContent) && - this.parameters.equals(segment.parameters)); + return this.value.equals(((DefaultPathSegment) other).value); } @Override public int hashCode() { - int result = this.value.hashCode(); - result = 31 * result + this.semicolonContent.hashCode(); - result = 31 * result + this.parameters.hashCode(); - return result; + return this.value.hashCode(); } public String toString() { - return "[value='" + this.value + "\', " + - "semicolonContent='" + this.semicolonContent + "\', " + - "parameters=" + this.parameters + "']"; - } + return "[value='" + this.value + "', parameters=" + this.parameters + "']"; } } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java index 2aadbbee866..90a6436448b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java @@ -65,9 +65,6 @@ class DefaultRequestPath implements RequestPath { for (int i=0; i < path.elements().size(); i++) { PathContainer.Element element = path.elements().get(i); counter += element.value().length(); - if (element instanceof PathContainer.Segment) { - counter += ((Segment) element).semicolonContent().length(); - } if (length == counter) { return path.subPath(0, i + 1); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java b/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java index 88045eed40e..784cbef5161 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java @@ -104,21 +104,16 @@ public interface PathContainer { interface Segment extends Element { /** - * Return the path segment {@link #value()} decoded. + * Return the path segment value to use for pattern matching purposes. + * This may differ from {@link #value()} such as being decoded, without + * path parameters, etc. */ - String valueDecoded(); + String valueToMatch(); /** - * Variant of {@link #valueDecoded()} as a {@code char[]}. + * Variant of {@link #valueToMatch()} as a {@code char[]}. */ - char[] valueDecodedChars(); - - /** - * Return the portion of the path segment after and including the first - * ";" (semicolon) representing path parameters. The actual parsed - * parameters if any can be obtained via {@link #parameters()}. - */ - String semicolonContent(); + char[] valueToMatchAsChars(); /** * Path parameters parsed from the path segment. diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java index 30a73ec6bb6..d9ed318731d 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureTheRestPathElement.java @@ -86,7 +86,7 @@ class CaptureTheRestPathElement extends PathElement { for (int i = fromSegment, max = pathElements.size(); i < max; i++) { Element element = pathElements.get(i); if (element instanceof Segment) { - buf.append(((Segment)element).valueDecoded()); + buf.append(((Segment)element).valueToMatch()); } else { buf.append(element.value()); diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/LiteralPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/LiteralPathElement.java index 0a33534061f..c1f1ca407f0 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/LiteralPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/LiteralPathElement.java @@ -62,13 +62,13 @@ class LiteralPathElement extends PathElement { if (!(element instanceof Segment)) { return false; } - String value = ((Segment)element).valueDecoded(); + String value = ((Segment)element).valueToMatch(); if (value.length() != len) { // Not enough data to match this path element return false; } - char[] data = ((Segment)element).valueDecodedChars(); + char[] data = ((Segment)element).valueToMatchAsChars(); if (this.caseSensitive) { for (int i = 0; i < len; i++) { if (data[i] != this.text[i]) { 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 51ff839eeeb..dc65fad876f 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 @@ -683,7 +683,7 @@ public class PathPattern implements Comparable { String pathElementValue(int pathIndex) { Element element = (pathIndex < pathLength) ? pathElements.get(pathIndex) : null; if (element instanceof Segment) { - return ((Segment)element).valueDecoded(); + return ((Segment)element).valueToMatch(); } return ""; } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/SingleCharWildcardedPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/SingleCharWildcardedPathElement.java index 95c6f58125d..cc366b23d5d 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/SingleCharWildcardedPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/SingleCharWildcardedPathElement.java @@ -68,13 +68,13 @@ class SingleCharWildcardedPathElement extends PathElement { if (!(element instanceof Segment)) { return false; } - String value = ((Segment)element).valueDecoded(); + String value = ((Segment)element).valueToMatch(); if (value.length() != len) { // Not enough data to match this path element return false; } - char[] data = ((Segment)element).valueDecodedChars(); + char[] data = ((Segment)element).valueToMatchAsChars(); if (this.caseSensitive) { for (int i = 0; i < len; i++) { char ch = this.text[i]; diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardPathElement.java index b3bf34b6a74..a9d49a393f7 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/WildcardPathElement.java @@ -50,7 +50,7 @@ class WildcardPathElement extends PathElement { // Should not match a separator return false; } - segmentData = ((Segment)element).valueDecoded(); + segmentData = ((Segment)element).valueToMatch(); pathIndex++; } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultPathContainerTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultPathContainerTests.java index dab1a246ada..abcf19c8dd4 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultPathContainerTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultPathContainerTests.java @@ -38,14 +38,14 @@ public class DefaultPathContainerTests { @Test public void pathSegment() throws Exception { // basic - testPathSegment("cars", "", "cars", "cars", new LinkedMultiValueMap<>()); + testPathSegment("cars", "cars", new LinkedMultiValueMap<>()); // empty - testPathSegment("", "", "", "", new LinkedMultiValueMap<>()); + testPathSegment("", "", new LinkedMultiValueMap<>()); // spaces - testPathSegment("%20%20", "", "%20%20", " ", new LinkedMultiValueMap<>()); - testPathSegment("%20a%20", "", "%20a%20", " a ", new LinkedMultiValueMap<>()); + testPathSegment("%20%20", " ", new LinkedMultiValueMap<>()); + testPathSegment("%20a%20", " a ", new LinkedMultiValueMap<>()); } @Test @@ -56,30 +56,29 @@ public class DefaultPathContainerTests { params.add("colors", "blue"); params.add("colors", "green"); params.add("year", "2012"); - testPathSegment("cars", ";colors=red,blue,green;year=2012", "cars", "cars", params); + testPathSegment("cars;colors=red,blue,green;year=2012", "cars", params); // trailing semicolon params = new LinkedMultiValueMap<>(); params.add("p", "1"); - testPathSegment("path", ";p=1;", "path", "path", params); + testPathSegment("path;p=1;", "path", params); // params with spaces params = new LinkedMultiValueMap<>(); params.add("param name", "param value"); - testPathSegment("path", ";param%20name=param%20value;%20", "path", "path", params); + testPathSegment("path;param%20name=param%20value;%20", "path", params); // empty params params = new LinkedMultiValueMap<>(); params.add("p", "1"); - testPathSegment("path", ";;;%20;%20;p=1;%20", "path", "path", params); + testPathSegment("path;;;%20;%20;p=1;%20", "path", params); } - private void testPathSegment(String rawValue, String semicolonContent, - String value, String valueDecoded, MultiValueMap params) { + private void testPathSegment(String rawValue, String valueToMatch, MultiValueMap params) { - PathContainer container = DefaultPathContainer.parsePath(rawValue + semicolonContent, UTF_8); + PathContainer container = DefaultPathContainer.parsePath(rawValue, UTF_8); - if ("".equals(value)) { + if ("".equals(rawValue)) { assertEquals(0, container.elements().size()); return; } @@ -87,9 +86,8 @@ public class DefaultPathContainerTests { assertEquals(1, container.elements().size()); PathContainer.Segment segment = (PathContainer.Segment) container.elements().get(0); - assertEquals("value: '" + rawValue + "'", value, segment.value()); - assertEquals("valueDecoded: '" + rawValue + "'", valueDecoded, segment.valueDecoded()); - assertEquals("semicolonContent: '" + rawValue + "'", semicolonContent, segment.semicolonContent()); + assertEquals("value: '" + rawValue + "'", rawValue, segment.value()); + assertEquals("valueToMatch: '" + rawValue + "'", valueToMatch, segment.valueToMatch()); assertEquals("params: '" + rawValue + "'", params, segment.parameters()); }