From 9f7d57f933e84ea9f002ab5376fb12bcdb1120db Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 10 Jul 2017 14:55:01 +0200 Subject: [PATCH 1/5] Encapsulate choice of URI encoding within RequestPath Currently the URI path is always treated as UTF-8 so we can eliminate the encoding parameter from RequestPath#parse. --- .../reactive/function/server/MockServerRequest.java | 3 +-- .../server/reactive/AbstractServerHttpRequest.java | 2 +- .../http/server/reactive/DefaultRequestPath.java | 6 ++---- .../reactive/DefaultServerHttpRequestBuilder.java | 11 +++++------ .../http/server/reactive/RequestPath.java | 8 +++++--- .../springframework/web/util/pattern/PathPattern.java | 3 +-- .../http/server/reactive/DefaultRequestPathTests.java | 3 +-- .../reactive/function/server/MockServerRequest.java | 3 +-- 8 files changed, 17 insertions(+), 22 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/reactive/function/server/MockServerRequest.java b/spring-test/src/main/java/org/springframework/mock/web/reactive/function/server/MockServerRequest.java index ddcb5157f9..832a8da4f6 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/reactive/function/server/MockServerRequest.java +++ b/spring-test/src/main/java/org/springframework/mock/web/reactive/function/server/MockServerRequest.java @@ -19,7 +19,6 @@ package org.springframework.mock.web.reactive.function.server; import java.net.InetSocketAddress; import java.net.URI; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.security.Principal; import java.util.Arrays; import java.util.Collections; @@ -92,7 +91,7 @@ public class MockServerRequest implements ServerRequest { this.method = method; this.uri = uri; - this.pathContainer = RequestPath.create(uri, contextPath, StandardCharsets.UTF_8); + this.pathContainer = RequestPath.parse(uri, contextPath); this.headers = headers; this.cookies = cookies; this.body = body; diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java index 99de10d2d1..00e86b9c2a 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java @@ -61,7 +61,7 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { */ public AbstractServerHttpRequest(URI uri, String contextPath, HttpHeaders headers) { this.uri = uri; - this.path = new DefaultRequestPath(uri, contextPath, StandardCharsets.UTF_8); + this.path = RequestPath.parse(uri, contextPath); this.headers = HttpHeaders.readOnlyHttpHeaders(headers); } 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 08d9750496..3cf2d29f04 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 @@ -17,7 +17,6 @@ package org.springframework.http.server.reactive; import java.net.URI; -import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.List; @@ -40,8 +39,8 @@ class DefaultRequestPath implements RequestPath { private final PathContainer pathWithinApplication; - DefaultRequestPath(URI uri, @Nullable String contextPath, Charset charset) { - this.fullPath = PathContainer.parse(uri.getRawPath(), charset); + DefaultRequestPath(URI uri, @Nullable String contextPath) { + this.fullPath = PathContainer.parse(uri.getRawPath(), StandardCharsets.UTF_8); this.contextPath = initContextPath(this.fullPath, contextPath); this.pathWithinApplication = extractPathWithinApplication(this.fullPath, this.contextPath); } @@ -86,7 +85,6 @@ class DefaultRequestPath implements RequestPath { // PathContainer methods.. - @Override public String value() { return this.fullPath.value(); diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java index 43b51c98a0..3c5c24da20 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java @@ -18,7 +18,6 @@ package org.springframework.http.server.reactive; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -107,14 +106,14 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { @Nullable private RequestPath getRequestPathToUse(@Nullable URI uriToUse) { - if (uriToUse == null) { - if (this.contextPath == null) { - return null; - } + if (uriToUse == null && this.contextPath == null) { + return null; + } + else if (uriToUse == null) { return new DefaultRequestPath(this.delegate.getPath(), this.contextPath); } else { - return new DefaultRequestPath(uriToUse, this.contextPath, StandardCharsets.UTF_8); + return RequestPath.parse(uriToUse, this.contextPath); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/RequestPath.java b/spring-web/src/main/java/org/springframework/http/server/reactive/RequestPath.java index e63489e4f3..6c8c295b74 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/RequestPath.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/RequestPath.java @@ -16,7 +16,8 @@ package org.springframework.http.server.reactive; import java.net.URI; -import java.nio.charset.Charset; + +import org.springframework.lang.Nullable; /** * Represents the complete path for a request. @@ -41,11 +42,12 @@ public interface RequestPath extends PathContainer { */ PathContainer pathWithinApplication(); + /** * Create a new {@code RequestPath} with the given parameters. */ - static RequestPath create(URI uri, String contextPath, Charset charset) { - return new DefaultRequestPath(uri, contextPath, charset); + static RequestPath parse(URI uri, @Nullable String contextPath) { + return new DefaultRequestPath(uri, contextPath); } } 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 2a86dac8e8..3e103fa0cf 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 @@ -261,12 +261,11 @@ public class PathPattern implements Comparable { * @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) { + // TODO: implement extractPathWithinPattern for PathContainer String result = extractPathWithinPattern(path.value()); return PathContainer.parse(result, StandardCharsets.UTF_8); } - // TODO: implement extractPathWithinPattern natively for PathContainer - private String extractPathWithinPattern(String path) { // assert this.matches(path) PathElement elem = head; diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultRequestPathTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultRequestPathTests.java index 4bd8dcfd80..f1b42ee8be 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultRequestPathTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/DefaultRequestPathTests.java @@ -19,7 +19,6 @@ import java.net.URI; import org.junit.Test; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; /** @@ -54,7 +53,7 @@ public class DefaultRequestPathTests { private void testRequestPath(String fullPath, String contextPath, String pathWithinApplication) { URI uri = URI.create("http://localhost:8080" + fullPath); - RequestPath requestPath = new DefaultRequestPath(uri, contextPath, UTF_8); + RequestPath requestPath = RequestPath.parse(uri, contextPath); assertEquals(contextPath.equals("/") ? "" : contextPath, requestPath.contextPath().value()); assertEquals(pathWithinApplication, requestPath.pathWithinApplication().value()); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/MockServerRequest.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/MockServerRequest.java index fa3a67383c..644100fa1f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/MockServerRequest.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/MockServerRequest.java @@ -19,7 +19,6 @@ package org.springframework.web.reactive.function.server; import java.net.InetSocketAddress; import java.net.URI; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.security.Principal; import java.util.Arrays; import java.util.Collections; @@ -91,7 +90,7 @@ public class MockServerRequest implements ServerRequest { this.method = method; this.uri = uri; - this.pathContainer = RequestPath.create(uri, contextPath, StandardCharsets.UTF_8); + this.pathContainer = RequestPath.parse(uri, contextPath); this.headers = headers; this.cookies = cookies; this.body = body; From 2f17c5f3b61b40cee1abcb654fbba9d5e55f9890 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 10 Jul 2017 19:06:12 +0200 Subject: [PATCH 2/5] From static to instance-based PathContainer#subPath --- .../server/reactive/DefaultRequestPath.java | 4 +- .../http/server/reactive/PathContainer.java | 58 ++++++++++--------- .../web/util/pattern/PathPattern.java | 2 +- .../reactive/DefaultPathContainerTests.java | 10 ++-- .../resource/ResourceUrlProvider.java | 2 +- 5 files changed, 41 insertions(+), 35 deletions(-) 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 3cf2d29f04..2aadbbee86 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 @@ -69,7 +69,7 @@ class DefaultRequestPath implements RequestPath { counter += ((Segment) element).semicolonContent().length(); } if (length == counter) { - return DefaultPathContainer.subPath(path, 0, i + 1); + return path.subPath(0, i + 1); } } @@ -79,7 +79,7 @@ class DefaultRequestPath implements RequestPath { } private static PathContainer extractPathWithinApplication(PathContainer fullPath, PathContainer contextPath) { - return PathContainer.subPath(fullPath, contextPath.elements().size()); + return fullPath.subPath(contextPath.elements().size()); } 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 c6d00dc4eb..88045eed40 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 @@ -22,16 +22,21 @@ import java.util.List; import org.springframework.util.MultiValueMap; /** - * Structured path representation. + * Structured representation of a path whose {@link Element Elements} are + * accessible as a sequence of either {@link Separator Separator} and/or + * {@link Segment Segment} (element) types. * - *

