From 742feddaf93ce3489bb82340dacdb9f621c3f3f2 Mon Sep 17 00:00:00 2001 From: Fabrice Bibonne Date: Fri, 17 Oct 2025 16:35:12 +0200 Subject: [PATCH 1/4] feat (Uri components builder expanded) query params key can be duplicated #34788 - Use List instead of MultiValueMap in HierarchicalUriComponents to record query params Signed-off-by: Fabrice Bibonne --- .../web/util/HierarchicalUriComponents.java | 219 ++++++++---------- .../web/util/UriComponentsBuilder.java | 19 +- .../web/util/UriComponentsBuilderTests.java | 63 ++++- 3 files changed, 165 insertions(+), 136 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java index 83a564638052..f1e8e74daf35 100644 --- a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java +++ b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java @@ -16,29 +16,21 @@ package org.springframework.web.util; +import org.jspecify.annotations.Nullable; +import org.springframework.util.*; + import java.io.ByteArrayOutputStream; import java.io.Serializable; import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Objects; -import java.util.StringJoiner; +import java.util.*; import java.util.function.BiFunction; import java.util.function.UnaryOperator; +import java.util.stream.Collectors; -import org.jspecify.annotations.Nullable; - -import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; -import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MultiValueMap; -import org.springframework.util.ObjectUtils; -import org.springframework.util.StreamUtils; -import org.springframework.util.StringUtils; +import static java.util.stream.Collectors.mapping; +import static java.util.stream.Collectors.toList; /** * Extension of {@link UriComponents} for hierarchical URIs. @@ -48,8 +40,8 @@ * @author Rossen Stoyanchev * @author Phillip Webb * @author Sam Brannen - * @since 3.1.3 * @see Hierarchical URIs + * @since 3.1.3 */ @SuppressWarnings("serial") final class HierarchicalUriComponents extends UriComponents { @@ -58,8 +50,7 @@ final class HierarchicalUriComponents extends UriComponents { private static final String PATH_DELIMITER_STRING = String.valueOf(PATH_DELIMITER); - private static final MultiValueMap EMPTY_QUERY_PARAMS = - CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>()); + private static final List EMPTY_QUERY_PARAMS = List.of(); /** @@ -70,28 +61,35 @@ final class HierarchicalUriComponents extends UriComponents { public String getPath() { return ""; } + @Override public List getPathSegments() { return Collections.emptyList(); } + @Override public PathComponent encode(BiFunction encoder) { return this; } + @Override public void verify() { } + @Override public PathComponent expand(UriTemplateVariables uriVariables, @Nullable UnaryOperator encoder) { return this; } + @Override public void copyToUriComponentsBuilder(UriComponentsBuilder builder) { } + @Override public boolean equals(@Nullable Object other) { return (this == other); } + @Override public int hashCode() { return getClass().hashCode(); @@ -107,7 +105,7 @@ public int hashCode() { private final PathComponent path; - private final MultiValueMap queryParams; + private final List queryParams; private final EncodeState encodeState; @@ -116,18 +114,19 @@ public int hashCode() { /** * Package-private constructor. All arguments are optional, and can be {@code null}. - * @param scheme the scheme + * + * @param scheme the scheme * @param userInfo the user info - * @param host the host - * @param port the port - * @param path the path - * @param query the query parameters + * @param host the host + * @param port the port + * @param path the path + * @param query the query parameters * @param fragment the fragment - * @param encoded whether the components are already encoded + * @param encoded whether the components are already encoded */ HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, @Nullable String userInfo, - @Nullable String host, @Nullable String port, @Nullable PathComponent path, - @Nullable MultiValueMap query, boolean encoded) { + @Nullable String host, @Nullable String port, @Nullable PathComponent path, + @Nullable List query, boolean encoded) { super(scheme, fragment); @@ -135,7 +134,7 @@ public int hashCode() { this.host = host; this.port = port; this.path = path != null ? path : NULL_PATH_COMPONENT; - this.queryParams = query != null ? CollectionUtils.unmodifiableMultiValueMap(query) : EMPTY_QUERY_PARAMS; + this.queryParams = query != null ? Collections.unmodifiableList(query) : EMPTY_QUERY_PARAMS; this.encodeState = encoded ? EncodeState.FULLY_ENCODED : EncodeState.RAW; // Check for illegal characters.. @@ -145,9 +144,9 @@ public int hashCode() { } private HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, - @Nullable String userInfo, @Nullable String host, @Nullable String port, - PathComponent path, MultiValueMap queryParams, - EncodeState encodeState, @Nullable UnaryOperator variableEncoder) { + @Nullable String userInfo, @Nullable String host, @Nullable String port, + PathComponent path, List queryParams, + EncodeState encodeState, @Nullable UnaryOperator variableEncoder) { super(scheme, fragment); @@ -182,15 +181,13 @@ private HierarchicalUriComponents(@Nullable String scheme, @Nullable String frag public int getPort() { if (this.port == null) { return -1; - } - else if (this.port.contains("{")) { + } else if (this.port.contains("{")) { throw new IllegalStateException( "The port contains a URI variable but has not been expanded yet: " + this.port); } try { return Integer.parseInt(this.port); - } - catch (NumberFormatException ex) { + } catch (NumberFormatException ex) { throw new IllegalStateException("The port must be an integer: " + this.port); } } @@ -208,29 +205,10 @@ public List getPathSegments() { @Override public @Nullable String getQuery() { if (!this.queryParams.isEmpty()) { - StringBuilder queryBuilder = new StringBuilder(); - this.queryParams.forEach((name, values) -> { - if (CollectionUtils.isEmpty(values)) { - if (queryBuilder.length() != 0) { - queryBuilder.append('&'); - } - queryBuilder.append(name); - } - else { - for (Object value : values) { - if (queryBuilder.length() != 0) { - queryBuilder.append('&'); - } - queryBuilder.append(name); - if (value != null) { - queryBuilder.append('=').append(value.toString()); - } - } - } - }); - return queryBuilder.toString(); - } - else { + return this.queryParams.stream() + .map(QueryParam::toUriString) + .collect(Collectors.joining("&")); + } else { return null; } } @@ -240,7 +218,9 @@ public List getPathSegments() { */ @Override public MultiValueMap getQueryParams() { - return this.queryParams; + Map> collected = this.queryParams.stream().collect(Collectors.groupingBy(QueryParam::name, + mapping(QueryParam::value, toList()))); + return MultiValueMap.fromMultiValue(collected); } @@ -265,7 +245,7 @@ HierarchicalUriComponents encodeTemplate(Charset charset) { String userInfoTo = (getUserInfo() != null ? encoder.apply(getUserInfo(), Type.USER_INFO) : null); String hostTo = (getHost() != null ? encoder.apply(getHost(), getHostType()) : null); PathComponent pathTo = this.path.encode(encoder); - MultiValueMap queryParamsTo = encodeQueryParams(encoder); + List queryParamsTo = encodeQueryParams(encoder); return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo, hostTo, this.port, pathTo, queryParamsTo, EncodeState.TEMPLATE_ENCODED, this.variableEncoder); @@ -284,32 +264,23 @@ public HierarchicalUriComponents encode(Charset charset) { String hostTo = (this.host != null ? encodeUriComponent(this.host, charset, getHostType()) : null); BiFunction encoder = (s, type) -> encodeUriComponent(s, charset, type); PathComponent pathTo = this.path.encode(encoder); - MultiValueMap queryParamsTo = encodeQueryParams(encoder); + List queryParamsTo = encodeQueryParams(encoder); return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo, hostTo, this.port, pathTo, queryParamsTo, EncodeState.FULLY_ENCODED, null); } - private MultiValueMap encodeQueryParams(BiFunction encoder) { - int size = this.queryParams.size(); - MultiValueMap result = new LinkedMultiValueMap<>(size); - this.queryParams.forEach((key, values) -> { - String name = encoder.apply(key, Type.QUERY_PARAM); - List encodedValues = new ArrayList<>(values.size()); - for (String value : values) { - encodedValues.add(value != null ? encoder.apply(value, Type.QUERY_PARAM) : null); - } - result.put(name, encodedValues); - }); - return CollectionUtils.unmodifiableMultiValueMap(result); + private List encodeQueryParams(BiFunction encoder) { + return this.queryParams.stream().map(q -> q.encodeQueryParam(encoder)).toList(); } /** * Encode the given source into an encoded String using the rules specified * by the given component and with the given options. - * @param source the source String + * + * @param source the source String * @param encoding the encoding of the source String - * @param type the URI component for the source + * @param type the URI component for the source * @return the encoded URI * @throws IllegalArgumentException when the given value is not a valid URI component */ @@ -320,9 +291,10 @@ static String encodeUriComponent(String source, String encoding, Type type) { /** * Encode the given source into an encoded String using the rules specified * by the given component and with the given options. - * @param source the source String + * + * @param source the source String * @param charset the encoding of the source String - * @param type the URI component for the source + * @param type the URI component for the source * @return the encoded URI * @throws IllegalArgumentException when the given value is not a valid URI component */ @@ -349,8 +321,7 @@ static String encodeUriComponent(String source, Charset charset, Type type) { for (byte b : bytes) { if (type.isAllowed(b)) { baos.write(b); - } - else { + } else { baos.write('%'); char hex1 = Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16)); char hex2 = Character.toUpperCase(Character.forDigit(b & 0xF, 16)); @@ -369,6 +340,7 @@ private Type getHostType() { /** * Check if any of the URI components contain any illegal characters. + * * @throws IllegalArgumentException if any component has illegal characters */ private void verify() { @@ -376,12 +348,7 @@ private void verify() { verifyUriComponent(this.userInfo, Type.USER_INFO); verifyUriComponent(this.host, getHostType()); this.path.verify(); - this.queryParams.forEach((key, values) -> { - verifyUriComponent(key, Type.QUERY_PARAM); - for (String value : values) { - verifyUriComponent(value, Type.QUERY_PARAM); - } - }); + this.queryParams.forEach(QueryParam::verifyUriComponent); verifyUriComponent(getFragment(), Type.FRAGMENT); } @@ -403,13 +370,11 @@ private static void verifyUriComponent(@Nullable String source, Type type) { source.substring(i) + "\""); } i += 2; - } - else { + } else { throw new IllegalArgumentException("Invalid encoded sequence \"" + source.substring(i) + "\""); } - } - else if (!type.isAllowed(ch)) { + } else if (!type.isAllowed(ch)) { throw new IllegalArgumentException("Invalid character '" + ch + "' for " + type.name() + " in \"" + source + "\""); } @@ -430,25 +395,18 @@ protected HierarchicalUriComponents expandInternal(UriTemplateVariables uriVaria String hostTo = expandUriComponent(this.host, uriVariables, this.variableEncoder); String portTo = expandUriComponent(this.port, uriVariables, this.variableEncoder); PathComponent pathTo = this.path.expand(uriVariables, this.variableEncoder); - MultiValueMap queryParamsTo = expandQueryParams(uriVariables); + List queryParamsTo = expandQueryParams(uriVariables); String fragmentTo = expandUriComponent(getFragment(), uriVariables, this.variableEncoder); return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo, hostTo, portTo, pathTo, queryParamsTo, this.encodeState, this.variableEncoder); } - private MultiValueMap expandQueryParams(UriTemplateVariables variables) { - int size = this.queryParams.size(); - MultiValueMap result = new LinkedMultiValueMap<>(size); + private List expandQueryParams(UriTemplateVariables variables) { UriTemplateVariables queryVariables = new QueryUriTemplateVariables(variables); - this.queryParams.forEach((key, values) -> { - String name = expandUriComponent(key, queryVariables, this.variableEncoder); - List expandedValues = result.computeIfAbsent(name, k -> new ArrayList<>(values.size())); - for (String value : values) { - expandedValues.add(expandUriComponent(value, queryVariables, this.variableEncoder)); - } - }); - return CollectionUtils.unmodifiableMultiValueMap(result); + return this.queryParams.stream() + .map(qp->qp.expandUriComponent(queryVariables, this.variableEncoder)) + .toList(); } @Override @@ -502,8 +460,7 @@ public URI toUri() { try { if (this.encodeState.isEncoded()) { return new URI(toUriString()); - } - else { + } else { String path = getPath(); if (StringUtils.hasLength(path) && path.charAt(0) != PATH_DELIMITER) { // Only prefix the path delimiter if something exists before it @@ -513,8 +470,7 @@ public URI toUri() { } return new URI(getScheme(), getUserInfo(), getHost(), getPort(), path, getQuery(), getFragment()); } - } - catch (URISyntaxException ex) { + } catch (URISyntaxException ex) { throw new IllegalStateException("Could not create URI object: " + ex.getMessage(), ex); } } @@ -568,6 +524,7 @@ public int hashCode() { /** * Enumeration used to identify the allowed characters per URI component. *

