diff --git a/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java b/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java index ddea35fc75..7d4571dfb8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,12 +36,11 @@ import org.springframework.util.StringUtils; * Default implementation of {@link PathContainer}. * * @author Rossen Stoyanchev + * @author Sam Brannen * @since 5.0 */ final class DefaultPathContainer implements PathContainer { - private static final MultiValueMap EMPTY_PARAMS = new LinkedMultiValueMap<>(); - private static final PathContainer EMPTY_PATH = new DefaultPathContainer("", Collections.emptyList()); private static final Map SEPARATORS = new HashMap<>(2); @@ -120,7 +119,7 @@ final class DefaultPathContainer implements PathContainer { if (!segment.isEmpty()) { elements.add(options.shouldDecodeAndParseSegments() ? decodeAndParsePathSegment(segment) : - new DefaultPathSegment(segment, separatorElement)); + DefaultPathSegment.from(segment, separatorElement)); } if (end == -1) { break; @@ -136,13 +135,13 @@ final class DefaultPathContainer implements PathContainer { int index = segment.indexOf(';'); if (index == -1) { String valueToMatch = StringUtils.uriDecode(segment, charset); - return new DefaultPathSegment(segment, valueToMatch, EMPTY_PARAMS); + return DefaultPathSegment.from(segment, valueToMatch); } else { String valueToMatch = StringUtils.uriDecode(segment.substring(0, index), charset); String pathParameterContent = segment.substring(index); MultiValueMap parameters = parsePathParams(pathParameterContent, charset); - return new DefaultPathSegment(segment, valueToMatch, parameters); + return DefaultPathSegment.from(segment, valueToMatch, parameters); } } @@ -226,7 +225,10 @@ final class DefaultPathContainer implements PathContainer { } - private static class DefaultPathSegment implements PathSegment { + private static final class DefaultPathSegment implements PathSegment { + + private static final MultiValueMap EMPTY_PARAMS = + CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>()); private final String value; @@ -234,24 +236,33 @@ final class DefaultPathContainer implements PathContainer { private final MultiValueMap parameters; - /** - * Constructor for decoded and parsed segments. + * Factory for segments without decoding and parsing. */ - DefaultPathSegment(String value, String valueToMatch, MultiValueMap params) { - this.value = value; - this.valueToMatch = valueToMatch; - this.parameters = CollectionUtils.unmodifiableMultiValueMap(params); + static DefaultPathSegment from(String value, DefaultSeparator separator) { + String valueToMatch = value.contains(separator.encodedSequence()) ? + value.replaceAll(separator.encodedSequence(), separator.value()) : value; + return from(value, valueToMatch); } /** - * Constructor for segments without decoding and parsing. + * Factory for decoded and parsed segments. */ - DefaultPathSegment(String value, DefaultSeparator separator) { + static DefaultPathSegment from(String value, String valueToMatch) { + return new DefaultPathSegment(value, valueToMatch, EMPTY_PARAMS); + } + + /** + * Factory for decoded and parsed segments. + */ + static DefaultPathSegment from(String value, String valueToMatch, MultiValueMap params) { + return new DefaultPathSegment(value, valueToMatch, CollectionUtils.unmodifiableMultiValueMap(params)); + } + + private DefaultPathSegment(String value, String valueToMatch, MultiValueMap params) { this.value = value; - this.valueToMatch = value.contains(separator.encodedSequence()) ? - value.replaceAll(separator.encodedSequence(), separator.value()) : value; - this.parameters = EMPTY_PARAMS; + this.valueToMatch = valueToMatch; + this.parameters = params; } diff --git a/spring-web/src/main/java/org/springframework/http/server/PathContainer.java b/spring-web/src/main/java/org/springframework/http/server/PathContainer.java index 9da528f52e..a125795e3d 100644 --- a/spring-web/src/main/java/org/springframework/http/server/PathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/PathContainer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -124,6 +124,7 @@ public interface PathContainer { /** * Path parameters associated with this path segment. + * @return an unmodifiable map containing the parameters */ MultiValueMap parameters(); } @@ -136,15 +137,16 @@ public interface PathContainer { class Options { /** - * Options for HTTP URL paths: - *

Separator '/' with URL decoding and parsing of path params. + * Options for HTTP URL paths. + *

Separator '/' with URL decoding and parsing of path parameters. */ public final static Options HTTP_PATH = Options.create('/', true); /** - * Options for a message route: - *

Separator '.' without URL decoding nor parsing of params. Escape - * sequences for the separator char in segment values are still decoded. + * Options for a message route. + *

Separator '.' with neither URL decoding nor parsing of path parameters. + * Escape sequences for the separator character in segment values are still + * decoded. */ public final static Options MESSAGE_ROUTE = Options.create('.', false); diff --git a/spring-web/src/test/java/org/springframework/http/server/DefaultPathContainerTests.java b/spring-web/src/test/java/org/springframework/http/server/DefaultPathContainerTests.java index cb78bfb377..2f434fd7c9 100644 --- a/spring-web/src/test/java/org/springframework/http/server/DefaultPathContainerTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/DefaultPathContainerTests.java @@ -20,11 +20,14 @@ import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.springframework.http.server.PathContainer.Element; +import org.springframework.http.server.PathContainer.Options; import org.springframework.http.server.PathContainer.PathSegment; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Unit tests for {@link DefaultPathContainer}. @@ -37,20 +40,20 @@ class DefaultPathContainerTests { @Test void pathSegment() { // basic - testPathSegment("cars", "cars", new LinkedMultiValueMap<>()); + testPathSegment("cars", "cars", emptyMap()); // empty - testPathSegment("", "", new LinkedMultiValueMap<>()); + testPathSegment("", "", emptyMap()); // spaces - testPathSegment("%20%20", " ", new LinkedMultiValueMap<>()); - testPathSegment("%20a%20", " a ", new LinkedMultiValueMap<>()); + testPathSegment("%20%20", " ", emptyMap()); + testPathSegment("%20a%20", " a ", emptyMap()); } @Test void pathSegmentParams() { // basic - LinkedMultiValueMap params = new LinkedMultiValueMap<>(); + LinkedMultiValueMap params = emptyMap(); params.add("colors", "red"); params.add("colors", "blue"); params.add("colors", "green"); @@ -58,21 +61,45 @@ class DefaultPathContainerTests { testPathSegment("cars;colors=red,blue,green;year=2012", "cars", params); // trailing semicolon - params = new LinkedMultiValueMap<>(); + params = emptyMap(); params.add("p", "1"); testPathSegment("path;p=1;", "path", params); // params with spaces - params = new LinkedMultiValueMap<>(); + params = emptyMap(); params.add("param name", "param value"); testPathSegment("path;param%20name=param%20value;%20", "path", params); // empty params - params = new LinkedMultiValueMap<>(); + params = emptyMap(); params.add("p", "1"); testPathSegment("path;;;%20;%20;p=1;%20", "path", params); } + @Test + void pathSegmentParamsAreImmutable() { + assertPathSegmentParamsAreImmutable("cars", emptyMap(), Options.HTTP_PATH); + + LinkedMultiValueMap params = emptyMap(); + params.add("colors", "red"); + params.add("colors", "blue"); + params.add("colors", "green"); + assertPathSegmentParamsAreImmutable(";colors=red,blue,green", params, Options.HTTP_PATH); + + assertPathSegmentParamsAreImmutable(";colors=red,blue,green", emptyMap(), Options.MESSAGE_ROUTE); + } + + private void assertPathSegmentParamsAreImmutable(String path, LinkedMultiValueMap params, Options options) { + PathContainer container = PathContainer.parsePath(path, options); + assertThat(container.elements()).hasSize(1); + + PathSegment segment = (PathSegment) container.elements().get(0); + MultiValueMap segmentParams = segment.parameters(); + assertThat(segmentParams).isEqualTo(params); + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> segmentParams.add("enigma", "boom")); + } + private void testPathSegment(String rawValue, String valueToMatch, MultiValueMap params) { PathContainer container = PathContainer.parsePath(rawValue); @@ -111,10 +138,10 @@ class DefaultPathContainerTests { } private void testPath(String input, String value, String... expectedElements) { - PathContainer path = PathContainer.parsePath(input, PathContainer.Options.HTTP_PATH); + PathContainer path = PathContainer.parsePath(input, Options.HTTP_PATH); assertThat(path.value()).as("value: '" + input + "'").isEqualTo(value); - assertThat(path.elements()).map(PathContainer.Element::value).as("elements: " + input) + assertThat(path.elements()).map(Element::value).as("elements: " + input) .containsExactly(expectedElements); } @@ -137,7 +164,7 @@ class DefaultPathContainerTests { @Test // gh-23310 void pathWithCustomSeparator() { - PathContainer path = PathContainer.parsePath("a.b%2Eb.c", PathContainer.Options.MESSAGE_ROUTE); + PathContainer path = PathContainer.parsePath("a.b%2Eb.c", Options.MESSAGE_ROUTE); Stream decodedSegments = path.elements().stream() .filter(PathSegment.class::isInstance) @@ -147,4 +174,8 @@ class DefaultPathContainerTests { assertThat(decodedSegments).containsExactly("a", "b.b", "c"); } + private static LinkedMultiValueMap emptyMap() { + return new LinkedMultiValueMap<>(); + } + }