Typically consumed via {@link ServerHttpRequest#getPath()} but can also - * be created by parsing a path value via {@link #parse(String, Charset)}. + *

Each {@code Segment} exposes its own structure decoded safely without the + * risk of encoded reserved characters altering the path or segment structure. + * + *

An instance of this class can also be created via + * {@link #parse(String, Charset)}. The path for an HTTP request is parsed once + * and subsequently accessible via {@link ServerHttpRequest#getPath()}. * * @author Rossen Stoyanchev */ public interface PathContainer { - /** * The original, raw (encoded) path value including path parameters. */ @@ -42,6 +47,26 @@ public interface PathContainer { */ List elements(); + /** + * Extract a sub-path from the given offset into the elements list. + * @param index the start element index (inclusive) + * @return the sub-path + */ + default PathContainer subPath(int index) { + return subPath(index, elements().size()); + } + + /** + * Extract a sub-path from the given start offset (inclusive) into the + * element list and to the end offset (exclusive). + * @param startIndex the start element index (inclusive) + * @param endIndex the end element index (exclusive) + * @return the sub-path + */ + default PathContainer subPath(int startIndex, int endIndex) { + return DefaultPathContainer.subPath(this, startIndex, endIndex); + } + /** * Parse the given path value into a {@link PathContainer}. @@ -53,29 +78,10 @@ public interface PathContainer { return DefaultPathContainer.parsePath(path, encoding); } - /** - * Extract a sub-path from the given offset into the path elements list. - * @param path the path to extract from - * @param index the start element index (inclusive) - * @return the sub-path - */ - static PathContainer subPath(PathContainer path, int index) { - return subPath(path, index, path.elements().size()); - } /** - * Extract a sub-path from the given start offset (inclusive) into the path - * element list and to the end offset (exclusive). - * @param path the path to extract from - * @param startIndex the start element index (inclusive) - * @param endIndex the end element index (exclusive) - * @return the sub-path + * Common representation of a path element, e.g. separator or segment. */ - static PathContainer subPath(PathContainer path, int startIndex, int endIndex) { - return DefaultPathContainer.subPath(path, startIndex, endIndex); - } - - interface Element { /** @@ -86,14 +92,14 @@ public interface PathContainer { /** - * A path separator element. + * Path separator element. */ interface Separator extends Element { } /** - * A path segment element. + * Path segment element. */ interface Segment extends Element { 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 3e103fa0cf..51ff839eee 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 @@ -205,7 +205,7 @@ public class PathPattern implements Comparable { info = new PathRemainingMatchInfo(EMPTY_PATH, matchingContext.getPathMatchResult()); } else { - info = new PathRemainingMatchInfo(PathContainer.subPath(pathContainer, matchingContext.remainingPathIndex), + info = new PathRemainingMatchInfo(pathContainer.subPath(matchingContext.remainingPathIndex), matchingContext.getPathMatchResult()); } return info; 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 3905478157..dab1a246ad 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 @@ -127,17 +127,17 @@ public class DefaultPathContainerTests { public void subPath() throws Exception { // basic PathContainer path = PathContainer.parse("/a/b/c", UTF_8); - assertSame(path, PathContainer.subPath(path, 0)); - assertEquals("/b/c", PathContainer.subPath(path, 2).value()); - assertEquals("/c", PathContainer.subPath(path, 4).value()); + assertSame(path, path.subPath(0)); + assertEquals("/b/c", path.subPath(2).value()); + assertEquals("/c", path.subPath(4).value()); // root path path = PathContainer.parse("/", UTF_8); - assertEquals("/", PathContainer.subPath(path, 0).value()); + assertEquals("/", path.subPath(0).value()); // trailing slash path = PathContainer.parse("/a/b/", UTF_8); - assertEquals("/b/", PathContainer.subPath(path, 2).value()); + assertEquals("/b/", path.subPath(2).value()); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 536d76901f..8542c0e336 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -215,7 +215,7 @@ public class ResourceUrlProvider implements ApplicationListener { PathContainer path = entry.getKey().extractPathWithinPattern(lookupPath); int endIndex = lookupPath.elements().size() - path.elements().size(); - PathContainer mapping = PathContainer.subPath(lookupPath, 0, endIndex); + PathContainer mapping = lookupPath.subPath(0, endIndex); if (logger.isTraceEnabled()) { logger.trace("Invoking ResourceResolverChain for URL pattern " + "\"" + entry.getKey() + "\""); From 1d201a55dbea44f6a7499c9203c3f598eeb1ffa2 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Jul 2017 06:35:59 +0200 Subject: [PATCH 3/5] 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 bc9b674802..a6b2cce48c 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 String valueToMatch; - private final char[] valueDecodedChars; - - 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 2aadbbee86..90a6436448 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 88045eed40..784cbef516 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 30a73ec6bb..d9ed318731 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 0a33534061..c1f1ca407f 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 51ff839eee..dc65fad876 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 95c6f58125..cc366b23d5 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 b3bf34b6a7..a9d49a393f 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 dab1a246ad..abcf19c8dd 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()); } From 0e370e07038da5d5d14185c30fd5dcbfc8b2309c Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Jul 2017 10:06:15 +0200 Subject: [PATCH 4/5] Separate out URL path parsing in PathContainer PathContainer now has separate parsePath and parseUrlPath methods each creating PathSegment and UrlPathSegment respectively. --- .../server/reactive/DefaultPathContainer.java | 88 +++++++++++++------ .../server/reactive/DefaultRequestPath.java | 5 +- .../http/server/reactive/PathContainer.java | 60 ++++++++----- .../pattern/CaptureTheRestPathElement.java | 10 +-- .../pattern/CaptureVariablePathElement.java | 4 +- .../web/util/pattern/LiteralPathElement.java | 9 +- .../web/util/pattern/ParsingPathMatcher.java | 9 +- .../web/util/pattern/PathPattern.java | 12 ++- .../web/util/pattern/RegexPathElement.java | 4 +- .../SingleCharWildcardedPathElement.java | 8 +- .../web/util/pattern/WildcardPathElement.java | 6 +- .../reactive/DefaultPathContainerTests.java | 14 +-- .../util/pattern/PathPatternMatcherTests.java | 5 +- .../function/server/ServerRequest.java | 4 +- .../resource/ResourceUrlProvider.java | 3 +- .../config/ResourceHandlerRegistryTests.java | 2 +- .../resource/ResourceUrlProviderTests.java | 7 +- .../resource/ResourceWebHandlerTests.java | 2 +- 18 files changed, 149 insertions(+), 103 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 a6b2cce48c..d30b51bed8 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 @@ -20,6 +20,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; import org.springframework.lang.Nullable; @@ -88,7 +89,11 @@ class DefaultPathContainer implements PathContainer { } - static PathContainer parsePath(String path, Charset charset) { + static PathContainer createFromPath(String path) { + return parsePathInternal(path, DefaultPathSegment::new); + } + + private static PathContainer parsePathInternal(String path, Function segmentParser) { if (path.equals("")) { return EMPTY_PATH; } @@ -105,7 +110,7 @@ class DefaultPathContainer implements PathContainer { int end = path.indexOf('/', begin); String segment = (end != -1 ? path.substring(begin, end) : path.substring(begin)); if (!segment.equals("")) { - elements.add(parsePathSegment(segment, charset)); + elements.add(segmentParser.apply(segment)); } if (end == -1) { break; @@ -116,27 +121,31 @@ class DefaultPathContainer implements PathContainer { return new DefaultPathContainer(path, elements); } - private static PathContainer.Segment parsePathSegment(String input, Charset charset) { + static PathContainer createFromUrlPath(String path, Charset charset) { + return parsePathInternal(path, segment -> parseUrlPathSegment(segment, charset)); + } + + private static PathContainer.UrlPathSegment parseUrlPathSegment(String input, Charset charset) { int index = input.indexOf(';'); if (index == -1) { String valueToMatch = StringUtils.uriDecode(input, charset); - return new DefaultPathSegment(input, valueToMatch, EMPTY_MAP); + return new DefaultUrlPathSegment(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); + MultiValueMap parameters = parsePathParams(pathParameterContent, charset); + return new DefaultUrlPathSegment(input, valueToMatch, parameters); } } - private static MultiValueMap parseParams(String input, Charset charset) { + private static MultiValueMap parsePathParams(String input, Charset charset) { MultiValueMap result = new LinkedMultiValueMap<>(); int begin = 1; while (begin < input.length()) { int end = input.indexOf(';', begin); String param = (end != -1 ? input.substring(begin, end) : input.substring(begin)); - parseParamValues(param, charset, result); + parsePathParamValues(param, charset, result); if (end == -1) { break; } @@ -145,7 +154,7 @@ class DefaultPathContainer implements PathContainer { return result; } - private static void parseParamValues(String input, Charset charset, MultiValueMap output) { + private static void parsePathParamValues(String input, Charset charset, MultiValueMap output) { if (StringUtils.hasText(input)) { int index = input.indexOf("="); if (index != -1) { @@ -186,24 +195,19 @@ class DefaultPathContainer implements PathContainer { } - private static class DefaultPathSegment implements PathContainer.Segment { + private static class DefaultPathSegment implements PathSegment { private final String value; - private final String valueToMatch; + private final char[] valueAsChars; - private final char[] valueToMatchAsChars; - private final MultiValueMap parameters; - - DefaultPathSegment(String value, String valueToMatch, MultiValueMap params) { - Assert.isTrue(!value.contains("/"), () -> "Invalid path segment value: " + value); + DefaultPathSegment(String value) { this.value = value; - this.valueToMatch = valueToMatch; - this.valueToMatchAsChars = valueToMatch.toCharArray(); - this.parameters = CollectionUtils.unmodifiableMultiValueMap(params); + this.valueAsChars = value.toCharArray(); } + @Override public String value() { return this.value; @@ -211,17 +215,12 @@ class DefaultPathContainer implements PathContainer { @Override public String valueToMatch() { - return this.valueToMatch; + return this.value; } @Override public char[] valueToMatchAsChars() { - return this.valueToMatchAsChars; - } - - @Override - public MultiValueMap parameters() { - return this.parameters; + return this.valueAsChars; } @Override @@ -241,7 +240,42 @@ class DefaultPathContainer implements PathContainer { } public String toString() { - return "[value='" + this.value + "', parameters=" + this.parameters + "']"; } + return "[value='" + this.value + "']"; } + } + + + private static class DefaultUrlPathSegment extends DefaultPathSegment implements UrlPathSegment { + + private final String valueToMatch; + + private final char[] valueToMatchAsChars; + + private final MultiValueMap parameters; + + + DefaultUrlPathSegment(String value, String valueToMatch, MultiValueMap params) { + super(value); + Assert.isTrue(!value.contains("/"), () -> "Invalid path segment value: " + value); + this.valueToMatch = valueToMatch; + this.valueToMatchAsChars = valueToMatch.toCharArray(); + this.parameters = CollectionUtils.unmodifiableMultiValueMap(params); + } + + + @Override + public String valueToMatch() { + return this.valueToMatch; + } + + @Override + public char[] valueToMatchAsChars() { + return this.valueToMatchAsChars; + } + + @Override + public MultiValueMap parameters() { + return 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 90a6436448..b2e79eb319 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 @@ -17,7 +17,6 @@ package org.springframework.http.server.reactive; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.util.List; import org.springframework.lang.Nullable; @@ -40,7 +39,7 @@ class DefaultRequestPath implements RequestPath { DefaultRequestPath(URI uri, @Nullable String contextPath) { - this.fullPath = PathContainer.parse(uri.getRawPath(), StandardCharsets.UTF_8); + this.fullPath = PathContainer.parseUrlPath(uri.getRawPath()); this.contextPath = initContextPath(this.fullPath, contextPath); this.pathWithinApplication = extractPathWithinApplication(this.fullPath, this.contextPath); } @@ -53,7 +52,7 @@ class DefaultRequestPath implements RequestPath { private static PathContainer initContextPath(PathContainer path, @Nullable String contextPath) { if (!StringUtils.hasText(contextPath) || "/".equals(contextPath)) { - return PathContainer.parse("", StandardCharsets.UTF_8); + return PathContainer.parseUrlPath(""); } Assert.isTrue(contextPath.startsWith("/") && !contextPath.endsWith("/") && 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 784cbef516..fd9356bb38 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 @@ -16,34 +16,36 @@ package org.springframework.http.server.reactive; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.List; import org.springframework.util.MultiValueMap; /** - * Structured representation of a path whose {@link Element Elements} are - * accessible as a sequence of either {@link Separator Separator} and/or - * {@link Segment Segment} (element) types. + * Structured representation of a path whose elements are parsed into a sequence + * of {@link Separator Separator} and {@link PathSegment PathSegment} elements. * - *

Each {@code Segment} exposes its own structure decoded safely without the - * risk of encoded reserved characters altering the path or segment structure. + *

An instance of this class can be created via {@link #parsePath(String)} or + * {@link #parseUrlPath(String)}. For an HTTP request the path can be + * accessed via {@link ServerHttpRequest#getPath()}. * - *

An instance of this class can also be created via - * {@link #parse(String, Charset)}. The path for an HTTP request is parsed once - * and subsequently accessible via {@link ServerHttpRequest#getPath()}. + *

For a URL path each {@link UrlPathSegment UrlPathSegment} exposes its + * structure decoded safely without the risk of encoded reserved characters + * altering the path or segment structure and without path parameters for + * path matching purposes. * * @author Rossen Stoyanchev + * @since 5.0 */ public interface PathContainer { /** - * The original, raw (encoded) path value including path parameters. + * The original path that was parsed. */ String value(); /** - * The list of path elements, either {@link Separator} or {@link Segment}. + * The list of path elements, either {@link Separator} or {@link PathSegment}. */ List elements(); @@ -69,13 +71,23 @@ public interface PathContainer { /** - * Parse the given path value into a {@link PathContainer}. - * @param path the encoded, raw path value to parse - * @param encoding the charset to use for decoded path segment values + * Parse the path value into a sequence of {@link Separator Separator} and + * {@link PathSegment PathSegment} elements. + * @param path the path value to parse * @return the parsed path */ - static PathContainer parse(String path, Charset encoding) { - return DefaultPathContainer.parsePath(path, encoding); + static PathContainer parsePath(String path) { + return DefaultPathContainer.createFromPath(path); + } + + /** + * Parse the path value into a sequence of {@link Separator Separator} and + * {@link UrlPathSegment UrlPathSegment} elements. + * @param path the encoded, raw URL path value to parse + * @return the parsed path + */ + static PathContainer parseUrlPath(String path) { + return DefaultPathContainer.createFromUrlPath(path, StandardCharsets.UTF_8); } @@ -101,19 +113,27 @@ public interface PathContainer { /** * Path segment element. */ - interface Segment extends Element { + interface PathSegment extends Element { /** * 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. + * By default this is the same as {@link #value()} but may also differ + * in sub-interfaces (e.g. decoded, sanitized, etc.). */ String valueToMatch(); /** - * Variant of {@link #valueToMatch()} as a {@code char[]}. + * The same as {@link #valueToMatch()} but as a {@code char[]}. */ char[] valueToMatchAsChars(); + } + + + /** + * Specialization of {@link PathSegment} for a URL path. + * The {@link #valueToMatch()} is decoded and without path parameters. + */ + interface UrlPathSegment extends PathSegment { /** * 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 d9ed318731..9f95833afb 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 @@ -19,7 +19,7 @@ package org.springframework.web.util.pattern; import java.util.List; import org.springframework.http.server.reactive.PathContainer.Element; -import org.springframework.http.server.reactive.PathContainer.Segment; +import org.springframework.http.server.reactive.PathContainer.UrlPathSegment; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.util.pattern.PathPattern.MatchingContext; @@ -65,8 +65,8 @@ class CaptureTheRestPathElement extends PathElement { MultiValueMap parametersCollector = null; for (int i = pathIndex; i < matchingContext.pathLength; i++) { Element element = matchingContext.pathElements.get(i); - if (element instanceof Segment) { - MultiValueMap parameters = ((Segment) element).parameters(); + if (element instanceof UrlPathSegment) { + MultiValueMap parameters = ((UrlPathSegment) element).parameters(); if (!parameters.isEmpty()) { if (parametersCollector == null) { parametersCollector = new LinkedMultiValueMap<>(); @@ -85,8 +85,8 @@ class CaptureTheRestPathElement extends PathElement { StringBuilder buf = new StringBuilder(); for (int i = fromSegment, max = pathElements.size(); i < max; i++) { Element element = pathElements.get(i); - if (element instanceof Segment) { - buf.append(((Segment)element).valueToMatch()); + if (element instanceof UrlPathSegment) { + buf.append(((UrlPathSegment)element).valueToMatch()); } else { buf.append(element.value()); diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java index 018fda6dcf..e759ca07e8 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java @@ -19,7 +19,7 @@ package org.springframework.web.util.pattern; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.springframework.http.server.reactive.PathContainer.Segment; +import org.springframework.http.server.reactive.PathContainer.UrlPathSegment; import org.springframework.lang.Nullable; /** @@ -120,7 +120,7 @@ class CaptureVariablePathElement extends PathElement { } if (match && matchingContext.extractingVariables) { - matchingContext.set(this.variableName, candidateCapture, ((Segment)matchingContext.pathElements.get(pathIndex-1)).parameters()); + matchingContext.set(this.variableName, candidateCapture, ((UrlPathSegment)matchingContext.pathElements.get(pathIndex-1)).parameters()); } return match; } 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 c1f1ca407f..5c897c41ae 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 @@ -16,8 +16,9 @@ package org.springframework.web.util.pattern; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.http.server.reactive.PathContainer.Element; -import org.springframework.http.server.reactive.PathContainer.Segment; +import org.springframework.http.server.reactive.PathContainer.PathSegment; import org.springframework.web.util.pattern.PathPattern.MatchingContext; /** @@ -59,16 +60,16 @@ class LiteralPathElement extends PathElement { return false; } Element element = matchingContext.pathElements.get(pathIndex); - if (!(element instanceof Segment)) { + if (!(element instanceof PathContainer.PathSegment)) { return false; } - String value = ((Segment)element).valueToMatch(); + String value = ((PathSegment)element).valueToMatch(); if (value.length() != len) { // Not enough data to match this path element return false; } - char[] data = ((Segment)element).valueToMatchAsChars(); + char[] data = ((PathContainer.PathSegment)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/ParsingPathMatcher.java b/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java index f044888091..4fa8d4730a 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java @@ -16,7 +16,6 @@ package org.springframework.web.util.pattern; -import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Comparator; import java.util.Map; @@ -61,26 +60,26 @@ public class ParsingPathMatcher implements PathMatcher { @Override public boolean match(String pattern, String path) { PathPattern pathPattern = getPathPattern(pattern); - return pathPattern.matches(PathContainer.parse(path, StandardCharsets.UTF_8)); + return pathPattern.matches(PathContainer.parseUrlPath(path)); } @Override public boolean matchStart(String pattern, String path) { PathPattern pathPattern = getPathPattern(pattern); - return pathPattern.matchStart(PathContainer.parse(path, StandardCharsets.UTF_8)); + return pathPattern.matchStart(PathContainer.parseUrlPath(path)); } @Override public String extractPathWithinPattern(String pattern, String path) { PathPattern pathPattern = getPathPattern(pattern); - PathContainer pathContainer = PathContainer.parse(path, StandardCharsets.UTF_8); + PathContainer pathContainer = PathContainer.parseUrlPath(path); return pathPattern.extractPathWithinPattern(pathContainer).value(); } @Override public Map extractUriTemplateVariables(String pattern, String path) { PathPattern pathPattern = getPathPattern(pattern); - PathContainer pathContainer = PathContainer.parse(path, StandardCharsets.UTF_8); + PathContainer pathContainer = PathContainer.parseUrlPath(path); PathMatchResult results = pathPattern.matchAndExtract(pathContainer); // Collapse PathMatchResults to simple value results // TODO: (path parameters are lost in this translation) 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 dc65fad876..414bb146e5 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 @@ -16,7 +16,6 @@ package org.springframework.web.util.pattern; -import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -24,7 +23,6 @@ import java.util.Map; import org.springframework.http.server.reactive.PathContainer; import org.springframework.http.server.reactive.PathContainer.Element; -import org.springframework.http.server.reactive.PathContainer.Segment; import org.springframework.http.server.reactive.PathContainer.Separator; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; @@ -71,7 +69,7 @@ import org.springframework.util.StringUtils; */ public class PathPattern implements Comparable { - private final static PathContainer EMPTY_PATH = PathContainer.parse("", StandardCharsets.UTF_8); + private final static PathContainer EMPTY_PATH = PathContainer.parsePath(""); /** The parser used to construct this pattern */ private final PathPatternParser parser; @@ -263,7 +261,7 @@ public class PathPattern implements Comparable { public PathContainer extractPathWithinPattern(PathContainer path) { // TODO: implement extractPathWithinPattern for PathContainer String result = extractPathWithinPattern(path.value()); - return PathContainer.parse(result, StandardCharsets.UTF_8); + return PathContainer.parseUrlPath(result); } private String extractPathWithinPattern(String path) { @@ -405,7 +403,7 @@ public class PathPattern implements Comparable { // /usr + /user => /usr/user // /{foo} + /bar => /{foo}/bar if (!this.patternString.equals(pattern2string.patternString) && this.capturedVariableCount == 0 && - matches(PathContainer.parse(pattern2string.patternString, StandardCharsets.UTF_8))) { + matches(PathContainer.parseUrlPath(pattern2string.patternString))) { return pattern2string; } @@ -682,8 +680,8 @@ public class PathPattern implements Comparable { */ String pathElementValue(int pathIndex) { Element element = (pathIndex < pathLength) ? pathElements.get(pathIndex) : null; - if (element instanceof Segment) { - return ((Segment)element).valueToMatch(); + if (element instanceof PathContainer.PathSegment) { + return ((PathContainer.PathSegment)element).valueToMatch(); } return ""; } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java index d7743339ff..e916905476 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.springframework.http.server.reactive.PathContainer.Segment; +import org.springframework.http.server.reactive.PathContainer.UrlPathSegment; import org.springframework.web.util.pattern.PathPattern.MatchingContext; /** @@ -173,7 +173,7 @@ class RegexPathElement extends PathElement { String value = matcher.group(i); matchingContext.set(name, value, (i == this.variableNames.size())? - ((Segment)matchingContext.pathElements.get(pathIndex)).parameters(): + ((UrlPathSegment)matchingContext.pathElements.get(pathIndex)).parameters(): NO_PARAMETERS); } } 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 cc366b23d5..b435f8abaf 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 @@ -17,7 +17,7 @@ package org.springframework.web.util.pattern; import org.springframework.http.server.reactive.PathContainer.Element; -import org.springframework.http.server.reactive.PathContainer.Segment; +import org.springframework.http.server.reactive.PathContainer.PathSegment; import org.springframework.web.util.pattern.PathPattern.MatchingContext; /** @@ -65,16 +65,16 @@ class SingleCharWildcardedPathElement extends PathElement { } Element element = matchingContext.pathElements.get(pathIndex); - if (!(element instanceof Segment)) { + if (!(element instanceof PathSegment)) { return false; } - String value = ((Segment)element).valueToMatch(); + String value = ((PathSegment)element).valueToMatch(); if (value.length() != len) { // Not enough data to match this path element return false; } - char[] data = ((Segment)element).valueToMatchAsChars(); + char[] data = ((PathSegment)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 a9d49a393f..c25bd4af15 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 @@ -16,8 +16,8 @@ package org.springframework.web.util.pattern; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.http.server.reactive.PathContainer.Element; -import org.springframework.http.server.reactive.PathContainer.Segment; import org.springframework.web.util.pattern.PathPattern.MatchingContext; /** @@ -46,11 +46,11 @@ class WildcardPathElement extends PathElement { // Assert if it exists it is a segment if (pathIndex < matchingContext.pathLength) { Element element = matchingContext.pathElements.get(pathIndex); - if (!(element instanceof Segment)) { + if (!(element instanceof PathContainer.PathSegment)) { // Should not match a separator return false; } - segmentData = ((Segment)element).valueToMatch(); + segmentData = ((PathContainer.PathSegment)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 abcf19c8dd..748a45a4b5 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 @@ -22,10 +22,10 @@ import java.util.stream.Collectors; import org.junit.Test; +import org.springframework.http.server.reactive.PathContainer.UrlPathSegment; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; @@ -76,7 +76,7 @@ public class DefaultPathContainerTests { private void testPathSegment(String rawValue, String valueToMatch, MultiValueMap params) { - PathContainer container = DefaultPathContainer.parsePath(rawValue, UTF_8); + PathContainer container = PathContainer.parseUrlPath(rawValue); if ("".equals(rawValue)) { assertEquals(0, container.elements().size()); @@ -84,7 +84,7 @@ public class DefaultPathContainerTests { } assertEquals(1, container.elements().size()); - PathContainer.Segment segment = (PathContainer.Segment) container.elements().get(0); + UrlPathSegment segment = (UrlPathSegment) container.elements().get(0); assertEquals("value: '" + rawValue + "'", rawValue, segment.value()); assertEquals("valueToMatch: '" + rawValue + "'", valueToMatch, segment.valueToMatch()); @@ -114,7 +114,7 @@ public class DefaultPathContainerTests { private void testPath(String input, String value, List expectedElements) { - PathContainer path = PathContainer.parse(input, UTF_8); + PathContainer path = PathContainer.parseUrlPath(input); assertEquals("value: '" + input + "'", value, path.value()); assertEquals("elements: " + input, expectedElements, path.elements().stream() @@ -124,17 +124,17 @@ public class DefaultPathContainerTests { @Test public void subPath() throws Exception { // basic - PathContainer path = PathContainer.parse("/a/b/c", UTF_8); + PathContainer path = PathContainer.parseUrlPath("/a/b/c"); assertSame(path, path.subPath(0)); assertEquals("/b/c", path.subPath(2).value()); assertEquals("/c", path.subPath(4).value()); // root path - path = PathContainer.parse("/", UTF_8); + path = PathContainer.parseUrlPath("/"); assertEquals("/", path.subPath(0).value()); // trailing slash - path = PathContainer.parse("/a/b/", UTF_8); + path = PathContainer.parseUrlPath("/a/b/"); assertEquals("/b/", path.subPath(2).value()); } diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java index c425bff60b..3e37945298 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java @@ -16,7 +16,6 @@ package org.springframework.web.util.pattern; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -587,7 +586,7 @@ public class PathPatternMatcherTests { PathPatternParser ppp = new PathPatternParser(); ppp.setMatchOptionalTrailingSlash(false); PathPattern pp = ppp.parse("test"); - assertFalse(pp.matchStart(PathContainer.parse("test/",StandardCharsets.UTF_8))); + assertFalse(pp.matchStart(PathContainer.parsePath("test/"))); checkStartNoMatch("test/*/","test//"); checkStartMatches("test/*","test/abc"); @@ -1317,7 +1316,7 @@ public class PathPatternMatcherTests { if (path == null) { return null; } - return PathContainer.parse(path, StandardCharsets.UTF_8); + return PathContainer.parseUrlPath(path); } private void checkMatches(String uriTemplate, String path) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java index 7a10f0da9a..3ccd3c8e89 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java @@ -19,9 +19,7 @@ package org.springframework.web.reactive.function.server; import java.net.InetSocketAddress; import java.net.URI; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.security.Principal; -import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; @@ -80,7 +78,7 @@ public interface ServerRequest { * Return the request path as {@code PathContainer}. */ default PathContainer pathContainer() { - return PathContainer.parse(path(), StandardCharsets.UTF_8); + return PathContainer.parseUrlPath(path()); } /** diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 8542c0e336..883fbad9c0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -16,7 +16,6 @@ package org.springframework.web.reactive.resource; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -174,7 +173,7 @@ public class ResourceUrlProvider implements ApplicationListener request.getPath().contextPath().value() + resolvedPath + query); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java index 6b68f8b6ad..30babe29f3 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java @@ -83,7 +83,7 @@ public class ResourceHandlerRegistryTests { public void mapPathToLocation() throws Exception { MockServerWebExchange exchange = MockServerHttpRequest.get("").toExchange(); exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, - PathContainer.parse("/testStylesheet.css", StandardCharsets.UTF_8)); + PathContainer.parsePath("/testStylesheet.css")); ResourceWebHandler handler = getHandler("/resources/**"); handler.handle(exchange).block(Duration.ofSeconds(5)); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java index 9465b0c5c1..7211abcaa4 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java @@ -16,7 +16,6 @@ package org.springframework.web.reactive.resource; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; @@ -75,7 +74,7 @@ public class ResourceUrlProviderTests { @Test public void getStaticResourceUrl() { - PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + PathContainer path = PathContainer.parsePath("/resources/foo.css"); String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo.css", url); } @@ -105,7 +104,7 @@ public class ResourceUrlProviderTests { resolvers.add(new PathResourceResolver()); this.handler.setResourceResolvers(resolvers); - PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + PathContainer path = PathContainer.parsePath("/resources/foo.css"); String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); } @@ -127,7 +126,7 @@ public class ResourceUrlProviderTests { this.handlerMap.put("/resources/*.css", otherHandler); this.urlProvider.registerHandlers(this.handlerMap); - PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + PathContainer path = PathContainer.parsePath("/resources/foo.css"); String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index ca5a064824..9139a4690f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -553,7 +553,7 @@ public class ResourceWebHandlerTests { private void setPathWithinHandlerMapping(ServerWebExchange exchange, String path) { exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, - PathContainer.parse(path, StandardCharsets.UTF_8)); + PathContainer.parsePath(path)); } private long resourceLastModified(String resourceName) throws IOException { From c93792b8c9e5ecea45c739cf8bc3df40ec3e297a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Jul 2017 10:51:05 +0200 Subject: [PATCH 5/5] Allow custom separator in PathContainer --- .../server/reactive/DefaultPathContainer.java | 53 ++++++++++--------- .../http/server/reactive/PathContainer.java | 16 ++++-- 2 files changed, 41 insertions(+), 28 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 d30b51bed8..149557108b 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 @@ -17,6 +17,7 @@ package org.springframework.http.server.reactive; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -89,25 +90,28 @@ class DefaultPathContainer implements PathContainer { } - static PathContainer createFromPath(String path) { - return parsePathInternal(path, DefaultPathSegment::new); + static PathContainer createFromPath(String path, String separator) { + return parsePathInternal(path, separator, DefaultPathSegment::new); } - private static PathContainer parsePathInternal(String path, Function segmentParser) { + private static PathContainer parsePathInternal(String path, String separator, + Function segmentParser) { + if (path.equals("")) { return EMPTY_PATH; } + Separator separatorElement = separator.equals(SEPARATOR.value()) ? SEPARATOR : () -> separator; List elements = new ArrayList<>(); int begin; - if (path.length() > 0 && path.charAt(0) == '/') { - begin = 1; - elements.add(SEPARATOR); + if (path.length() > 0 && path.startsWith(separator)) { + begin = separator.length(); + elements.add(separatorElement); } else { begin = 0; } while (begin < path.length()) { - int end = path.indexOf('/', begin); + int end = path.indexOf(separator, begin); String segment = (end != -1 ? path.substring(begin, end) : path.substring(begin)); if (!segment.equals("")) { elements.add(segmentParser.apply(segment)); @@ -115,28 +119,27 @@ class DefaultPathContainer implements PathContainer { if (end == -1) { break; } - elements.add(SEPARATOR); - begin = end + 1; + elements.add(separatorElement); + begin = end + separator.length(); } return new DefaultPathContainer(path, elements); } - static PathContainer createFromUrlPath(String path, Charset charset) { - return parsePathInternal(path, segment -> parseUrlPathSegment(segment, charset)); - } - - private static PathContainer.UrlPathSegment parseUrlPathSegment(String input, Charset charset) { - int index = input.indexOf(';'); - if (index == -1) { - String valueToMatch = StringUtils.uriDecode(input, charset); - return new DefaultUrlPathSegment(input, valueToMatch, EMPTY_MAP); - } - else { - String valueToMatch = StringUtils.uriDecode(input.substring(0, index), charset); - String pathParameterContent = input.substring(index); - MultiValueMap parameters = parsePathParams(pathParameterContent, charset); - return new DefaultUrlPathSegment(input, valueToMatch, parameters); - } + static PathContainer createFromUrlPath(String path) { + return parsePathInternal(path, "/", segment -> { + Charset charset = StandardCharsets.UTF_8; + int index = segment.indexOf(';'); + if (index == -1) { + String valueToMatch = StringUtils.uriDecode(segment, charset); + return new DefaultUrlPathSegment(segment, valueToMatch, EMPTY_MAP); + } + else { + String valueToMatch = StringUtils.uriDecode(segment.substring(0, index), charset); + String pathParameterContent = segment.substring(index); + MultiValueMap parameters = parsePathParams(pathParameterContent, charset); + return new DefaultUrlPathSegment(segment, valueToMatch, parameters); + } + }); } private static MultiValueMap parsePathParams(String input, Charset charset) { 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 fd9356bb38..fa38d78a1a 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 @@ -16,7 +16,6 @@ package org.springframework.http.server.reactive; -import java.nio.charset.StandardCharsets; import java.util.List; import org.springframework.util.MultiValueMap; @@ -77,7 +76,18 @@ public interface PathContainer { * @return the parsed path */ static PathContainer parsePath(String path) { - return DefaultPathContainer.createFromPath(path); + return parsePath(path, "/"); + } + + /** + * Parse the path value into a sequence of {@link Separator Separator} and + * {@link PathSegment PathSegment} elements. + * @param path the path value to parse + * @param separator the value to treat as separator + * @return the parsed path + */ + static PathContainer parsePath(String path, String separator) { + return DefaultPathContainer.createFromPath(path, separator); } /** @@ -87,7 +97,7 @@ public interface PathContainer { * @return the parsed path */ static PathContainer parseUrlPath(String path) { - return DefaultPathContainer.createFromUrlPath(path, StandardCharsets.UTF_8); + return DefaultPathContainer.createFromUrlPath(path); }