Contains methods to indicate whether a given character is valid in a specific URI component. + * * @see RFC 3986 */ enum Type { @@ -631,8 +588,7 @@ public boolean isAllowed(int c) { public boolean isAllowed(int c) { if ('=' == c || '&' == c) { return false; - } - else { + } else { return (isPchar(c) || '/' == c || '?' == c); } } @@ -661,12 +617,14 @@ public boolean isAllowed(int c) { /** * Indicates whether the given character is allowed in this URI component. + * * @return {@code true} if the character is allowed; {@code false} otherwise */ public abstract boolean isAllowed(int c); /** * Indicates whether the given character is in the {@code ALPHA} set. + * * @see RFC 3986, appendix A */ protected boolean isAlpha(int c) { @@ -675,6 +633,7 @@ protected boolean isAlpha(int c) { /** * Indicates whether the given character is in the {@code DIGIT} set. + * * @see RFC 3986, appendix A */ protected boolean isDigit(int c) { @@ -683,6 +642,7 @@ protected boolean isDigit(int c) { /** * Indicates whether the given character is in the {@code gen-delims} set. + * * @see RFC 3986, appendix A */ protected boolean isGenericDelimiter(int c) { @@ -691,6 +651,7 @@ protected boolean isGenericDelimiter(int c) { /** * Indicates whether the given character is in the {@code sub-delims} set. + * * @see RFC 3986, appendix A */ protected boolean isSubDelimiter(int c) { @@ -700,6 +661,7 @@ protected boolean isSubDelimiter(int c) { /** * Indicates whether the given character is in the {@code reserved} set. + * * @see RFC 3986, appendix A */ protected boolean isReserved(int c) { @@ -708,6 +670,7 @@ protected boolean isReserved(int c) { /** * Indicates whether the given character is in the {@code unreserved} set. + * * @see RFC 3986, appendix A */ protected boolean isUnreserved(int c) { @@ -716,6 +679,7 @@ protected boolean isUnreserved(int c) { /** * Indicates whether the given character is in the {@code pchar} set. + * * @see RFC 3986, appendix A */ protected boolean isPchar(int c) { @@ -749,7 +713,7 @@ private enum EncodeState { * URI template encoded first by quoting illegal characters only, and * then URI vars encoded more strictly when expanded, by quoting both * illegal chars and chars with reserved meaning. - */ + */ TEMPLATE_ENCODED; @@ -806,16 +770,13 @@ public String apply(String source, Type type) { if (level == 0) { boolean encode = !isUriVariable(this.currentVariable); append(this.currentVariable, encode, type); - } - else if (!this.variableWithNameAndRegex) { + } else if (!this.variableWithNameAndRegex) { append(this.currentVariable, true, type); level = 0; } - } - else if (level > 0) { + } else if (level > 0) { this.currentVariable.append(c); - } - else { + } else { this.currentLiteral.append(c); } } @@ -833,7 +794,7 @@ else if (level > 0) { * for example, {@code "/{year:\d{1,4}}"}. */ private boolean isUriVariable(CharSequence source) { - if (source.length() < 2 || source.charAt(0) != '{' || source.charAt(source.length() -1) != '}') { + if (source.length() < 2 || source.charAt(0) != '{' || source.charAt(source.length() - 1) != '}') { return false; } boolean hasText = false; @@ -1089,12 +1050,36 @@ public QueryUriTemplateVariables(UriTemplateVariables delegate) { Object value = this.delegate.getValue(name); if (ObjectUtils.isArray(value)) { value = StringUtils.arrayToCommaDelimitedString(ObjectUtils.toObjectArray(value)); - } - else if (value instanceof Collection collection) { + } else if (value instanceof Collection collection) { value = StringUtils.collectionToCommaDelimitedString(collection); } return value; } } + record QueryParam(String name, @Nullable String value) { + public String toUriString() { + return name + (value == null ? "" : "=" + value); + } + + public QueryParam encodeQueryParam(BiFunction encoder) { + return new QueryParam(encoder.apply(name, Type.QUERY_PARAM), + value != null ? encoder.apply(value, Type.QUERY_PARAM) : null); + } + + public void verifyUriComponent() { + HierarchicalUriComponents.verifyUriComponent(name, Type.QUERY_PARAM); + HierarchicalUriComponents.verifyUriComponent(value, Type.QUERY_PARAM); + } + + public QueryParam expandUriComponent(UriTemplateVariables queryVariables, @Nullable UnaryOperator variableEncoder) { + return new QueryParam(Objects.requireNonNull(UriComponents.expandUriComponent(name, queryVariables, variableEncoder)), + UriComponents.expandUriComponent(value, queryVariables, variableEncoder)); + } + + public boolean hasName(String name) { + return name.equals(this.name); + } + } + } diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index e8783e4cf2e0..5e7d724a37cb 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -34,7 +34,6 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -89,7 +88,7 @@ public class UriComponentsBuilder implements UriBuilder, Cloneable { private CompositePathComponentBuilder pathBuilder; - private final MultiValueMap queryParams = new LinkedMultiValueMap<>(); + private final List queryParams = new ArrayList<>(); private @Nullable String fragment; @@ -286,7 +285,7 @@ private UriComponents buildInternal(EncodingHint hint) { result = new OpaqueUriComponents(this.scheme, this.ssp, this.fragment); } else { - MultiValueMap queryParams = new LinkedMultiValueMap<>(this.queryParams); + List queryParams = new ArrayList<>(this.queryParams); HierarchicalUriComponents uric = new HierarchicalUriComponents(this.scheme, this.fragment, this.userInfo, this.host, this.port, this.pathBuilder.build(), queryParams, hint == EncodingHint.FULLY_ENCODED); @@ -575,11 +574,11 @@ public UriComponentsBuilder queryParam(String name, @Nullable Object... values) if (!ObjectUtils.isEmpty(values)) { for (Object value : values) { String valueAsString = getQueryParamValue(value); - this.queryParams.add(name, valueAsString); + this.queryParams.add(new HierarchicalUriComponents.QueryParam(name, valueAsString)); } } else { - this.queryParams.add(name, null); + this.queryParams.add(new HierarchicalUriComponents.QueryParam(name, null)); } resetSchemeSpecificPart(); return this; @@ -619,16 +618,20 @@ public UriComponentsBuilder queryParamIfPresent(String name, Optional value) @Override public UriComponentsBuilder queryParams(@Nullable MultiValueMap params) { if (params != null) { - this.queryParams.addAll(params); + addAllToQueryParams(params); resetSchemeSpecificPart(); } return this; } + private void addAllToQueryParams(MultiValueMap params) { + params.forEach((key, values) -> values.forEach(v -> this.queryParam(key, v))); + } + @Override public UriComponentsBuilder replaceQueryParam(String name, Object... values) { Assert.notNull(name, "Name must not be null"); - this.queryParams.remove(name); + this.queryParams.removeIf(qp->qp.hasName(name)); if (!ObjectUtils.isEmpty(values)) { queryParam(name, values); } @@ -649,7 +652,7 @@ public UriComponentsBuilder replaceQueryParam(String name, @Nullable Collection< public UriComponentsBuilder replaceQueryParams(@Nullable MultiValueMap params) { this.queryParams.clear(); if (params != null) { - this.queryParams.putAll(params); + addAllToQueryParams(params); } return this; } diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index 991b91801d7f..2088b0702ec7 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -16,24 +16,19 @@ package org.springframework.web.util; -import java.net.URI; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.function.BiConsumer; - import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.EnumSource; - import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.util.UriComponentsBuilder.ParserType; +import java.net.URI; +import java.util.*; +import java.util.function.BiConsumer; +import java.util.stream.Stream; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -930,4 +925,50 @@ void expandPortAndPathWithoutSeparator(ParserType parserType) { assertThat(uri.toString()).isEqualTo("ws://localhost:7777/test"); } + @ParameterizedTest(name = "{0} for {3}, {4}, {5} with parser {1}") //gh 34788 + @CsvSource(textBlock = """ + #testedPattern parserType urlExpected p1 p2 p3 + 'http://myhost?a={p1}&b={p2}&a={p3}',RFC, 'http://myhost?a=a1&b=b1&a=a2','a1','b1','a2' + 'http://myhost?a={p1}&b={p2}&a={p3}',WHAT_WG, 'http://myhost?a=a1&b=b1&a=a2','a1','b1','a2' + 'http://myhost?a={p1}&b={p2}&a={p1}',RFC, 'http://myhost?a=a1&b=b1&a=a1','a1','b1','a1' + 'http://myhost?a={p1}&a={p1}&b={p3}',RFC, 'http://myhost?a=a1&a=a1&b=b1','a1','a1','b1', + 'http://myhost?a={p1}&a={p2}&b={p3}',RFC, 'http://myhost?a=a1&a=a2&b=b1','a1','a2','b1' + 'http://myhost?a={p1}&b={p2}&a={p1}',RFC, 'http://myhost?a=a1&b=&a=a1','a1','','a1' + 'http://myhost?a={p1}&b=&a={p2}', RFC, 'http://myhost?a=a1&b=&a=a2','a1','a2', + 'http://myhost?a={p1}&b=t&a={p2}', RFC, 'http://myhost?a=a1&b=t&a=a2','a1','a2', + 'http://myhost?a=t&b={p1}&a={p2}', RFC, 'http://myhost?a=t&b=b1&a=a1','b1','a1', + 'http://myhost?a={p1}&b={p2}&a={p1}',WHAT_WG, 'http://myhost?a=a1&b=b1&a=a1','a1','b1','a1' + 'http://myhost?a={p1}&a={p1}&b={p3}',WHAT_WG, 'http://myhost?a=a1&a=a1&b=b1','a1','a1','b1', + 'http://myhost?a={p1}&a={p2}&b={p3}',WHAT_WG, 'http://myhost?a=a1&a=a2&b=b1','a1','a2','b1' + 'http://myhost?a={p1}&b={p2}&a={p1}',WHAT_WG, 'http://myhost?a=a1&b=&a=a1','a1','','a1' + 'http://myhost?a={p1}&b=&a={p2}', WHAT_WG, 'http://myhost?a=a1&b=&a=a2','a1','a2', + 'http://myhost?a={p1}&b=t&a={p2}', WHAT_WG, 'http://myhost?a=a1&b=t&a=a2','a1','a2', + 'http://myhost?a=t&b={p1}&a={p2}', WHAT_WG, 'http://myhost?a=t&b=b1&a=a1','b1','a1', + """) + void queryParamOrderShouldBeKept(String testedPattern, ParserType parserType, String urlexpected, String p1, String p2, String p3){ + ArrayList params = new ArrayList<>(); + Stream.of(p1, p2, p3).forEach(p->addIfNotNull(p, params)); + Map paramsMap = new HashMap<>(); + putIfNotNull("p1", p1, paramsMap); + putIfNotNull("p2", p2, paramsMap); + putIfNotNull("p3", p3, paramsMap); + UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(testedPattern, parserType); + assertThat(uriComponentsBuilder).satisfies( + ucb -> assertThat(ucb.buildAndExpand(params.toArray())).as("with params as varags").hasToString(urlexpected), + ucb -> assertThat(ucb.buildAndExpand(paramsMap)).as("with params as Map").hasToString(urlexpected) + ); + } + + private static void putIfNotNull(String key, String param, Map paramsMap) { + if (param != null) { + paramsMap.put(key, param); + } + } + + private static void addIfNotNull(String param, ArrayList params) { + if (param !=null){ + params.add(param); + } + } + } From 42e5f3aea4541a607a0415c8c1e61a4b1dc6efd4 Mon Sep 17 00:00:00 2001 From: Fabrice Bibonne Date: Fri, 17 Oct 2025 22:46:13 +0200 Subject: [PATCH 2/4] fix (URI components) style check errors Signed-off-by: Fabrice Bibonne --- .../web/util/HierarchicalUriComponents.java | 103 ++++++++++-------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java index f1e8e74daf35..a6104184c9c7 100644 --- a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java +++ b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java @@ -16,21 +16,29 @@ package org.springframework.web.util; -import org.jspecify.annotations.Nullable; -import org.springframework.util.*; - import java.io.ByteArrayOutputStream; import java.io.Serializable; import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.StringJoiner; import java.util.function.BiFunction; import java.util.function.UnaryOperator; import java.util.stream.Collectors; -import static java.util.stream.Collectors.mapping; -import static java.util.stream.Collectors.toList; +import org.jspecify.annotations.Nullable; + +import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StreamUtils; +import org.springframework.util.StringUtils; /** * Extension of {@link UriComponents} for hierarchical URIs. @@ -40,8 +48,9 @@ * @author Rossen Stoyanchev * @author Phillip Webb * @author Sam Brannen - * @see Hierarchical URIs * @since 3.1.3 + * @see Hierarchical URIs + * */ @SuppressWarnings("serial") final class HierarchicalUriComponents extends UriComponents { @@ -125,8 +134,8 @@ public int hashCode() { * @param encoded whether the components are already encoded */ HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, @Nullable String userInfo, - @Nullable String host, @Nullable String port, @Nullable PathComponent path, - @Nullable List query, boolean encoded) { + @Nullable String host, @Nullable String port, @Nullable PathComponent path, + @Nullable List query, boolean encoded) { super(scheme, fragment); @@ -144,9 +153,9 @@ public int hashCode() { } private HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, - @Nullable String userInfo, @Nullable String host, @Nullable String port, - PathComponent path, List queryParams, - EncodeState encodeState, @Nullable UnaryOperator variableEncoder) { + @Nullable String userInfo, @Nullable String host, @Nullable String port, + PathComponent path, List queryParams, + EncodeState encodeState, @Nullable UnaryOperator variableEncoder) { super(scheme, fragment); @@ -181,13 +190,15 @@ private HierarchicalUriComponents(@Nullable String scheme, @Nullable String frag public int getPort() { if (this.port == null) { return -1; - } else if (this.port.contains("{")) { + } + else if (this.port.contains("{")) { throw new IllegalStateException( "The port contains a URI variable but has not been expanded yet: " + this.port); } try { return Integer.parseInt(this.port); - } catch (NumberFormatException ex) { + } + catch (NumberFormatException ex) { throw new IllegalStateException("The port must be an integer: " + this.port); } } @@ -208,7 +219,8 @@ public List getPathSegments() { return this.queryParams.stream() .map(QueryParam::toUriString) .collect(Collectors.joining("&")); - } else { + } + else { return null; } } @@ -219,7 +231,7 @@ public List getPathSegments() { @Override public MultiValueMap getQueryParams() { Map> collected = this.queryParams.stream().collect(Collectors.groupingBy(QueryParam::name, - mapping(QueryParam::value, toList()))); + Collectors.mapping(QueryParam::value, Collectors.toList()))); return MultiValueMap.fromMultiValue(collected); } @@ -277,7 +289,6 @@ private List encodeQueryParams(BiFunction enco /** * Encode the given source into an encoded String using the rules specified * by the given component and with the given options. - * * @param source the source String * @param encoding the encoding of the source String * @param type the URI component for the source @@ -291,7 +302,6 @@ static String encodeUriComponent(String source, String encoding, Type type) { /** * Encode the given source into an encoded String using the rules specified * by the given component and with the given options. - * * @param source the source String * @param charset the encoding of the source String * @param type the URI component for the source @@ -321,7 +331,8 @@ static String encodeUriComponent(String source, Charset charset, Type type) { for (byte b : bytes) { if (type.isAllowed(b)) { baos.write(b); - } else { + } + else { baos.write('%'); char hex1 = Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16)); char hex2 = Character.toUpperCase(Character.forDigit(b & 0xF, 16)); @@ -340,7 +351,6 @@ private Type getHostType() { /** * Check if any of the URI components contain any illegal characters. - * * @throws IllegalArgumentException if any component has illegal characters */ private void verify() { @@ -370,11 +380,13 @@ private static void verifyUriComponent(@Nullable String source, Type type) { source.substring(i) + "\""); } i += 2; - } else { + } + else { throw new IllegalArgumentException("Invalid encoded sequence \"" + source.substring(i) + "\""); } - } else if (!type.isAllowed(ch)) { + } + else if (!type.isAllowed(ch)) { throw new IllegalArgumentException("Invalid character '" + ch + "' for " + type.name() + " in \"" + source + "\""); } @@ -405,7 +417,7 @@ protected HierarchicalUriComponents expandInternal(UriTemplateVariables uriVaria private List expandQueryParams(UriTemplateVariables variables) { UriTemplateVariables queryVariables = new QueryUriTemplateVariables(variables); return this.queryParams.stream() - .map(qp->qp.expandUriComponent(queryVariables, this.variableEncoder)) + .map(qp -> qp.expandUriComponent(queryVariables, this.variableEncoder)) .toList(); } @@ -460,7 +472,8 @@ public URI toUri() { try { if (this.encodeState.isEncoded()) { return new URI(toUriString()); - } else { + } + else { String path = getPath(); if (StringUtils.hasLength(path) && path.charAt(0) != PATH_DELIMITER) { // Only prefix the path delimiter if something exists before it @@ -470,7 +483,8 @@ public URI toUri() { } return new URI(getScheme(), getUserInfo(), getHost(), getPort(), path, getQuery(), getFragment()); } - } catch (URISyntaxException ex) { + } + catch (URISyntaxException ex) { throw new IllegalStateException("Could not create URI object: " + ex.getMessage(), ex); } } @@ -588,7 +602,8 @@ public boolean isAllowed(int c) { public boolean isAllowed(int c) { if ('=' == c || '&' == c) { return false; - } else { + } + else { return (isPchar(c) || '/' == c || '?' == c); } } @@ -617,14 +632,12 @@ public boolean isAllowed(int c) { /** * Indicates whether the given character is allowed in this URI component. - * * @return {@code true} if the character is allowed; {@code false} otherwise */ public abstract boolean isAllowed(int c); /** * Indicates whether the given character is in the {@code ALPHA} set. - * * @see RFC 3986, appendix A */ protected boolean isAlpha(int c) { @@ -633,7 +646,6 @@ protected boolean isAlpha(int c) { /** * Indicates whether the given character is in the {@code DIGIT} set. - * * @see RFC 3986, appendix A */ protected boolean isDigit(int c) { @@ -642,7 +654,6 @@ protected boolean isDigit(int c) { /** * Indicates whether the given character is in the {@code gen-delims} set. - * * @see RFC 3986, appendix A */ protected boolean isGenericDelimiter(int c) { @@ -651,7 +662,6 @@ protected boolean isGenericDelimiter(int c) { /** * Indicates whether the given character is in the {@code sub-delims} set. - * * @see RFC 3986, appendix A */ protected boolean isSubDelimiter(int c) { @@ -661,7 +671,6 @@ protected boolean isSubDelimiter(int c) { /** * Indicates whether the given character is in the {@code reserved} set. - * * @see RFC 3986, appendix A */ protected boolean isReserved(int c) { @@ -670,7 +679,6 @@ protected boolean isReserved(int c) { /** * Indicates whether the given character is in the {@code unreserved} set. - * * @see RFC 3986, appendix A */ protected boolean isUnreserved(int c) { @@ -679,7 +687,6 @@ protected boolean isUnreserved(int c) { /** * Indicates whether the given character is in the {@code pchar} set. - * * @see RFC 3986, appendix A */ protected boolean isPchar(int c) { @@ -770,13 +777,16 @@ public String apply(String source, Type type) { if (level == 0) { boolean encode = !isUriVariable(this.currentVariable); append(this.currentVariable, encode, type); - } else if (!this.variableWithNameAndRegex) { + } + else if (!this.variableWithNameAndRegex) { append(this.currentVariable, true, type); level = 0; } - } else if (level > 0) { + } + else if (level > 0) { this.currentVariable.append(c); - } else { + } + else { this.currentLiteral.append(c); } } @@ -1050,31 +1060,32 @@ public QueryUriTemplateVariables(UriTemplateVariables delegate) { Object value = this.delegate.getValue(name); if (ObjectUtils.isArray(value)) { value = StringUtils.arrayToCommaDelimitedString(ObjectUtils.toObjectArray(value)); - } else if (value instanceof Collection collection) { + } + else if (value instanceof Collection collection) { value = StringUtils.collectionToCommaDelimitedString(collection); } return value; } } - record QueryParam(String name, @Nullable String value) { + record QueryParam(String name, @Nullable String value) implements Serializable{ public String toUriString() { - return name + (value == null ? "" : "=" + value); + return this.name + (this.value == null ? "" : "=" + this.value); } public QueryParam encodeQueryParam(BiFunction encoder) { - return new QueryParam(encoder.apply(name, Type.QUERY_PARAM), - value != null ? encoder.apply(value, Type.QUERY_PARAM) : null); + return new QueryParam(encoder.apply(this.name, Type.QUERY_PARAM), + this.value != null ? encoder.apply(this.value, Type.QUERY_PARAM) : null); } public void verifyUriComponent() { - HierarchicalUriComponents.verifyUriComponent(name, Type.QUERY_PARAM); - HierarchicalUriComponents.verifyUriComponent(value, Type.QUERY_PARAM); + HierarchicalUriComponents.verifyUriComponent(this.name, Type.QUERY_PARAM); + HierarchicalUriComponents.verifyUriComponent(this.value, Type.QUERY_PARAM); } public QueryParam expandUriComponent(UriTemplateVariables queryVariables, @Nullable UnaryOperator variableEncoder) { - return new QueryParam(Objects.requireNonNull(UriComponents.expandUriComponent(name, queryVariables, variableEncoder)), - UriComponents.expandUriComponent(value, queryVariables, variableEncoder)); + return new QueryParam(Objects.requireNonNull(UriComponents.expandUriComponent(this.name, queryVariables, variableEncoder)), + UriComponents.expandUriComponent(this.value, queryVariables, variableEncoder)); } public boolean hasName(String name) { From 8a1a56a774b9018ee543b6e29b931a09b74762e4 Mon Sep 17 00:00:00 2001 From: Fabrice Bibonne Date: Fri, 17 Oct 2025 22:47:46 +0200 Subject: [PATCH 3/4] fix (URI components) test - fix MockHttpServletRequestBuilderTests#queryParameterWithoutValues Signed-off-by: Fabrice Bibonne --- .../springframework/web/util/UriComponentsBuilder.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index 5e7d724a37cb..3e1cf162d0cf 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -625,7 +625,14 @@ public UriComponentsBuilder queryParams(@Nullable MultiValueMap } private void addAllToQueryParams(MultiValueMap params) { - params.forEach((key, values) -> values.forEach(v -> this.queryParam(key, v))); + params.forEach((key, values) -> { + if (values.isEmpty()) { + this.queryParams.add(new HierarchicalUriComponents.QueryParam(key, null)); + } + else{ + values.forEach(v -> this.queryParams.add(new HierarchicalUriComponents.QueryParam(key, v))); + } + }); } @Override From 6d75001e78f1259c317727d3f3ae8444e8987215 Mon Sep 17 00:00:00 2001 From: Fabrice Bibonne Date: Fri, 17 Oct 2025 22:50:15 +0200 Subject: [PATCH 4/4] test (URI components) correct tests impacted by feature - order of params change in result expressions in a few tests : expected results have to be changed Signed-off-by: Fabrice Bibonne --- .../reactive/MockServerHttpRequestTests.java | 2 +- .../web/filter/RequestLoggingFilterTests.java | 5 +++-- .../web/util/UriComponentsBuilderTests.java | 18 +++++++++++++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/spring-test/src/test/java/org/springframework/mock/http/server/reactive/MockServerHttpRequestTests.java b/spring-test/src/test/java/org/springframework/mock/http/server/reactive/MockServerHttpRequestTests.java index b2a4dff2ea2a..b2bac3ea887d 100644 --- a/spring-test/src/test/java/org/springframework/mock/http/server/reactive/MockServerHttpRequestTests.java +++ b/spring-test/src/test/java/org/springframework/mock/http/server/reactive/MockServerHttpRequestTests.java @@ -65,7 +65,7 @@ void queryParams() { .queryParam("name B", "value B1") .build(); - assertThat(request.getURI().toString()).isEqualTo("/foo%20bar?a=b&name%20A=value%20A1&name%20A=value%20A2&name%20B=value%20B1"); + assertThat(request.getURI().toString()).isEqualTo("/foo%20bar?a=b&name%20B=value%20B1&name%20A=value%20A1&name%20A=value%20A2"); } /** diff --git a/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java index 349834760492..6e6de9197283 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java @@ -114,8 +114,9 @@ void queryStringIncluded() throws Exception { applyFilter(); - assertThat(filter.beforeRequestMessage).contains("/hotels?booking=42&code=masked&category=hotel&category=resort&ignore=masked"); - assertThat(filter.afterRequestMessage).contains("/hotels?booking=42&code=masked&category=hotel&category=resort&ignore=masked"); + String expectedRequest = "/hotels?booking=42&code=masked&ignore=masked&category=hotel&category=resort"; + assertThat(filter.beforeRequestMessage).contains(expectedRequest); + assertThat(filter.afterRequestMessage).contains(expectedRequest); } @Test diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index 2088b0702ec7..94e6a46ac568 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -16,19 +16,27 @@ package org.springframework.web.util; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.EnumSource; + import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.util.UriComponentsBuilder.ParserType; -import java.net.URI; -import java.util.*; -import java.util.function.BiConsumer; -import java.util.stream.Stream; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;