From 51a5d1e94f2da3ba54ec48ad08094a3368048808 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 18 Mar 2026 17:14:04 +0100 Subject: [PATCH 01/29] Add RestUtils.decodeQueryStringMulti for repeated query parameters The existing decodeQueryString fills a Map so repeated parameters (e.g. match[]=foo&match[]=bar) silently drop all but the last value. Add decodeQueryStringMulti that returns Map> and preserves every occurrence. Internally the parsing loop is extracted into a private parseQueryStringPairs helper that accepts a BiConsumer, allowing both the single-value and multi-value variants to share the same decoding logic without duplication. --- .../org/elasticsearch/rest/RestUtils.java | 41 ++++++++++-- .../elasticsearch/rest/RestUtilsTests.java | 66 +++++++++++++++++++ 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 2c6b9568932a2..08bbb6f6930bc 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -23,9 +23,13 @@ import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.BiConsumer; import java.util.function.UnaryOperator; import java.util.regex.Pattern; @@ -49,6 +53,27 @@ public static void decodeQueryString(URI uri, Map params) { } public static void decodeQueryString(String s, int fromIndex, Map params) { + parseQueryStringPairs(s, fromIndex, (name, value) -> addParam(params, name, value)); + } + + /** + * Parses a URL-encoded query string into a multi-value map, preserving all values for + * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). + * + * @param s the full string containing the query string + * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) + * @return a map from parameter name to all its values, in encounter order + */ + public static Map> decodeQueryStringMulti(String s, int fromIndex) { + Map> result = new LinkedHashMap<>(); + parseQueryStringPairs(s, fromIndex, (name, value) -> { + checkReservedParam(name); + result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); + }); + return result; + } + + private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer consumer) { if (fromIndex < 0) { return; } @@ -74,9 +99,9 @@ public static void decodeQueryString(String s, int fromIndex, Map RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1) + ); + } + } + public void testCorsSettingIsARegex() { assertCorsSettingRegex("/foo/", Pattern.compile("foo")); assertCorsSettingRegex("/.*/", Pattern.compile(".*")); From 09c97c80a7da8d77e0c758a0385d3d5ff8379dc5 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 18 Mar 2026 16:48:57 +0000 Subject: [PATCH 02/29] [CI] Auto commit changes from spotless --- .../src/test/java/org/elasticsearch/rest/RestUtilsTests.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index e8eb201958f93..42989863e1c24 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -175,10 +175,7 @@ public void testDecodeQueryStringMultiUrlEncoded() { public void testDecodeQueryStringMultiReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { String uri = "something?" + reservedParam + "=value"; - expectThrows( - IllegalArgumentException.class, - () -> RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1) - ); + expectThrows(IllegalArgumentException.class, () -> RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1)); } } From 3ca02b6f5553a9483f2ff0850e1e9005537f769b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 10:30:39 +0100 Subject: [PATCH 03/29] Introduce ParameterMap and migrate decodeQueryString callers Adds ParameterMap, a Map that preserves multiple values per key via getAll()/getSingle(), and migrates all call sites that created a local HashMap just to pass to decodeQueryString over to the new decodeQueryStringMulti() which returns a ParameterMap directly. Also removes the now-dead decodeQueryString(URI, Map) overload. --- .../azure/AzureBlobContainerRetriesTests.java | 7 +- ...CloudStorageBlobContainerRetriesTests.java | 4 +- .../org/elasticsearch/rest/ParameterMap.java | 199 ++++++++++++++++++ .../org/elasticsearch/rest/RestRequest.java | 34 +-- .../org/elasticsearch/rest/RestUtils.java | 27 ++- .../elasticsearch/rest/ParameterMapTests.java | 161 ++++++++++++++ .../elasticsearch/rest/RestUtilsTests.java | 21 +- .../java/fixture/azure/AzureHttpHandler.java | 7 +- .../AzureOAuthTokenServiceHttpHandler.java | 5 +- .../gcs/GoogleCloudStorageHttpHandler.java | 12 +- .../test/rest/FakeRestRequest.java | 21 +- .../saml/authn/SamlAuthnRequestValidator.java | 3 +- .../exporter/http/HttpExporterIT.java | 6 +- .../microsoft/MicrosoftGraphHttpFixture.java | 4 +- .../authc/saml/SamlObjectHandler.java | 5 +- .../authc/oidc/OpenIdConnectRealmTests.java | 13 +- .../security/authc/jwt/JwtWithOidcAuthIT.java | 4 +- 17 files changed, 439 insertions(+), 94 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/rest/ParameterMap.java create mode 100644 server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java diff --git a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java index b6c2fe29636fe..f19ec98e81724 100644 --- a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java +++ b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java @@ -65,7 +65,6 @@ import java.time.Duration; import java.util.Arrays; import java.util.Base64; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -369,8 +368,7 @@ public void testWriteLargeBlob() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0, params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); @@ -439,8 +437,7 @@ public void testWriteLargeBlobStreaming() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob_streaming"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0, params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); diff --git a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java index 41505b58d6c1b..1fd0acc3839f3 100644 --- a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java +++ b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java @@ -68,7 +68,6 @@ import java.net.SocketTimeoutException; import java.nio.file.NoSuchFileException; import java.util.Arrays; -import java.util.HashMap; import java.util.Iterator; import java.util.Locale; import java.util.Map; @@ -421,8 +420,7 @@ public void testWriteLargeBlob() throws IOException { httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> { final BytesReference requestBody = Streams.readFully(exchange.getRequestBody()); - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0, params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getQuery(), 0); assertThat(params.get("uploadType"), equalTo("resumable")); if ("POST".equals(exchange.getRequestMethod())) { diff --git a/server/src/main/java/org/elasticsearch/rest/ParameterMap.java b/server/src/main/java/org/elasticsearch/rest/ParameterMap.java new file mode 100644 index 0000000000000..5941309b6a832 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/ParameterMap.java @@ -0,0 +1,199 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest; + +import java.util.AbstractMap; +import java.util.AbstractSet; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * A {@link Map}{@code } for HTTP request parameters that preserves multiple values + * per key (e.g. repeated query parameters such as {@code match[]=foo&match[]=bar}). + * + *

Each key maps to a non-empty ordered list of values. The standard {@link Map} interface + * operates on the last value in that list: + *

    + *
  • {@link #get(Object)} returns the last value for a key, or {@code null} if absent.
    • + *
  • {@link #put(String, String)} replaces all existing values with a single new one.
    • + *
+ * Use {@link #getAll(String)} to retrieve all values for a repeated key. + */ +public final class ParameterMap extends AbstractMap { + + /** Single backing store: key → non-empty ordered list of all values. */ + private final LinkedHashMap> map; + + // ------------------------------------------------------------------------- + // Factory methods + // ------------------------------------------------------------------------- + + /** + * Returns a new, empty {@code ParameterMap}. + */ + public static ParameterMap empty() { + return new ParameterMap(Map.of()); + } + + /** + * Creates a {@code ParameterMap} from a multi-value map. Each list must be non-empty. + * The last value in each list is what {@link #get(Object)} returns. + * + * @param multiValues a map from parameter name to all its values, in encounter order + */ + public static ParameterMap of(Map> multiValues) { + return new ParameterMap(multiValues); + } + + /** + * Creates a {@code ParameterMap} from a plain single-value map. + * Each value is wrapped in a singleton list so that {@link #getAll(String)} returns a + * one-element list rather than an empty one. + * + * @param singleValues a map whose values are treated as the sole value for each key + */ + public static ParameterMap fromSingleValues(Map singleValues) { + LinkedHashMap> wrapped = new LinkedHashMap<>(singleValues.size() * 2); + singleValues.forEach((k, v) -> wrapped.put(k, List.of(v))); + return new ParameterMap(wrapped); + } + + private ParameterMap(Map> multiValues) { + this.map = new LinkedHashMap<>(multiValues); + assert map.values().stream().allMatch(list -> list != null && list.isEmpty() == false) + : "ParameterMap requires every value list to be non-empty"; + } + + // ------------------------------------------------------------------------- + // Multi-value API + // ------------------------------------------------------------------------- + + /** + * Returns all values for {@code key} in the order they were added, + * or an empty list if the key is absent. + * + * @param key the parameter name + * @return a non-empty list of all values, or an empty list if absent; never {@code null} + */ + public List getAll(String key) { + var list = map.get(key); + return list == null ? List.of() : Collections.unmodifiableList(list); + } + + /** + * Returns the single value for {@code key}, or {@code null} if absent. + * Throws {@link IllegalArgumentException} if the key has more than one value. + * + * @param key the parameter name + * @return the single value, or {@code null} if absent + * @throws IllegalArgumentException if the key has multiple values + */ + public String getSingle(String key) { + var list = map.get(key); + if (list == null) { + return null; + } + if (list.size() > 1) { + throw new IllegalArgumentException("parameter [" + key + "] must have a single value, but found: " + list); + } + return list.getFirst(); + } + + // ------------------------------------------------------------------------- + // Map — single-value view over the backing list map + // ------------------------------------------------------------------------- + + /** + * Returns the last value associated with {@code key}, or {@code null} if absent. + */ + @Override + public String get(Object key) { + var list = map.get(key); + return list == null ? null : list.getLast(); + } + + /** + * Associates {@code key} with {@code value}, replacing all previous values for that key. + * Returns the previous last value, or {@code null}. + */ + @Override + public String put(String key, String value) { + var old = map.put(key, new ArrayList<>(List.of(value))); + return old == null ? null : old.getLast(); + } + + @Override + public String remove(Object key) { + var old = map.remove(key); + return old == null ? null : old.getLast(); + } + + @Override + public void clear() { + map.clear(); + } + + @Override + public int size() { + return map.size(); + } + + @Override + public boolean containsKey(Object key) { + return map.containsKey(key); + } + + /** Returns a live key set backed by the underlying map. */ + @Override + public Set keySet() { + return map.keySet(); + } + + /** + * Returns a live entry set where each entry's value is the last value for that key. + * Removal through the iterator is supported and removes the key from the map entirely. + */ + @Override + public Set> entrySet() { + return new AbstractSet<>() { + @Override + public Iterator> iterator() { + var inner = map.entrySet().iterator(); + return new Iterator<>() { + @Override + public boolean hasNext() { + return inner.hasNext(); + } + + @Override + public Entry next() { + var e = inner.next(); + return Map.entry(e.getKey(), e.getValue().getLast()); + } + + @Override + public void remove() { + inner.remove(); + } + }; + } + + @Override + public int size() { + return map.size(); + } + }; + } +} diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 92e83fb9701a3..dc7e02a45c160 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -40,7 +40,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -87,7 +86,7 @@ public class RestRequest implements ToXContent.Params, Traceable { private static final AtomicLong requestIdGenerator = new AtomicLong(); private final XContentParserConfiguration parserConfig; - private final Map params; + private final ParameterMap params; private final Map> headers; private final String rawPath; private final Set consumedParams = new HashSet<>(); @@ -109,7 +108,7 @@ public boolean isContentConsumed() { @SuppressWarnings("this-escape") protected RestRequest( XContentParserConfiguration parserConfig, - Map params, + ParameterMap params, String rawPath, Map> headers, HttpRequest httpRequest, @@ -121,7 +120,7 @@ protected RestRequest( @SuppressWarnings("this-escape") private RestRequest( XContentParserConfiguration parserConfig, - Map params, + ParameterMap params, String rawPath, Map> headers, HttpRequest httpRequest, @@ -199,7 +198,7 @@ protected RestRequest(RestRequest other) { * @throws MediaTypeHeaderException if the Content-Type or Accept header can not be parsed */ public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) { - Map params = params(httpRequest.uri()); + ParameterMap params = params(httpRequest.uri()); return new RestRequest( parserConfig, params, @@ -211,17 +210,16 @@ public static RestRequest request(XContentParserConfiguration parserConfig, Http ); } - private static Map params(final String uri) { - final Map params = new HashMap<>(); + private static ParameterMap params(final String uri) { int index = uri.indexOf('?'); if (index >= 0) { try { - RestUtils.decodeQueryString(uri, index + 1, params); + return RestUtils.decodeQueryStringMulti(uri, index + 1); } catch (final IllegalArgumentException e) { throw new BadParameterException(e); } } - return params; + return ParameterMap.empty(); } /** @@ -235,10 +233,9 @@ public static RestRequest requestWithoutParameters( HttpRequest httpRequest, HttpChannel httpChannel ) { - Map params = Collections.emptyMap(); return new RestRequest( parserConfig, - params, + ParameterMap.empty(), httpRequest.uri(), httpRequest.getHeaders(), httpRequest, @@ -423,10 +420,23 @@ public final String param(String key, String defaultValue) { return value; } - public Map params() { + public ParameterMap params() { return params; } + /** + * Returns all values for the given query parameter, preserving the order in which they appeared in the URL. + * This is useful for parameters that may be repeated (e.g. {@code match[]=foo&match[]=bar}). + * Unlike {@link #param(String)}, path parameters are not visible through this method. + * + * @param key the parameter name + * @return all values for the parameter, or an empty list if the parameter was not present + */ + public List paramAsList(String key) { + consumedParams.add(key); + return params.getAll(key); + } + /** * Returns a list of parameters that have been consumed. This method returns a copy, callers * are free to modify the returned list. diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 08bbb6f6930bc..f362f04917e89 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -45,32 +45,37 @@ public class RestUtils { public static final UnaryOperator REST_DECODER = RestUtils::decodeComponent; - public static void decodeQueryString(URI uri, Map params) { - final var rawQuery = uri.getRawQuery(); - if (Strings.hasLength(rawQuery)) { - decodeQueryString(rawQuery, 0, params); - } - } - public static void decodeQueryString(String s, int fromIndex, Map params) { parseQueryStringPairs(s, fromIndex, (name, value) -> addParam(params, name, value)); } /** - * Parses a URL-encoded query string into a multi-value map, preserving all values for + * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for + * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). + * + * @param uri the URI whose raw query string is parsed + * @return a {@link ParameterMap} from parameter name to all its values, in encounter order + */ + public static ParameterMap decodeQueryStringMulti(URI uri) { + final var rawQuery = uri.getRawQuery(); + return Strings.hasLength(rawQuery) ? decodeQueryStringMulti(rawQuery, 0) : ParameterMap.empty(); + } + + /** + * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). * * @param s the full string containing the query string * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) - * @return a map from parameter name to all its values, in encounter order + * @return a {@link ParameterMap} from parameter name to all its values, in encounter order */ - public static Map> decodeQueryStringMulti(String s, int fromIndex) { + public static ParameterMap decodeQueryStringMulti(String s, int fromIndex) { Map> result = new LinkedHashMap<>(); parseQueryStringPairs(s, fromIndex, (name, value) -> { checkReservedParam(name); result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); }); - return result; + return ParameterMap.of(result); } private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer consumer) { diff --git a/server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java b/server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java new file mode 100644 index 0000000000000..8f61afae38549 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java @@ -0,0 +1,161 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest; + +import org.elasticsearch.test.ESTestCase; + +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; + +public class ParameterMapTests extends ESTestCase { + + // ------------------------------------------------------------------------- + // Factory methods + // ------------------------------------------------------------------------- + + public void testEmpty() { + var map = ParameterMap.empty(); + assertThat(map.isEmpty(), is(true)); + assertThat(map.size(), equalTo(0)); + assertThat(map.get("x"), nullValue()); + assertThat(map.getAll("x"), equalTo(List.of())); + } + + public void testOf() { + var map = ParameterMap.of(Map.of("a", List.of("1", "2"), "b", List.of("3"))); + assertThat(map.size(), equalTo(2)); + assertThat(map.getAll("a"), equalTo(List.of("1", "2"))); + assertThat(map.getAll("b"), equalTo(List.of("3"))); + } + + public void testFromSingleValues() { + var map = ParameterMap.fromSingleValues(Map.of("a", "1", "b", "2")); + assertThat(map.size(), equalTo(2)); + assertThat(map.get("a"), equalTo("1")); + assertThat(map.get("b"), equalTo("2")); + assertThat(map.getAll("a"), equalTo(List.of("1"))); + assertThat(map.getAll("b"), equalTo(List.of("2"))); + } + + // ------------------------------------------------------------------------- + // get / getAll + // ------------------------------------------------------------------------- + + public void testGetReturnsLastValue() { + var map = ParameterMap.of(Map.of("k", List.of("first", "second", "last"))); + assertThat(map.get("k"), equalTo("last")); + } + + public void testGetReturnsNullForAbsentKey() { + var map = ParameterMap.of(Map.of("k", List.of("v"))); + assertThat(map.get("missing"), nullValue()); + } + + public void testGetAllReturnsAllValues() { + var map = ParameterMap.of(Map.of("k", List.of("a", "b", "c"))); + assertThat(map.getAll("k"), equalTo(List.of("a", "b", "c"))); + } + + public void testGetAllReturnsEmptyListForAbsentKey() { + var map = ParameterMap.empty(); + assertThat(map.getAll("missing"), equalTo(List.of())); + } + + // ------------------------------------------------------------------------- + // getSingle + // ------------------------------------------------------------------------- + + public void testGetSingleReturnsSingleValue() { + var map = ParameterMap.of(Map.of("k", List.of("only"))); + assertThat(map.getSingle("k"), equalTo("only")); + } + + public void testGetSingleReturnsNullForAbsentKey() { + var map = ParameterMap.empty(); + assertThat(map.getSingle("missing"), nullValue()); + } + + public void testGetSingleThrowsOnMultipleValues() { + var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var ex = expectThrows(IllegalArgumentException.class, () -> map.getSingle("k")); + assertThat(ex.getMessage(), equalTo("parameter [k] must have a single value, but found: [a, b]")); + } + + // ------------------------------------------------------------------------- + // put / remove / clear + // ------------------------------------------------------------------------- + + public void testPutReplacesAllValues() { + var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var previous = map.put("k", "new"); + assertThat(previous, equalTo("b")); // previous last value + assertThat(map.get("k"), equalTo("new")); + assertThat(map.getAll("k"), equalTo(List.of("new"))); + } + + public void testPutNewKey() { + var map = ParameterMap.empty(); + var previous = map.put("k", "v"); + assertThat(previous, nullValue()); + assertThat(map.get("k"), equalTo("v")); + } + + public void testRemove() { + var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var removed = map.remove("k"); + assertThat(removed, equalTo("b")); // last value + assertThat(map.containsKey("k"), is(false)); + } + + public void testRemoveAbsentKey() { + var map = ParameterMap.empty(); + assertThat(map.remove("missing"), nullValue()); + } + + public void testClear() { + var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + map.clear(); + assertThat(map.isEmpty(), is(true)); + } + + // ------------------------------------------------------------------------- + // Map interface — containsKey, keySet, entrySet + // ------------------------------------------------------------------------- + + public void testContainsKey() { + var map = ParameterMap.of(Map.of("present", List.of("v"))); + assertThat(map.containsKey("present"), is(true)); + assertThat(map.containsKey("absent"), is(false)); + } + + public void testKeySet() { + var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + assertThat(map.keySet(), equalTo(java.util.Set.of("a", "b"))); + } + + public void testEntrySetValuesAreLastValues() { + var map = ParameterMap.of(Map.of("k", List.of("first", "last"))); + var entry = map.entrySet().iterator().next(); + assertThat(entry.getKey(), equalTo("k")); + assertThat(entry.getValue(), equalTo("last")); + } + + public void testEntrySetRemove() { + var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + var it = map.entrySet().iterator(); + it.next(); + it.remove(); + assertThat(map.size(), equalTo(1)); + } +} diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index e8eb201958f93..8fa9ce09d345e 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -121,21 +121,21 @@ public void testDecodeQueryStringMultiBasic() { String uri = "something?test=value"; var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(1)); - assertThat(params.get("test"), equalTo(List.of("value"))); + assertThat(params.getAll("test"), equalTo(List.of("value"))); } public void testDecodeQueryStringMultiMultipleValues() { String uri = "something?match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"; var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); - assertThat(params.get("match[]"), equalTo(List.of("up", "http_requests_total"))); - assertThat(params.get("start"), equalTo(List.of("1609746000"))); + assertThat(params.getAll("match[]"), equalTo(List.of("up", "http_requests_total"))); + assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); } public void testDecodeQueryStringMultiDelimiters() { String uri = Strings.format("something?a=1%cb=2", randomDelimiter()); var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); - assertThat(params.get("a"), equalTo(List.of("1"))); - assertThat(params.get("b"), equalTo(List.of("2"))); + assertThat(params.getAll("a"), equalTo(List.of("1"))); + assertThat(params.getAll("b"), equalTo(List.of("2"))); } public void testDecodeQueryStringMultiEdgeCases() { @@ -155,30 +155,27 @@ public void testDecodeQueryStringMultiEdgeCases() { // key with no value uri = "something?a"; var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); - assertThat(params.get("a"), equalTo(List.of(""))); + assertThat(params.getAll("a"), equalTo(List.of(""))); } public void testDecodeQueryStringMultiFragment() { // fragment should be excluded String uri = "something?a=1#fragment"; var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); - assertThat(params.get("a"), equalTo(List.of("1"))); + assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.containsKey("fragment"), is(false)); } public void testDecodeQueryStringMultiUrlEncoded() { String uri = "something?match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"; var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); - assertThat(params.get("match[]"), equalTo(List.of("up{job=\"prometheus\"}"))); + assertThat(params.getAll("match[]"), equalTo(List.of("up{job=\"prometheus\"}"))); } public void testDecodeQueryStringMultiReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { String uri = "something?" + reservedParam + "=value"; - expectThrows( - IllegalArgumentException.class, - () -> RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1) - ); + expectThrows(IllegalArgumentException.class, () -> RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1)); } } diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java index 1226be6403bcf..e463fc1d3fb20 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java @@ -34,7 +34,6 @@ import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -137,8 +136,7 @@ public void handle(final HttpExchange exchange) throws IOException { try { if (Regex.simpleMatch("PUT /" + account + "/" + container + "/*blockid=*", request)) { // Put Block (https://docs.microsoft.com/en-us/rest/api/storageservices/put-block) - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0, params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert assertValidBlockId(blockId); @@ -273,8 +271,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("GET /" + account + "/" + container + "?*restype=container*comp=list*", request)) { // List Blobs (https://docs.microsoft.com/en-us/rest/api/storageservices/list-blobs) - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0, params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getQuery(), 0); final StringBuilder list = new StringBuilder(); list.append(""" diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java index 570757ef17b8a..8002d06d7cb72 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java @@ -28,8 +28,6 @@ import java.io.InputStreamReader; import java.io.StringWriter; import java.nio.charset.StandardCharsets; -import java.util.HashMap; -import java.util.Map; @SuppressForbidden(reason = "Uses a HttpServer to emulate an Azure endpoint") public class AzureOAuthTokenServiceHttpHandler implements HttpHandler { @@ -68,8 +66,7 @@ public void handle(HttpExchange exchange) throws IOException { && ("/" + tenantId + "/oauth2/v2.0/token").equals(exchange.getRequestURI().getPath())) { final String requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), StandardCharsets.UTF_8)); - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(requestBody, 0, params); + final var params = RestUtils.decodeQueryStringMulti(requestBody, 0); if (clientId.equals(params.get("client_id")) && federatedToken.equals(params.get("client_assertion")) diff --git a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java index bfa361ac6d09f..5b66e1e8189ba 100644 --- a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java +++ b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java @@ -103,8 +103,7 @@ public void handle(final HttpExchange exchange) throws IOException { writeBlobVersionAsJson(exchange, blob); } else if (Regex.simpleMatch("GET /storage/v1/b/" + bucket + "/o*", request)) { // List Objects https://cloud.google.com/storage/docs/json_api/v1/objects/list - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI(), params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); final String prefix = params.getOrDefault("prefix", ""); final int maxResults = Integer.parseInt(params.getOrDefault("maxResults", String.valueOf(defaultPageLimit.get()))); final String delimiter = params.getOrDefault("delimiter", ""); @@ -215,8 +214,7 @@ public void handle(final HttpExchange exchange) throws IOException { } } else if (Regex.simpleMatch("POST /upload/storage/v1/b/" + bucket + "/*uploadType=resumable*", request)) { // Resumable upload initialization https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI(), params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); final String blobName = params.get("name"); final Long ifGenerationMatch = parseOptionalLongParameter(exchange, IF_GENERATION_MATCH); final MockGcsBlobStore.ResumableUpload resumableUpload = mockGcsBlobStore.createResumableUpload( @@ -242,8 +240,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("PUT /upload/storage/v1/b/" + bucket + "/o?*uploadType=resumable*", request)) { // Resumable upload https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI(), params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); final String contentRangeValue = requireHeader(exchange, "Content-Range"); final HttpHeaderParser.ContentRange contentRange = HttpHeaderParser.parseContentRangeHeader(contentRangeValue); @@ -471,8 +468,7 @@ private static String requireHeader(HttpExchange exchange, String headerName) { } private static Long parseOptionalLongParameter(HttpExchange exchange, String parameterName) { - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI(), params); + final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); if (params.containsKey(parameterName)) { try { return Long.parseLong(params.get(parameterName)); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java index e55387a715d97..3afccce9d46bf 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java @@ -19,6 +19,7 @@ import org.elasticsearch.http.HttpRequest; import org.elasticsearch.http.HttpResponse; import org.elasticsearch.rest.ChunkedRestResponseBodyPart; +import org.elasticsearch.rest.ParameterMap; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -37,17 +38,12 @@ public FakeRestRequest() { this( XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE), new FakeHttpRequest(Method.GET, "", BytesArray.EMPTY, new HashMap<>()), - new HashMap<>(), + ParameterMap.empty(), new FakeHttpChannel(null) ); } - private FakeRestRequest( - XContentParserConfiguration config, - HttpRequest httpRequest, - Map params, - HttpChannel httpChannel - ) { + private FakeRestRequest(XContentParserConfiguration config, HttpRequest httpRequest, ParameterMap params, HttpChannel httpChannel) { super(config, params, httpRequest.uri(), httpRequest.getHeaders(), httpRequest, httpChannel); } @@ -207,7 +203,7 @@ public static class Builder { private Map> headers = new HashMap<>(); - private Map params = new HashMap<>(); + private ParameterMap params = ParameterMap.empty(); private HttpBody content = HttpBody.empty(); @@ -230,7 +226,14 @@ public Builder withHeaders(Map> headers) { } public Builder withParams(Map params) { - this.params = params; + if (params != null) { + this.params = ParameterMap.fromSingleValues(params); + } + return this; + } + + public Builder withMultiParams(ParameterMap multiParams) { + this.params = multiParams; return this; } diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java index 1f0151e8b10b1..6b5d02bfa8424 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java @@ -125,8 +125,7 @@ public void processQueryString(String queryString, ActionListener parameters = new HashMap<>(); - RestUtils.decodeQueryString(queryString, 0, parameters); + final var parameters = RestUtils.decodeQueryStringMulti(queryString, 0); if (parameters.isEmpty()) { throw new ElasticsearchSecurityException("Invalid Authentication Request query string (zero parameters)"); } diff --git a/x-pack/plugin/monitoring/src/internalClusterTest/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java b/x-pack/plugin/monitoring/src/internalClusterTest/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java index 5250a1f764e5c..df1c25abbb30f 100644 --- a/x-pack/plugin/monitoring/src/internalClusterTest/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java +++ b/x-pack/plugin/monitoring/src/internalClusterTest/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java @@ -514,11 +514,9 @@ private void assertMonitorTemplates( } private void assertMonitorVersionQueryString(String query, final Map parameters) { - Map expectedQueryStringMap = new HashMap<>(); - RestUtils.decodeQueryString(query, 0, expectedQueryStringMap); + var expectedQueryStringMap = RestUtils.decodeQueryStringMulti(query, 0); - Map resourceVersionQueryStringMap = new HashMap<>(); - RestUtils.decodeQueryString(resourceVersionQueryString(), 0, resourceVersionQueryStringMap); + var resourceVersionQueryStringMap = RestUtils.decodeQueryStringMulti(resourceVersionQueryString(), 0); Map actualQueryStringMap = new HashMap<>(); actualQueryStringMap.putAll(resourceVersionQueryStringMap); diff --git a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java index 12c57764916e6..7422ad4d5ae85 100644 --- a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java +++ b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java @@ -32,7 +32,6 @@ import java.nio.file.Path; import java.security.SecureRandom; import java.security.cert.Certificate; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -123,8 +122,7 @@ private void registerGetAccessTokenHandler() { } final var requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), Charset.defaultCharset())); - final var formFields = new HashMap(); - RestUtils.decodeQueryString(requestBody, 0, formFields); + final var formFields = RestUtils.decodeQueryStringMulti(requestBody, 0); if (formFields.get("grant_type").equals("client_credentials") == false) { graphError(exchange, RestStatus.BAD_REQUEST, Strings.format("Unexpected Grant Type: %s", formFields.get("grant_type"))); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index b118bcef25207..0228c56bf8576 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -63,9 +63,7 @@ import java.util.Base64; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.zip.Inflater; @@ -398,8 +396,7 @@ protected void validateNotOnOrAfter(Instant notOnOrAfter) { protected ParsedQueryString parseQueryStringAndValidateSignature(String queryString, String samlMessageParameterName) { final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); - final Map parameters = new HashMap<>(); - RestUtils.decodeQueryString(queryString, 0, parameters); + final var parameters = RestUtils.decodeQueryStringMulti(queryString, 0); final String samlMessage = parameters.get(samlMessageParameterName); if (samlMessage == null) { throw samlException("Could not parse {} from query string: [{}]", samlMessageParameterName, queryString); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 273f8e4c111e3..1d8a933cb8750 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -42,7 +42,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -358,8 +357,7 @@ public void testBuildLogoutResponse() throws Exception { final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final Map parameters = new HashMap<>(); - RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1, parameters); + final var parameters = RestUtils.decodeQueryStringMulti(endSessionUrl, endSessionUrl.indexOf("?") + 1); assertThat(parameters, aMapWithSize(3)); assertThat(parameters, hasKey("id_token_hint")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -382,8 +380,7 @@ public void testBuildLogoutResponseFromEndsessionEndpointWithExistingParameters( final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final Map parameters = new HashMap<>(); - RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1, parameters); + final var parameters = RestUtils.decodeQueryStringMulti(endSessionUrl, endSessionUrl.indexOf("?") + 1); assertThat(parameters, aMapWithSize(4)); assertThat(parameters, hasKey("parameter")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -456,11 +453,9 @@ private void assertEqualUrlStrings(String actual, String expected) { assertThat(endOfPath, greaterThan(-1)); assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1))); - final HashMap actualParams = new HashMap<>(); - RestUtils.decodeQueryString(actual, endOfPath + 1, actualParams); + final var actualParams = RestUtils.decodeQueryStringMulti(actual, endOfPath + 1); - final HashMap expectedParams = new HashMap<>(); - RestUtils.decodeQueryString(expected, endOfPath + 1, expectedParams); + final var expectedParams = RestUtils.decodeQueryStringMulti(expected, endOfPath + 1); assertThat(actualParams, equalTo(expectedParams)); } diff --git a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java index cb1e588dc220e..a571909b0d6b9 100644 --- a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java +++ b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java @@ -31,7 +31,6 @@ import java.io.IOException; import java.net.URI; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -139,8 +138,7 @@ public void testAuthenticateWithOidcIssuedJwt() throws Exception { * The three-part-encoded JWT id_token will be in the "id_token" field */ final int hashChar = implicitFlowURI.indexOf('#'); - final Map hashParams = new HashMap<>(); - RestUtils.decodeQueryString(implicitFlowURI.substring(hashChar + 1), 0, hashParams); + final var hashParams = RestUtils.decodeQueryStringMulti(implicitFlowURI.substring(hashChar + 1), 0); assertThat("Hash value of URI [" + implicitFlowURI + "] should be a JWT with an id Token", hashParams, hasKey("id_token")); String idJwt = hashParams.get("id_token"); From 2e10f206960490ed50ed4468f6b977b603b7c7b9 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 11:06:56 +0100 Subject: [PATCH 04/29] Use decodeQueryStringMulti in Watcher HttpRequest/Template fromUrl Replaces the decodeQueryString + local HashMap pattern in both HttpRequest.Builder and HttpRequestTemplate.Builder fromUrl() with decodeQueryStringMulti, keeping all Map signatures unchanged. --- .../xpack/watcher/common/http/HttpRequest.java | 2 +- .../xpack/watcher/common/http/HttpRequestTemplate.java | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java index 4ef46374fba0f..35cd929351ae3 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java @@ -560,7 +560,7 @@ public Builder fromUrl(String supposedUrl) { } String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - RestUtils.decodeQueryString(rawQuery, 0, params); + setParams(RestUtils.decodeQueryStringMulti(rawQuery, 0)); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java index 389278cb88398..5c41923b6f28c 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java @@ -542,11 +542,7 @@ public Builder fromUrl(String supposedUrl) { String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - Map stringParams = new HashMap<>(); - RestUtils.decodeQueryString(rawQuery, 0, stringParams); - for (Map.Entry entry : stringParams.entrySet()) { - params.put(entry.getKey(), new TextTemplate(entry.getValue())); - } + RestUtils.decodeQueryStringMulti(rawQuery, 0).forEach((k, v) -> params.put(k, new TextTemplate(v))); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); From 36e3c24a578d85604649028b7ae21c837ccad427 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 11:26:31 +0100 Subject: [PATCH 05/29] Remove decodeQueryString(Map) and drop Multi suffix from RestUtils --- .../azure/AzureBlobContainerRetriesTests.java | 4 +- ...CloudStorageBlobContainerRetriesTests.java | 2 +- .../org/elasticsearch/rest/RestRequest.java | 2 +- .../org/elasticsearch/rest/RestUtils.java | 15 +--- .../elasticsearch/rest/RestUtilsTests.java | 88 +++++++------------ .../java/fixture/azure/AzureHttpHandler.java | 4 +- .../AzureOAuthTokenServiceHttpHandler.java | 2 +- .../gcs/GoogleCloudStorageHttpHandler.java | 8 +- .../saml/authn/SamlAuthnRequestValidator.java | 2 +- .../exporter/http/HttpExporterIT.java | 4 +- .../microsoft/MicrosoftGraphHttpFixture.java | 2 +- .../authc/saml/SamlObjectHandler.java | 2 +- .../authc/oidc/OpenIdConnectRealmTests.java | 8 +- .../watcher/common/http/HttpRequest.java | 2 +- .../common/http/HttpRequestTemplate.java | 2 +- .../security/authc/jwt/JwtWithOidcAuthIT.java | 2 +- 16 files changed, 59 insertions(+), 90 deletions(-) diff --git a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java index f19ec98e81724..45fbb6f24edda 100644 --- a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java +++ b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java @@ -368,7 +368,7 @@ public void testWriteLargeBlob() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); @@ -437,7 +437,7 @@ public void testWriteLargeBlobStreaming() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob_streaming"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); diff --git a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java index 1fd0acc3839f3..9b34029829c75 100644 --- a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java +++ b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java @@ -420,7 +420,7 @@ public void testWriteLargeBlob() throws IOException { httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> { final BytesReference requestBody = Streams.readFully(exchange.getRequestBody()); - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getQuery(), 0); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0); assertThat(params.get("uploadType"), equalTo("resumable")); if ("POST".equals(exchange.getRequestMethod())) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index dc7e02a45c160..8152bb6f2a549 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -214,7 +214,7 @@ private static ParameterMap params(final String uri) { int index = uri.indexOf('?'); if (index >= 0) { try { - return RestUtils.decodeQueryStringMulti(uri, index + 1); + return RestUtils.decodeQueryString(uri, index + 1); } catch (final IllegalArgumentException e) { throw new BadParameterException(e); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index f362f04917e89..2dcaab26ebbb4 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -45,10 +45,6 @@ public class RestUtils { public static final UnaryOperator REST_DECODER = RestUtils::decodeComponent; - public static void decodeQueryString(String s, int fromIndex, Map params) { - parseQueryStringPairs(s, fromIndex, (name, value) -> addParam(params, name, value)); - } - /** * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). @@ -56,9 +52,9 @@ public static void decodeQueryString(String s, int fromIndex, Map> result = new LinkedHashMap<>(); parseQueryStringPairs(s, fromIndex, (name, value) -> { checkReservedParam(name); @@ -136,11 +132,6 @@ private static void checkReservedParam(String name) { } } - private static void addParam(Map params, String name, String value) { - checkReservedParam(name); - params.put(name, value); - } - /** * Decodes a bit of an URL encoded by a browser. *

diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index 8fa9ce09d345e..07bf982641b1e 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -32,84 +31,65 @@ static char randomDelimiter() { } public void testDecodeQueryString() { - Map params = new HashMap<>(); - String uri = "something?test=value"; - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(1)); assertThat(params.get("test"), equalTo("value")); - params.clear(); uri = Strings.format("something?test=value%ctest1=value1", randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(2)); assertThat(params.get("test"), equalTo("value")); assertThat(params.get("test1"), equalTo("value1")); - params.clear(); uri = "something"; - RestUtils.decodeQueryString(uri, uri.length(), params); + params = RestUtils.decodeQueryString(uri, uri.length()); assertThat(params.size(), equalTo(0)); - params.clear(); uri = "something"; - RestUtils.decodeQueryString(uri, -1, params); + params = RestUtils.decodeQueryString(uri, -1); assertThat(params.size(), equalTo(0)); } public void testDecodeQueryStringEdgeCases() { - Map params = new HashMap<>(); - String uri = "something?"; - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); - assertThat(params.size(), equalTo(0)); + assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); - params.clear(); uri = Strings.format("something?%c", randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); - assertThat(params.size(), equalTo(0)); + assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); - params.clear(); uri = Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(2)); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - params.clear(); uri = "something?="; - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); - assertThat(params.size(), equalTo(0)); + assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); - params.clear(); uri = Strings.format("something?%c=", randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); - assertThat(params.size(), equalTo(0)); + assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); - params.clear(); uri = "something?a"; - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(1)); assertThat(params.get("a"), equalTo("")); - params.clear(); uri = Strings.format("something?p=v%ca", randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(2)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); - params.clear(); uri = Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(3)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - params.clear(); uri = Strings.format("something?p=v%ca%cb%cp1=v1", randomDelimiter(), randomDelimiter(), randomDelimiter()); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(4)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("b"), equalTo("")); @@ -117,65 +97,65 @@ public void testDecodeQueryStringEdgeCases() { assertThat(params.get("p1"), equalTo("v1")); } - public void testDecodeQueryStringMultiBasic() { + public void testDecodeQueryStringBasic() { String uri = "something?test=value"; - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.size(), equalTo(1)); assertThat(params.getAll("test"), equalTo(List.of("value"))); } - public void testDecodeQueryStringMultiMultipleValues() { + public void testDecodeQueryStringMultipleValues() { String uri = "something?match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"; - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.getAll("match[]"), equalTo(List.of("up", "http_requests_total"))); assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); } - public void testDecodeQueryStringMultiDelimiters() { + public void testDecodeQueryStringDelimiters() { String uri = Strings.format("something?a=1%cb=2", randomDelimiter()); - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.getAll("b"), equalTo(List.of("2"))); } - public void testDecodeQueryStringMultiEdgeCases() { + public void testDecodeQueryStringEdgeCasesMulti() { // empty query string String uri = "something?"; - assertThat(RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1).isEmpty(), is(true)); + assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).isEmpty(), is(true)); // fromIndex past end - assertThat(RestUtils.decodeQueryStringMulti("something", 9).isEmpty(), is(true)); + assertThat(RestUtils.decodeQueryString("something", 9).isEmpty(), is(true)); // fromIndex negative - assertThat(RestUtils.decodeQueryStringMulti("something", -1).isEmpty(), is(true)); + assertThat(RestUtils.decodeQueryString("something", -1).isEmpty(), is(true)); // empty string - assertThat(RestUtils.decodeQueryStringMulti("", 0).isEmpty(), is(true)); + assertThat(RestUtils.decodeQueryString("", 0).isEmpty(), is(true)); // key with no value uri = "something?a"; - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.getAll("a"), equalTo(List.of(""))); } - public void testDecodeQueryStringMultiFragment() { + public void testDecodeQueryStringFragment() { // fragment should be excluded String uri = "something?a=1#fragment"; - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.containsKey("fragment"), is(false)); } - public void testDecodeQueryStringMultiUrlEncoded() { + public void testDecodeQueryStringUrlEncoded() { String uri = "something?match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"; - var params = RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.getAll("match[]"), equalTo(List.of("up{job=\"prometheus\"}"))); } - public void testDecodeQueryStringMultiReservedParameters() { + public void testDecodeQueryStringReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { String uri = "something?" + reservedParam + "=value"; - expectThrows(IllegalArgumentException.class, () -> RestUtils.decodeQueryStringMulti(uri, uri.indexOf('?') + 1)); + expectThrows(IllegalArgumentException.class, () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1)); } } @@ -207,7 +187,6 @@ public void testCorsSettingIsARegex() { public void testCrazyURL() { String host = "example.com"; - Map params = new HashMap<>(); // This is a valid URL String uri = String.format( @@ -218,18 +197,17 @@ public void testCrazyURL() { randomDelimiter(), randomDelimiter() ); - RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params); + var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); assertThat(params.get("/?:@-._~!$'()* ,"), equalTo("/?:@-._~!$'()* ,==")); assertThat(params.size(), equalTo(1)); } public void testReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { - Map params = new HashMap<>(); String uri = "something?" + reservedParam + "=value"; IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params) + () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1) ); assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); } diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java index e463fc1d3fb20..0d08975b48437 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java @@ -136,7 +136,7 @@ public void handle(final HttpExchange exchange) throws IOException { try { if (Regex.simpleMatch("PUT /" + account + "/" + container + "/*blockid=*", request)) { // Put Block (https://docs.microsoft.com/en-us/rest/api/storageservices/put-block) - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getRawQuery(), 0); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); final String blockId = params.get("blockid"); assert assertValidBlockId(blockId); @@ -271,7 +271,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("GET /" + account + "/" + container + "?*restype=container*comp=list*", request)) { // List Blobs (https://docs.microsoft.com/en-us/rest/api/storageservices/list-blobs) - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI().getQuery(), 0); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0); final StringBuilder list = new StringBuilder(); list.append(""" diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java index 8002d06d7cb72..85278bd60821d 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java @@ -66,7 +66,7 @@ public void handle(HttpExchange exchange) throws IOException { && ("/" + tenantId + "/oauth2/v2.0/token").equals(exchange.getRequestURI().getPath())) { final String requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), StandardCharsets.UTF_8)); - final var params = RestUtils.decodeQueryStringMulti(requestBody, 0); + final var params = RestUtils.decodeQueryString(requestBody, 0); if (clientId.equals(params.get("client_id")) && federatedToken.equals(params.get("client_assertion")) diff --git a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java index 5b66e1e8189ba..71b72feec1adb 100644 --- a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java +++ b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java @@ -103,7 +103,7 @@ public void handle(final HttpExchange exchange) throws IOException { writeBlobVersionAsJson(exchange, blob); } else if (Regex.simpleMatch("GET /storage/v1/b/" + bucket + "/o*", request)) { // List Objects https://cloud.google.com/storage/docs/json_api/v1/objects/list - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); final String prefix = params.getOrDefault("prefix", ""); final int maxResults = Integer.parseInt(params.getOrDefault("maxResults", String.valueOf(defaultPageLimit.get()))); final String delimiter = params.getOrDefault("delimiter", ""); @@ -214,7 +214,7 @@ public void handle(final HttpExchange exchange) throws IOException { } } else if (Regex.simpleMatch("POST /upload/storage/v1/b/" + bucket + "/*uploadType=resumable*", request)) { // Resumable upload initialization https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); final String blobName = params.get("name"); final Long ifGenerationMatch = parseOptionalLongParameter(exchange, IF_GENERATION_MATCH); final MockGcsBlobStore.ResumableUpload resumableUpload = mockGcsBlobStore.createResumableUpload( @@ -240,7 +240,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("PUT /upload/storage/v1/b/" + bucket + "/o?*uploadType=resumable*", request)) { // Resumable upload https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); final String contentRangeValue = requireHeader(exchange, "Content-Range"); final HttpHeaderParser.ContentRange contentRange = HttpHeaderParser.parseContentRangeHeader(contentRangeValue); @@ -468,7 +468,7 @@ private static String requireHeader(HttpExchange exchange, String headerName) { } private static Long parseOptionalLongParameter(HttpExchange exchange, String parameterName) { - final var params = RestUtils.decodeQueryStringMulti(exchange.getRequestURI()); + final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); if (params.containsKey(parameterName)) { try { return Long.parseLong(params.get(parameterName)); diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java index 6b5d02bfa8424..1b2111f686896 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java @@ -125,7 +125,7 @@ public void processQueryString(String queryString, ActionListener parameters) { - var expectedQueryStringMap = RestUtils.decodeQueryStringMulti(query, 0); + var expectedQueryStringMap = RestUtils.decodeQueryString(query, 0); - var resourceVersionQueryStringMap = RestUtils.decodeQueryStringMulti(resourceVersionQueryString(), 0); + var resourceVersionQueryStringMap = RestUtils.decodeQueryString(resourceVersionQueryString(), 0); Map actualQueryStringMap = new HashMap<>(); actualQueryStringMap.putAll(resourceVersionQueryStringMap); diff --git a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java index 7422ad4d5ae85..30993b677371d 100644 --- a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java +++ b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java @@ -122,7 +122,7 @@ private void registerGetAccessTokenHandler() { } final var requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), Charset.defaultCharset())); - final var formFields = RestUtils.decodeQueryStringMulti(requestBody, 0); + final var formFields = RestUtils.decodeQueryString(requestBody, 0); if (formFields.get("grant_type").equals("client_credentials") == false) { graphError(exchange, RestStatus.BAD_REQUEST, Strings.format("Unexpected Grant Type: %s", formFields.get("grant_type"))); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index 0228c56bf8576..1c088e9a15398 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -396,7 +396,7 @@ protected void validateNotOnOrAfter(Instant notOnOrAfter) { protected ParsedQueryString parseQueryStringAndValidateSignature(String queryString, String samlMessageParameterName) { final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); - final var parameters = RestUtils.decodeQueryStringMulti(queryString, 0); + final var parameters = RestUtils.decodeQueryString(queryString, 0); final String samlMessage = parameters.get(samlMessageParameterName); if (samlMessage == null) { throw samlException("Could not parse {} from query string: [{}]", samlMessageParameterName, queryString); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 1d8a933cb8750..201d9392d3a27 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -357,7 +357,7 @@ public void testBuildLogoutResponse() throws Exception { final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RestUtils.decodeQueryStringMulti(endSessionUrl, endSessionUrl.indexOf("?") + 1); + final var parameters = RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1); assertThat(parameters, aMapWithSize(3)); assertThat(parameters, hasKey("id_token_hint")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -380,7 +380,7 @@ public void testBuildLogoutResponseFromEndsessionEndpointWithExistingParameters( final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RestUtils.decodeQueryStringMulti(endSessionUrl, endSessionUrl.indexOf("?") + 1); + final var parameters = RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1); assertThat(parameters, aMapWithSize(4)); assertThat(parameters, hasKey("parameter")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -453,9 +453,9 @@ private void assertEqualUrlStrings(String actual, String expected) { assertThat(endOfPath, greaterThan(-1)); assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1))); - final var actualParams = RestUtils.decodeQueryStringMulti(actual, endOfPath + 1); + final var actualParams = RestUtils.decodeQueryString(actual, endOfPath + 1); - final var expectedParams = RestUtils.decodeQueryStringMulti(expected, endOfPath + 1); + final var expectedParams = RestUtils.decodeQueryString(expected, endOfPath + 1); assertThat(actualParams, equalTo(expectedParams)); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java index 35cd929351ae3..601be19322c4b 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java @@ -560,7 +560,7 @@ public Builder fromUrl(String supposedUrl) { } String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - setParams(RestUtils.decodeQueryStringMulti(rawQuery, 0)); + setParams(RestUtils.decodeQueryString(rawQuery, 0)); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java index 5c41923b6f28c..4e8864eabe0be 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java @@ -542,7 +542,7 @@ public Builder fromUrl(String supposedUrl) { String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - RestUtils.decodeQueryStringMulti(rawQuery, 0).forEach((k, v) -> params.put(k, new TextTemplate(v))); + RestUtils.decodeQueryString(rawQuery, 0).forEach((k, v) -> params.put(k, new TextTemplate(v))); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java index a571909b0d6b9..10f6432f2a305 100644 --- a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java +++ b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java @@ -138,7 +138,7 @@ public void testAuthenticateWithOidcIssuedJwt() throws Exception { * The three-part-encoded JWT id_token will be in the "id_token" field */ final int hashChar = implicitFlowURI.indexOf('#'); - final var hashParams = RestUtils.decodeQueryStringMulti(implicitFlowURI.substring(hashChar + 1), 0); + final var hashParams = RestUtils.decodeQueryString(implicitFlowURI.substring(hashChar + 1), 0); assertThat("Hash value of URI [" + implicitFlowURI + "] should be a JWT with an id Token", hashParams, hasKey("id_token")); String idJwt = hashParams.get("id_token"); From 8804194ce2f24bdf73bc2468a85de4f01100d613 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 11:47:59 +0100 Subject: [PATCH 06/29] Move query-string parsing to ParameterMap as fromQueryString/fromUrl/from factory methods --- .../azure/AzureBlobContainerRetriesTests.java | 5 +- ...CloudStorageBlobContainerRetriesTests.java | 3 +- .../org/elasticsearch/rest/ParameterMap.java | 36 +++++++ .../org/elasticsearch/rest/RestRequest.java | 12 +-- .../org/elasticsearch/rest/RestUtils.java | 15 +-- .../elasticsearch/rest/RestUtilsTests.java | 94 +++++++------------ .../java/fixture/azure/AzureHttpHandler.java | 4 +- .../AzureOAuthTokenServiceHttpHandler.java | 3 +- .../gcs/GoogleCloudStorageHttpHandler.java | 9 +- .../saml/authn/SamlAuthnRequestValidator.java | 3 +- .../exporter/http/HttpExporterIT.java | 5 +- .../microsoft/MicrosoftGraphHttpFixture.java | 3 +- .../authc/saml/SamlObjectHandler.java | 3 +- .../authc/oidc/OpenIdConnectRealmTests.java | 9 +- .../watcher/common/http/HttpRequest.java | 3 +- .../common/http/HttpRequestTemplate.java | 3 +- .../security/authc/jwt/JwtWithOidcAuthIT.java | 3 +- 17 files changed, 96 insertions(+), 117 deletions(-) diff --git a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java index 45fbb6f24edda..24c16fe56298e 100644 --- a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java +++ b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java @@ -47,7 +47,6 @@ import org.elasticsearch.repositories.RepositoriesMetrics; import org.elasticsearch.repositories.blobstore.AbstractBlobContainerRetriesTestCase; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.fixture.HttpHeaderParser; import org.elasticsearch.threadpool.TestThreadPool; @@ -368,7 +367,7 @@ public void testWriteLargeBlob() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); + final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); @@ -437,7 +436,7 @@ public void testWriteLargeBlobStreaming() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob_streaming"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); + final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); diff --git a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java index 9b34029829c75..eaa31df83bdcf 100644 --- a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java +++ b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java @@ -56,7 +56,6 @@ import org.elasticsearch.repositories.blobstore.AbstractBlobContainerRetriesTestCase; import org.elasticsearch.repositories.blobstore.ESMockAPIBasedRepositoryIntegTestCase; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.fixture.HttpHeaderParser; import org.threeten.bp.Duration; @@ -420,7 +419,7 @@ public void testWriteLargeBlob() throws IOException { httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> { final BytesReference requestBody = Streams.readFully(exchange.getRequestBody()); - final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0); + final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getQuery()); assertThat(params.get("uploadType"), equalTo("resumable")); if ("POST".equals(exchange.getRequestMethod())) { diff --git a/server/src/main/java/org/elasticsearch/rest/ParameterMap.java b/server/src/main/java/org/elasticsearch/rest/ParameterMap.java index 5941309b6a832..11488116a8d26 100644 --- a/server/src/main/java/org/elasticsearch/rest/ParameterMap.java +++ b/server/src/main/java/org/elasticsearch/rest/ParameterMap.java @@ -9,6 +9,7 @@ package org.elasticsearch.rest; +import java.net.URI; import java.util.AbstractMap; import java.util.AbstractSet; import java.util.ArrayList; @@ -57,6 +58,41 @@ public static ParameterMap of(Map> multiValues) { return new ParameterMap(multiValues); } + /** + * Parses a URL-encoded query string into a {@code ParameterMap}, preserving all values for + * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). + * + * @param queryString the raw query string (the part after {@code ?}, without the {@code ?} itself) + * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + */ + public static ParameterMap fromQueryString(String queryString) { + return RestUtils.decodeQueryString(queryString, 0); + } + + /** + * Parses the query string from a full URL string into a {@code ParameterMap}, preserving all + * values for repeated parameters. Returns an empty map if the URL contains no {@code ?}. + * + * @param url a full URL string, e.g. {@code /index/_search?pretty&size=10} + * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + */ + public static ParameterMap fromUrl(String url) { + int index = url.indexOf('?'); + return index >= 0 ? RestUtils.decodeQueryString(url, index + 1) : ParameterMap.empty(); + } + + /** + * Parses the query string from a {@link URI} into a {@code ParameterMap}, preserving all + * values for repeated parameters. Returns an empty map if the URI has no query. + * + * @param uri the URI whose raw query string is parsed + * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + */ + public static ParameterMap from(URI uri) { + final var rawQuery = uri.getRawQuery(); + return rawQuery != null && rawQuery.isEmpty() == false ? fromQueryString(rawQuery) : ParameterMap.empty(); + } + /** * Creates a {@code ParameterMap} from a plain single-value map. * Each value is wrapped in a singleton list so that {@link #getAll(String)} returns a diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 8152bb6f2a549..4c592ae6c119e 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -211,15 +211,11 @@ public static RestRequest request(XContentParserConfiguration parserConfig, Http } private static ParameterMap params(final String uri) { - int index = uri.indexOf('?'); - if (index >= 0) { - try { - return RestUtils.decodeQueryString(uri, index + 1); - } catch (final IllegalArgumentException e) { - throw new BadParameterException(e); - } + try { + return ParameterMap.fromUrl(uri); + } catch (final IllegalArgumentException e) { + throw new BadParameterException(e); } - return ParameterMap.empty(); } /** diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 2dcaab26ebbb4..bdfcf70d7e666 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -20,7 +20,6 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.UpdateForV10; -import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -45,18 +44,6 @@ public class RestUtils { public static final UnaryOperator REST_DECODER = RestUtils::decodeComponent; - /** - * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for - * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). - * - * @param uri the URI whose raw query string is parsed - * @return a {@link ParameterMap} from parameter name to all its values, in encounter order - */ - public static ParameterMap decodeQueryString(URI uri) { - final var rawQuery = uri.getRawQuery(); - return Strings.hasLength(rawQuery) ? decodeQueryString(rawQuery, 0) : ParameterMap.empty(); - } - /** * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). @@ -65,7 +52,7 @@ public static ParameterMap decodeQueryString(URI uri) { * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) * @return a {@link ParameterMap} from parameter name to all its values, in encounter order */ - public static ParameterMap decodeQueryString(String s, int fromIndex) { + static ParameterMap decodeQueryString(String s, int fromIndex) { Map> result = new LinkedHashMap<>(); parseQueryStringPairs(s, fromIndex, (name, value) -> { checkReservedParam(name); diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index 07bf982641b1e..ad9c00afb6207 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -30,66 +30,53 @@ static char randomDelimiter() { return randomBoolean() ? '&' : ';'; } - public void testDecodeQueryString() { - String uri = "something?test=value"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + public void testDecodeQueryStringFromUrl() { + var params = ParameterMap.fromUrl("something?test=value"); assertThat(params.size(), equalTo(1)); assertThat(params.get("test"), equalTo("value")); - uri = Strings.format("something?test=value%ctest1=value1", randomDelimiter()); - params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + params = ParameterMap.fromUrl(Strings.format("something?test=value%ctest1=value1", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("test"), equalTo("value")); assertThat(params.get("test1"), equalTo("value1")); - uri = "something"; - params = RestUtils.decodeQueryString(uri, uri.length()); - assertThat(params.size(), equalTo(0)); - - uri = "something"; - params = RestUtils.decodeQueryString(uri, -1); - assertThat(params.size(), equalTo(0)); + // no query string + assertThat(ParameterMap.fromUrl("something").isEmpty(), is(true)); } - public void testDecodeQueryStringEdgeCases() { - String uri = "something?"; - assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); + public void testDecodeQueryStringFromUrlEdgeCases() { + // empty query string + assertThat(ParameterMap.fromUrl("something?").size(), equalTo(0)); - uri = Strings.format("something?%c", randomDelimiter()); - assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); + assertThat(ParameterMap.fromUrl(Strings.format("something?%c", randomDelimiter())).size(), equalTo(0)); - uri = Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter()); - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromUrl(Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - uri = "something?="; - assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); + assertThat(ParameterMap.fromUrl("something?=").size(), equalTo(0)); - uri = Strings.format("something?%c=", randomDelimiter()); - assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).size(), equalTo(0)); + assertThat(ParameterMap.fromUrl(Strings.format("something?%c=", randomDelimiter())).size(), equalTo(0)); - uri = "something?a"; - params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + params = ParameterMap.fromUrl("something?a"); assertThat(params.size(), equalTo(1)); assertThat(params.get("a"), equalTo("")); - uri = Strings.format("something?p=v%ca", randomDelimiter()); - params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + params = ParameterMap.fromUrl(Strings.format("something?p=v%ca", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); - uri = Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter()); - params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + params = ParameterMap.fromUrl(Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(3)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - uri = Strings.format("something?p=v%ca%cb%cp1=v1", randomDelimiter(), randomDelimiter(), randomDelimiter()); - params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + params = ParameterMap.fromUrl( + Strings.format("something?p=v%ca%cb%cp1=v1", randomDelimiter(), randomDelimiter(), randomDelimiter()) + ); assertThat(params.size(), equalTo(4)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("b"), equalTo("")); @@ -97,65 +84,51 @@ public void testDecodeQueryStringEdgeCases() { assertThat(params.get("p1"), equalTo("v1")); } - public void testDecodeQueryStringBasic() { - String uri = "something?test=value"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + public void testDecodeQueryString() { + var params = ParameterMap.fromQueryString("test=value"); assertThat(params.size(), equalTo(1)); assertThat(params.getAll("test"), equalTo(List.of("value"))); } public void testDecodeQueryStringMultipleValues() { - String uri = "something?match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromQueryString("match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"); assertThat(params.getAll("match[]"), equalTo(List.of("up", "http_requests_total"))); assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); } public void testDecodeQueryStringDelimiters() { - String uri = Strings.format("something?a=1%cb=2", randomDelimiter()); - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromQueryString(Strings.format("a=1%cb=2", randomDelimiter())); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.getAll("b"), equalTo(List.of("2"))); } - public void testDecodeQueryStringEdgeCasesMulti() { + public void testDecodeQueryStringEdgeCases() { // empty query string - String uri = "something?"; - assertThat(RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1).isEmpty(), is(true)); + assertThat(ParameterMap.fromQueryString("").isEmpty(), is(true)); - // fromIndex past end - assertThat(RestUtils.decodeQueryString("something", 9).isEmpty(), is(true)); + // key with no value + assertThat(ParameterMap.fromQueryString("a").getAll("a"), equalTo(List.of(""))); - // fromIndex negative + // package-private fromIndex edge cases + assertThat(RestUtils.decodeQueryString("something", 9).isEmpty(), is(true)); assertThat(RestUtils.decodeQueryString("something", -1).isEmpty(), is(true)); - - // empty string - assertThat(RestUtils.decodeQueryString("", 0).isEmpty(), is(true)); - - // key with no value - uri = "something?a"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); - assertThat(params.getAll("a"), equalTo(List.of(""))); } public void testDecodeQueryStringFragment() { // fragment should be excluded - String uri = "something?a=1#fragment"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromUrl("something?a=1#fragment"); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.containsKey("fragment"), is(false)); } public void testDecodeQueryStringUrlEncoded() { - String uri = "something?match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"; - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromQueryString("match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"); assertThat(params.getAll("match[]"), equalTo(List.of("up{job=\"prometheus\"}"))); } public void testDecodeQueryStringReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { - String uri = "something?" + reservedParam + "=value"; - expectThrows(IllegalArgumentException.class, () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1)); + expectThrows(IllegalArgumentException.class, () -> ParameterMap.fromQueryString(reservedParam + "=value")); } } @@ -197,17 +170,16 @@ public void testCrazyURL() { randomDelimiter(), randomDelimiter() ); - var params = RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1); + var params = ParameterMap.fromUrl(uri); assertThat(params.get("/?:@-._~!$'()* ,"), equalTo("/?:@-._~!$'()* ,==")); assertThat(params.size(), equalTo(1)); } public void testReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { - String uri = "something?" + reservedParam + "=value"; IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1) + () -> ParameterMap.fromUrl("something?" + reservedParam + "=value") ); assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); } diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java index 0d08975b48437..ba66b71783389 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java @@ -136,7 +136,7 @@ public void handle(final HttpExchange exchange) throws IOException { try { if (Regex.simpleMatch("PUT /" + account + "/" + container + "/*blockid=*", request)) { // Put Block (https://docs.microsoft.com/en-us/rest/api/storageservices/put-block) - final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getRawQuery(), 0); + final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert assertValidBlockId(blockId); @@ -271,7 +271,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("GET /" + account + "/" + container + "?*restype=container*comp=list*", request)) { // List Blobs (https://docs.microsoft.com/en-us/rest/api/storageservices/list-blobs) - final var params = RestUtils.decodeQueryString(exchange.getRequestURI().getQuery(), 0); + final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getQuery()); final StringBuilder list = new StringBuilder(); list.append(""" diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java index 85278bd60821d..90e613e67e2f4 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java @@ -20,7 +20,6 @@ import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; @@ -66,7 +65,7 @@ public void handle(HttpExchange exchange) throws IOException { && ("/" + tenantId + "/oauth2/v2.0/token").equals(exchange.getRequestURI().getPath())) { final String requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), StandardCharsets.UTF_8)); - final var params = RestUtils.decodeQueryString(requestBody, 0); + final var params = ParameterMap.fromQueryString(requestBody); if (clientId.equals(params.get("client_id")) && federatedToken.equals(params.get("client_assertion")) diff --git a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java index 71b72feec1adb..376a20507dd0a 100644 --- a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java +++ b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java @@ -17,7 +17,6 @@ import org.elasticsearch.common.regex.Regex; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.fixture.HttpHeaderParser; import org.elasticsearch.xcontent.ToXContent; @@ -103,7 +102,7 @@ public void handle(final HttpExchange exchange) throws IOException { writeBlobVersionAsJson(exchange, blob); } else if (Regex.simpleMatch("GET /storage/v1/b/" + bucket + "/o*", request)) { // List Objects https://cloud.google.com/storage/docs/json_api/v1/objects/list - final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); + final var params = ParameterMap.from(exchange.getRequestURI()); final String prefix = params.getOrDefault("prefix", ""); final int maxResults = Integer.parseInt(params.getOrDefault("maxResults", String.valueOf(defaultPageLimit.get()))); final String delimiter = params.getOrDefault("delimiter", ""); @@ -214,7 +213,7 @@ public void handle(final HttpExchange exchange) throws IOException { } } else if (Regex.simpleMatch("POST /upload/storage/v1/b/" + bucket + "/*uploadType=resumable*", request)) { // Resumable upload initialization https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); + final var params = ParameterMap.from(exchange.getRequestURI()); final String blobName = params.get("name"); final Long ifGenerationMatch = parseOptionalLongParameter(exchange, IF_GENERATION_MATCH); final MockGcsBlobStore.ResumableUpload resumableUpload = mockGcsBlobStore.createResumableUpload( @@ -240,7 +239,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("PUT /upload/storage/v1/b/" + bucket + "/o?*uploadType=resumable*", request)) { // Resumable upload https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); + final var params = ParameterMap.from(exchange.getRequestURI()); final String contentRangeValue = requireHeader(exchange, "Content-Range"); final HttpHeaderParser.ContentRange contentRange = HttpHeaderParser.parseContentRangeHeader(contentRangeValue); @@ -468,7 +467,7 @@ private static String requireHeader(HttpExchange exchange, String headerName) { } private static Long parseOptionalLongParameter(HttpExchange exchange, String parameterName) { - final var params = RestUtils.decodeQueryString(exchange.getRequestURI()); + final var params = ParameterMap.from(exchange.getRequestURI()); if (params.containsKey(parameterName)) { try { return Long.parseLong(params.get(parameterName)); diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java index 1b2111f686896..6b710883da01c 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java @@ -14,7 +14,6 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Streams; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xpack.idp.action.SamlValidateAuthnRequestResponse; import org.elasticsearch.xpack.idp.saml.idp.SamlIdentityProvider; import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProvider; @@ -125,7 +124,7 @@ public void processQueryString(String queryString, ActionListener parameters) { - var expectedQueryStringMap = RestUtils.decodeQueryString(query, 0); + var expectedQueryStringMap = ParameterMap.fromQueryString(query); - var resourceVersionQueryStringMap = RestUtils.decodeQueryString(resourceVersionQueryString(), 0); + var resourceVersionQueryStringMap = ParameterMap.fromQueryString(resourceVersionQueryString()); Map actualQueryStringMap = new HashMap<>(); actualQueryStringMap.putAll(resourceVersionQueryStringMap); diff --git a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java index 30993b677371d..4d58dfbf9c4ce 100644 --- a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java +++ b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java @@ -19,7 +19,6 @@ import org.elasticsearch.common.ssl.PemUtils; import org.elasticsearch.core.Strings; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; import org.junit.rules.ExternalResource; @@ -122,7 +121,7 @@ private void registerGetAccessTokenHandler() { } final var requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), Charset.defaultCharset())); - final var formFields = RestUtils.decodeQueryString(requestBody, 0); + final var formFields = ParameterMap.fromQueryString(requestBody); if (formFields.get("grant_type").equals("client_credentials") == false) { graphError(exchange, RestStatus.BAD_REQUEST, Strings.format("Unexpected Grant Type: %s", formFields.get("grant_type"))); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index 1c088e9a15398..1f0faec9683ea 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -15,7 +15,6 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Streams; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xpack.core.security.support.RestorableContextClassLoader; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.io.Unmarshaller; @@ -396,7 +395,7 @@ protected void validateNotOnOrAfter(Instant notOnOrAfter) { protected ParsedQueryString parseQueryStringAndValidateSignature(String queryString, String samlMessageParameterName) { final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); - final var parameters = RestUtils.decodeQueryString(queryString, 0); + final var parameters = ParameterMap.fromQueryString(queryString); final String samlMessage = parameters.get(samlMessageParameterName); if (samlMessage == null) { throw samlException("Could not parse {} from query string: [{}]", samlMessageParameterName, queryString); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 201d9392d3a27..1bf6630aa2737 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -19,7 +19,6 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.license.MockLicenseState; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xpack.core.security.action.oidc.OpenIdConnectLogoutResponse; import org.elasticsearch.xpack.core.security.action.oidc.OpenIdConnectPrepareAuthenticationResponse; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; @@ -357,7 +356,7 @@ public void testBuildLogoutResponse() throws Exception { final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1); + final var parameters = ParameterMap.fromUrl(endSessionUrl); assertThat(parameters, aMapWithSize(3)); assertThat(parameters, hasKey("id_token_hint")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -380,7 +379,7 @@ public void testBuildLogoutResponseFromEndsessionEndpointWithExistingParameters( final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RestUtils.decodeQueryString(endSessionUrl, endSessionUrl.indexOf("?") + 1); + final var parameters = ParameterMap.fromUrl(endSessionUrl); assertThat(parameters, aMapWithSize(4)); assertThat(parameters, hasKey("parameter")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -453,9 +452,9 @@ private void assertEqualUrlStrings(String actual, String expected) { assertThat(endOfPath, greaterThan(-1)); assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1))); - final var actualParams = RestUtils.decodeQueryString(actual, endOfPath + 1); + final var actualParams = ParameterMap.fromUrl(actual); - final var expectedParams = RestUtils.decodeQueryString(expected, endOfPath + 1); + final var expectedParams = ParameterMap.fromUrl(expected); assertThat(actualParams, equalTo(expectedParams)); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java index 601be19322c4b..03105e13c16ca 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java @@ -10,7 +10,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; @@ -560,7 +559,7 @@ public Builder fromUrl(String supposedUrl) { } String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - setParams(RestUtils.decodeQueryString(rawQuery, 0)); + setParams(ParameterMap.fromQueryString(rawQuery)); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java index 4e8864eabe0be..8bc316f7691e7 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.util.Maps; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.script.ScriptType; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; @@ -542,7 +541,7 @@ public Builder fromUrl(String supposedUrl) { String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - RestUtils.decodeQueryString(rawQuery, 0).forEach((k, v) -> params.put(k, new TextTemplate(v))); + ParameterMap.fromQueryString(rawQuery).forEach((k, v) -> params.put(k, new TextTemplate(v))); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java index 10f6432f2a305..068099a7a8d62 100644 --- a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java +++ b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java @@ -19,7 +19,6 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.core.Strings; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.TestMatchers; import org.elasticsearch.test.TestSecurityClient; import org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings; @@ -138,7 +137,7 @@ public void testAuthenticateWithOidcIssuedJwt() throws Exception { * The three-part-encoded JWT id_token will be in the "id_token" field */ final int hashChar = implicitFlowURI.indexOf('#'); - final var hashParams = RestUtils.decodeQueryString(implicitFlowURI.substring(hashChar + 1), 0); + final var hashParams = ParameterMap.fromQueryString(implicitFlowURI.substring(hashChar + 1)); assertThat("Hash value of URI [" + implicitFlowURI + "] should be a JWT with an id Token", hashParams, hasKey("id_token")); String idJwt = hashParams.get("id_token"); From 285febec2fc50c954ee992210be7ac9e48652497 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 11:59:37 +0100 Subject: [PATCH 07/29] Rename ParameterMap to RequestParams --- .../azure/AzureBlobContainerRetriesTests.java | 5 +- ...CloudStorageBlobContainerRetriesTests.java | 3 +- .../{ParameterMap.java => RequestParams.java} | 46 +++++++++---------- .../org/elasticsearch/rest/RestRequest.java | 16 +++---- .../org/elasticsearch/rest/RestUtils.java | 8 ++-- ...rMapTests.java => RequestParamsTests.java} | 40 ++++++++-------- .../elasticsearch/rest/RestUtilsTests.java | 44 +++++++++--------- .../java/fixture/azure/AzureHttpHandler.java | 5 +- .../AzureOAuthTokenServiceHttpHandler.java | 3 +- .../gcs/GoogleCloudStorageHttpHandler.java | 9 ++-- .../test/rest/FakeRestRequest.java | 12 ++--- .../saml/authn/SamlAuthnRequestValidator.java | 3 +- .../exporter/http/HttpExporterIT.java | 5 +- .../microsoft/MicrosoftGraphHttpFixture.java | 3 +- .../authc/saml/SamlObjectHandler.java | 3 +- .../authc/oidc/OpenIdConnectRealmTests.java | 9 ++-- .../watcher/common/http/HttpRequest.java | 3 +- .../common/http/HttpRequestTemplate.java | 3 +- .../security/authc/jwt/JwtWithOidcAuthIT.java | 3 +- 19 files changed, 118 insertions(+), 105 deletions(-) rename server/src/main/java/org/elasticsearch/rest/{ParameterMap.java => RequestParams.java} (83%) rename server/src/test/java/org/elasticsearch/rest/{ParameterMapTests.java => RequestParamsTests.java} (79%) diff --git a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java index 24c16fe56298e..6ec584111e204 100644 --- a/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java +++ b/modules/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java @@ -46,6 +46,7 @@ import org.elasticsearch.mocksocket.MockHttpServer; import org.elasticsearch.repositories.RepositoriesMetrics; import org.elasticsearch.repositories.blobstore.AbstractBlobContainerRetriesTestCase; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.fixture.HttpHeaderParser; @@ -367,7 +368,7 @@ public void testWriteLargeBlob() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); + final var params = RequestParams.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); @@ -436,7 +437,7 @@ public void testWriteLargeBlobStreaming() throws Exception { httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_large_blob_streaming"), exchange -> { if ("PUT".equals(exchange.getRequestMethod())) { - final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); + final var params = RequestParams.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert Strings.hasText(blockId) == false || AzureFixtureHelper.assertValidBlockId(blockId); diff --git a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java index eaa31df83bdcf..a4b99d9740487 100644 --- a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java +++ b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java @@ -55,6 +55,7 @@ import org.elasticsearch.http.ResponseInjectingHttpHandler; import org.elasticsearch.repositories.blobstore.AbstractBlobContainerRetriesTestCase; import org.elasticsearch.repositories.blobstore.ESMockAPIBasedRepositoryIntegTestCase; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.fixture.HttpHeaderParser; @@ -419,7 +420,7 @@ public void testWriteLargeBlob() throws IOException { httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> { final BytesReference requestBody = Streams.readFully(exchange.getRequestBody()); - final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getQuery()); + final var params = RequestParams.fromQueryString(exchange.getRequestURI().getQuery()); assertThat(params.get("uploadType"), equalTo("resumable")); if ("POST".equals(exchange.getRequestMethod())) { diff --git a/server/src/main/java/org/elasticsearch/rest/ParameterMap.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java similarity index 83% rename from server/src/main/java/org/elasticsearch/rest/ParameterMap.java rename to server/src/main/java/org/elasticsearch/rest/RequestParams.java index 11488116a8d26..8198af6a4db50 100644 --- a/server/src/main/java/org/elasticsearch/rest/ParameterMap.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -32,7 +32,7 @@ * * Use {@link #getAll(String)} to retrieve all values for a repeated key. */ -public final class ParameterMap extends AbstractMap { +public final class RequestParams extends AbstractMap { /** Single backing store: key → non-empty ordered list of all values. */ private final LinkedHashMap> map; @@ -42,74 +42,74 @@ public final class ParameterMap extends AbstractMap { // ------------------------------------------------------------------------- /** - * Returns a new, empty {@code ParameterMap}. + * Returns a new, empty {@code RequestParams}. */ - public static ParameterMap empty() { - return new ParameterMap(Map.of()); + public static RequestParams empty() { + return new RequestParams(Map.of()); } /** - * Creates a {@code ParameterMap} from a multi-value map. Each list must be non-empty. + * Creates a {@code RequestParams} from a multi-value map. Each list must be non-empty. * The last value in each list is what {@link #get(Object)} returns. * * @param multiValues a map from parameter name to all its values, in encounter order */ - public static ParameterMap of(Map> multiValues) { - return new ParameterMap(multiValues); + public static RequestParams of(Map> multiValues) { + return new RequestParams(multiValues); } /** - * Parses a URL-encoded query string into a {@code ParameterMap}, preserving all values for + * Parses a URL-encoded query string into a {@code RequestParams}, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). * * @param queryString the raw query string (the part after {@code ?}, without the {@code ?} itself) - * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + * @return a {@code RequestParams} from parameter name to all its values, in encounter order */ - public static ParameterMap fromQueryString(String queryString) { + public static RequestParams fromQueryString(String queryString) { return RestUtils.decodeQueryString(queryString, 0); } /** - * Parses the query string from a full URL string into a {@code ParameterMap}, preserving all + * Parses the query string from a full URL string into a {@code RequestParams}, preserving all * values for repeated parameters. Returns an empty map if the URL contains no {@code ?}. * * @param url a full URL string, e.g. {@code /index/_search?pretty&size=10} - * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + * @return a {@code RequestParams} from parameter name to all its values, in encounter order */ - public static ParameterMap fromUrl(String url) { + public static RequestParams fromUrl(String url) { int index = url.indexOf('?'); - return index >= 0 ? RestUtils.decodeQueryString(url, index + 1) : ParameterMap.empty(); + return index >= 0 ? RestUtils.decodeQueryString(url, index + 1) : RequestParams.empty(); } /** - * Parses the query string from a {@link URI} into a {@code ParameterMap}, preserving all + * Parses the query string from a {@link URI} into a {@code RequestParams}, preserving all * values for repeated parameters. Returns an empty map if the URI has no query. * * @param uri the URI whose raw query string is parsed - * @return a {@code ParameterMap} from parameter name to all its values, in encounter order + * @return a {@code RequestParams} from parameter name to all its values, in encounter order */ - public static ParameterMap from(URI uri) { + public static RequestParams from(URI uri) { final var rawQuery = uri.getRawQuery(); - return rawQuery != null && rawQuery.isEmpty() == false ? fromQueryString(rawQuery) : ParameterMap.empty(); + return rawQuery != null && rawQuery.isEmpty() == false ? fromQueryString(rawQuery) : RequestParams.empty(); } /** - * Creates a {@code ParameterMap} from a plain single-value map. + * Creates a {@code RequestParams} from a plain single-value map. * Each value is wrapped in a singleton list so that {@link #getAll(String)} returns a * one-element list rather than an empty one. * * @param singleValues a map whose values are treated as the sole value for each key */ - public static ParameterMap fromSingleValues(Map singleValues) { + public static RequestParams fromSingleValues(Map singleValues) { LinkedHashMap> wrapped = new LinkedHashMap<>(singleValues.size() * 2); singleValues.forEach((k, v) -> wrapped.put(k, List.of(v))); - return new ParameterMap(wrapped); + return new RequestParams(wrapped); } - private ParameterMap(Map> multiValues) { + private RequestParams(Map> multiValues) { this.map = new LinkedHashMap<>(multiValues); assert map.values().stream().allMatch(list -> list != null && list.isEmpty() == false) - : "ParameterMap requires every value list to be non-empty"; + : "RequestParams requires every value list to be non-empty"; } // ------------------------------------------------------------------------- diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 4c592ae6c119e..80e716741fe9d 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -86,7 +86,7 @@ public class RestRequest implements ToXContent.Params, Traceable { private static final AtomicLong requestIdGenerator = new AtomicLong(); private final XContentParserConfiguration parserConfig; - private final ParameterMap params; + private final RequestParams params; private final Map> headers; private final String rawPath; private final Set consumedParams = new HashSet<>(); @@ -108,7 +108,7 @@ public boolean isContentConsumed() { @SuppressWarnings("this-escape") protected RestRequest( XContentParserConfiguration parserConfig, - ParameterMap params, + RequestParams params, String rawPath, Map> headers, HttpRequest httpRequest, @@ -120,7 +120,7 @@ protected RestRequest( @SuppressWarnings("this-escape") private RestRequest( XContentParserConfiguration parserConfig, - ParameterMap params, + RequestParams params, String rawPath, Map> headers, HttpRequest httpRequest, @@ -198,7 +198,7 @@ protected RestRequest(RestRequest other) { * @throws MediaTypeHeaderException if the Content-Type or Accept header can not be parsed */ public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) { - ParameterMap params = params(httpRequest.uri()); + RequestParams params = params(httpRequest.uri()); return new RestRequest( parserConfig, params, @@ -210,9 +210,9 @@ public static RestRequest request(XContentParserConfiguration parserConfig, Http ); } - private static ParameterMap params(final String uri) { + private static RequestParams params(final String uri) { try { - return ParameterMap.fromUrl(uri); + return RequestParams.fromUrl(uri); } catch (final IllegalArgumentException e) { throw new BadParameterException(e); } @@ -231,7 +231,7 @@ public static RestRequest requestWithoutParameters( ) { return new RestRequest( parserConfig, - ParameterMap.empty(), + RequestParams.empty(), httpRequest.uri(), httpRequest.getHeaders(), httpRequest, @@ -416,7 +416,7 @@ public final String param(String key, String defaultValue) { return value; } - public ParameterMap params() { + public RequestParams params() { return params; } diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index bdfcf70d7e666..64e30a7c08c92 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -45,20 +45,20 @@ public class RestUtils { public static final UnaryOperator REST_DECODER = RestUtils::decodeComponent; /** - * Parses a URL-encoded query string into a {@link ParameterMap}, preserving all values for + * Parses a URL-encoded query string into a {@link RequestParams}, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). * * @param s the full string containing the query string * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) - * @return a {@link ParameterMap} from parameter name to all its values, in encounter order + * @return a {@link RequestParams} from parameter name to all its values, in encounter order */ - static ParameterMap decodeQueryString(String s, int fromIndex) { + static RequestParams decodeQueryString(String s, int fromIndex) { Map> result = new LinkedHashMap<>(); parseQueryStringPairs(s, fromIndex, (name, value) -> { checkReservedParam(name); result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); }); - return ParameterMap.of(result); + return RequestParams.of(result); } private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer consumer) { diff --git a/server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java similarity index 79% rename from server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java rename to server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index 8f61afae38549..e3e01b18e5a67 100644 --- a/server/src/test/java/org/elasticsearch/rest/ParameterMapTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -18,14 +18,14 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; -public class ParameterMapTests extends ESTestCase { +public class RequestParamsTests extends ESTestCase { // ------------------------------------------------------------------------- // Factory methods // ------------------------------------------------------------------------- public void testEmpty() { - var map = ParameterMap.empty(); + var map = RequestParams.empty(); assertThat(map.isEmpty(), is(true)); assertThat(map.size(), equalTo(0)); assertThat(map.get("x"), nullValue()); @@ -33,14 +33,14 @@ public void testEmpty() { } public void testOf() { - var map = ParameterMap.of(Map.of("a", List.of("1", "2"), "b", List.of("3"))); + var map = RequestParams.of(Map.of("a", List.of("1", "2"), "b", List.of("3"))); assertThat(map.size(), equalTo(2)); assertThat(map.getAll("a"), equalTo(List.of("1", "2"))); assertThat(map.getAll("b"), equalTo(List.of("3"))); } public void testFromSingleValues() { - var map = ParameterMap.fromSingleValues(Map.of("a", "1", "b", "2")); + var map = RequestParams.fromSingleValues(Map.of("a", "1", "b", "2")); assertThat(map.size(), equalTo(2)); assertThat(map.get("a"), equalTo("1")); assertThat(map.get("b"), equalTo("2")); @@ -53,22 +53,22 @@ public void testFromSingleValues() { // ------------------------------------------------------------------------- public void testGetReturnsLastValue() { - var map = ParameterMap.of(Map.of("k", List.of("first", "second", "last"))); + var map = RequestParams.of(Map.of("k", List.of("first", "second", "last"))); assertThat(map.get("k"), equalTo("last")); } public void testGetReturnsNullForAbsentKey() { - var map = ParameterMap.of(Map.of("k", List.of("v"))); + var map = RequestParams.of(Map.of("k", List.of("v"))); assertThat(map.get("missing"), nullValue()); } public void testGetAllReturnsAllValues() { - var map = ParameterMap.of(Map.of("k", List.of("a", "b", "c"))); + var map = RequestParams.of(Map.of("k", List.of("a", "b", "c"))); assertThat(map.getAll("k"), equalTo(List.of("a", "b", "c"))); } public void testGetAllReturnsEmptyListForAbsentKey() { - var map = ParameterMap.empty(); + var map = RequestParams.empty(); assertThat(map.getAll("missing"), equalTo(List.of())); } @@ -77,17 +77,17 @@ public void testGetAllReturnsEmptyListForAbsentKey() { // ------------------------------------------------------------------------- public void testGetSingleReturnsSingleValue() { - var map = ParameterMap.of(Map.of("k", List.of("only"))); + var map = RequestParams.of(Map.of("k", List.of("only"))); assertThat(map.getSingle("k"), equalTo("only")); } public void testGetSingleReturnsNullForAbsentKey() { - var map = ParameterMap.empty(); + var map = RequestParams.empty(); assertThat(map.getSingle("missing"), nullValue()); } public void testGetSingleThrowsOnMultipleValues() { - var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var map = RequestParams.of(Map.of("k", List.of("a", "b"))); var ex = expectThrows(IllegalArgumentException.class, () -> map.getSingle("k")); assertThat(ex.getMessage(), equalTo("parameter [k] must have a single value, but found: [a, b]")); } @@ -97,7 +97,7 @@ public void testGetSingleThrowsOnMultipleValues() { // ------------------------------------------------------------------------- public void testPutReplacesAllValues() { - var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var map = RequestParams.of(Map.of("k", List.of("a", "b"))); var previous = map.put("k", "new"); assertThat(previous, equalTo("b")); // previous last value assertThat(map.get("k"), equalTo("new")); @@ -105,26 +105,26 @@ public void testPutReplacesAllValues() { } public void testPutNewKey() { - var map = ParameterMap.empty(); + var map = RequestParams.empty(); var previous = map.put("k", "v"); assertThat(previous, nullValue()); assertThat(map.get("k"), equalTo("v")); } public void testRemove() { - var map = ParameterMap.of(Map.of("k", List.of("a", "b"))); + var map = RequestParams.of(Map.of("k", List.of("a", "b"))); var removed = map.remove("k"); assertThat(removed, equalTo("b")); // last value assertThat(map.containsKey("k"), is(false)); } public void testRemoveAbsentKey() { - var map = ParameterMap.empty(); + var map = RequestParams.empty(); assertThat(map.remove("missing"), nullValue()); } public void testClear() { - var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); map.clear(); assertThat(map.isEmpty(), is(true)); } @@ -134,25 +134,25 @@ public void testClear() { // ------------------------------------------------------------------------- public void testContainsKey() { - var map = ParameterMap.of(Map.of("present", List.of("v"))); + var map = RequestParams.of(Map.of("present", List.of("v"))); assertThat(map.containsKey("present"), is(true)); assertThat(map.containsKey("absent"), is(false)); } public void testKeySet() { - var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); assertThat(map.keySet(), equalTo(java.util.Set.of("a", "b"))); } public void testEntrySetValuesAreLastValues() { - var map = ParameterMap.of(Map.of("k", List.of("first", "last"))); + var map = RequestParams.of(Map.of("k", List.of("first", "last"))); var entry = map.entrySet().iterator().next(); assertThat(entry.getKey(), equalTo("k")); assertThat(entry.getValue(), equalTo("last")); } public void testEntrySetRemove() { - var map = ParameterMap.of(Map.of("a", List.of("1"), "b", List.of("2"))); + var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); var it = map.entrySet().iterator(); it.next(); it.remove(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index ad9c00afb6207..9f00a2e290d40 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -31,50 +31,50 @@ static char randomDelimiter() { } public void testDecodeQueryStringFromUrl() { - var params = ParameterMap.fromUrl("something?test=value"); + var params = RequestParams.fromUrl("something?test=value"); assertThat(params.size(), equalTo(1)); assertThat(params.get("test"), equalTo("value")); - params = ParameterMap.fromUrl(Strings.format("something?test=value%ctest1=value1", randomDelimiter())); + params = RequestParams.fromUrl(Strings.format("something?test=value%ctest1=value1", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("test"), equalTo("value")); assertThat(params.get("test1"), equalTo("value1")); // no query string - assertThat(ParameterMap.fromUrl("something").isEmpty(), is(true)); + assertThat(RequestParams.fromUrl("something").isEmpty(), is(true)); } public void testDecodeQueryStringFromUrlEdgeCases() { // empty query string - assertThat(ParameterMap.fromUrl("something?").size(), equalTo(0)); + assertThat(RequestParams.fromUrl("something?").size(), equalTo(0)); - assertThat(ParameterMap.fromUrl(Strings.format("something?%c", randomDelimiter())).size(), equalTo(0)); + assertThat(RequestParams.fromUrl(Strings.format("something?%c", randomDelimiter())).size(), equalTo(0)); - var params = ParameterMap.fromUrl(Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter())); + var params = RequestParams.fromUrl(Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - assertThat(ParameterMap.fromUrl("something?=").size(), equalTo(0)); + assertThat(RequestParams.fromUrl("something?=").size(), equalTo(0)); - assertThat(ParameterMap.fromUrl(Strings.format("something?%c=", randomDelimiter())).size(), equalTo(0)); + assertThat(RequestParams.fromUrl(Strings.format("something?%c=", randomDelimiter())).size(), equalTo(0)); - params = ParameterMap.fromUrl("something?a"); + params = RequestParams.fromUrl("something?a"); assertThat(params.size(), equalTo(1)); assertThat(params.get("a"), equalTo("")); - params = ParameterMap.fromUrl(Strings.format("something?p=v%ca", randomDelimiter())); + params = RequestParams.fromUrl(Strings.format("something?p=v%ca", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); - params = ParameterMap.fromUrl(Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter())); + params = RequestParams.fromUrl(Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(3)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - params = ParameterMap.fromUrl( + params = RequestParams.fromUrl( Strings.format("something?p=v%ca%cb%cp1=v1", randomDelimiter(), randomDelimiter(), randomDelimiter()) ); assertThat(params.size(), equalTo(4)); @@ -85,29 +85,29 @@ public void testDecodeQueryStringFromUrlEdgeCases() { } public void testDecodeQueryString() { - var params = ParameterMap.fromQueryString("test=value"); + var params = RequestParams.fromQueryString("test=value"); assertThat(params.size(), equalTo(1)); assertThat(params.getAll("test"), equalTo(List.of("value"))); } public void testDecodeQueryStringMultipleValues() { - var params = ParameterMap.fromQueryString("match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"); + var params = RequestParams.fromQueryString("match%5B%5D=up&match%5B%5D=http_requests_total&start=1609746000"); assertThat(params.getAll("match[]"), equalTo(List.of("up", "http_requests_total"))); assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); } public void testDecodeQueryStringDelimiters() { - var params = ParameterMap.fromQueryString(Strings.format("a=1%cb=2", randomDelimiter())); + var params = RequestParams.fromQueryString(Strings.format("a=1%cb=2", randomDelimiter())); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.getAll("b"), equalTo(List.of("2"))); } public void testDecodeQueryStringEdgeCases() { // empty query string - assertThat(ParameterMap.fromQueryString("").isEmpty(), is(true)); + assertThat(RequestParams.fromQueryString("").isEmpty(), is(true)); // key with no value - assertThat(ParameterMap.fromQueryString("a").getAll("a"), equalTo(List.of(""))); + assertThat(RequestParams.fromQueryString("a").getAll("a"), equalTo(List.of(""))); // package-private fromIndex edge cases assertThat(RestUtils.decodeQueryString("something", 9).isEmpty(), is(true)); @@ -116,19 +116,19 @@ public void testDecodeQueryStringEdgeCases() { public void testDecodeQueryStringFragment() { // fragment should be excluded - var params = ParameterMap.fromUrl("something?a=1#fragment"); + var params = RequestParams.fromUrl("something?a=1#fragment"); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.containsKey("fragment"), is(false)); } public void testDecodeQueryStringUrlEncoded() { - var params = ParameterMap.fromQueryString("match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"); + var params = RequestParams.fromQueryString("match%5B%5D=up%7Bjob%3D%22prometheus%22%7D"); assertThat(params.getAll("match[]"), equalTo(List.of("up{job=\"prometheus\"}"))); } public void testDecodeQueryStringReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { - expectThrows(IllegalArgumentException.class, () -> ParameterMap.fromQueryString(reservedParam + "=value")); + expectThrows(IllegalArgumentException.class, () -> RequestParams.fromQueryString(reservedParam + "=value")); } } @@ -170,7 +170,7 @@ public void testCrazyURL() { randomDelimiter(), randomDelimiter() ); - var params = ParameterMap.fromUrl(uri); + var params = RequestParams.fromUrl(uri); assertThat(params.get("/?:@-._~!$'()* ,"), equalTo("/?:@-._~!$'()* ,==")); assertThat(params.size(), equalTo(1)); } @@ -179,7 +179,7 @@ public void testReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> ParameterMap.fromUrl("something?" + reservedParam + "=value") + () -> RequestParams.fromUrl("something?" + reservedParam + "=value") ); assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); } diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java index ba66b71783389..94e2783364e88 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.regex.Regex; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.fixture.HttpHeaderParser; @@ -136,7 +137,7 @@ public void handle(final HttpExchange exchange) throws IOException { try { if (Regex.simpleMatch("PUT /" + account + "/" + container + "/*blockid=*", request)) { // Put Block (https://docs.microsoft.com/en-us/rest/api/storageservices/put-block) - final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getRawQuery()); + final var params = RequestParams.fromQueryString(exchange.getRequestURI().getRawQuery()); final String blockId = params.get("blockid"); assert assertValidBlockId(blockId); @@ -271,7 +272,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("GET /" + account + "/" + container + "?*restype=container*comp=list*", request)) { // List Blobs (https://docs.microsoft.com/en-us/rest/api/storageservices/list-blobs) - final var params = ParameterMap.fromQueryString(exchange.getRequestURI().getQuery()); + final var params = RequestParams.fromQueryString(exchange.getRequestURI().getQuery()); final StringBuilder list = new StringBuilder(); list.append(""" diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java index 90e613e67e2f4..26bd23336b83f 100644 --- a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java +++ b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureOAuthTokenServiceHttpHandler.java @@ -20,6 +20,7 @@ import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; @@ -65,7 +66,7 @@ public void handle(HttpExchange exchange) throws IOException { && ("/" + tenantId + "/oauth2/v2.0/token").equals(exchange.getRequestURI().getPath())) { final String requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), StandardCharsets.UTF_8)); - final var params = ParameterMap.fromQueryString(requestBody); + final var params = RequestParams.fromQueryString(requestBody); if (clientId.equals(params.get("client_id")) && federatedToken.equals(params.get("client_assertion")) diff --git a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java index 376a20507dd0a..65260f75bf2c6 100644 --- a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java +++ b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.fixture.HttpHeaderParser; @@ -102,7 +103,7 @@ public void handle(final HttpExchange exchange) throws IOException { writeBlobVersionAsJson(exchange, blob); } else if (Regex.simpleMatch("GET /storage/v1/b/" + bucket + "/o*", request)) { // List Objects https://cloud.google.com/storage/docs/json_api/v1/objects/list - final var params = ParameterMap.from(exchange.getRequestURI()); + final var params = RequestParams.from(exchange.getRequestURI()); final String prefix = params.getOrDefault("prefix", ""); final int maxResults = Integer.parseInt(params.getOrDefault("maxResults", String.valueOf(defaultPageLimit.get()))); final String delimiter = params.getOrDefault("delimiter", ""); @@ -213,7 +214,7 @@ public void handle(final HttpExchange exchange) throws IOException { } } else if (Regex.simpleMatch("POST /upload/storage/v1/b/" + bucket + "/*uploadType=resumable*", request)) { // Resumable upload initialization https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = ParameterMap.from(exchange.getRequestURI()); + final var params = RequestParams.from(exchange.getRequestURI()); final String blobName = params.get("name"); final Long ifGenerationMatch = parseOptionalLongParameter(exchange, IF_GENERATION_MATCH); final MockGcsBlobStore.ResumableUpload resumableUpload = mockGcsBlobStore.createResumableUpload( @@ -239,7 +240,7 @@ public void handle(final HttpExchange exchange) throws IOException { } else if (Regex.simpleMatch("PUT /upload/storage/v1/b/" + bucket + "/o?*uploadType=resumable*", request)) { // Resumable upload https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload - final var params = ParameterMap.from(exchange.getRequestURI()); + final var params = RequestParams.from(exchange.getRequestURI()); final String contentRangeValue = requireHeader(exchange, "Content-Range"); final HttpHeaderParser.ContentRange contentRange = HttpHeaderParser.parseContentRangeHeader(contentRangeValue); @@ -467,7 +468,7 @@ private static String requireHeader(HttpExchange exchange, String headerName) { } private static Long parseOptionalLongParameter(HttpExchange exchange, String parameterName) { - final var params = ParameterMap.from(exchange.getRequestURI()); + final var params = RequestParams.from(exchange.getRequestURI()); if (params.containsKey(parameterName)) { try { return Long.parseLong(params.get(parameterName)); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java index 3afccce9d46bf..99cb2795a46a5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java @@ -19,7 +19,7 @@ import org.elasticsearch.http.HttpRequest; import org.elasticsearch.http.HttpResponse; import org.elasticsearch.rest.ChunkedRestResponseBodyPart; -import org.elasticsearch.rest.ParameterMap; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -38,12 +38,12 @@ public FakeRestRequest() { this( XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE), new FakeHttpRequest(Method.GET, "", BytesArray.EMPTY, new HashMap<>()), - ParameterMap.empty(), + RequestParams.empty(), new FakeHttpChannel(null) ); } - private FakeRestRequest(XContentParserConfiguration config, HttpRequest httpRequest, ParameterMap params, HttpChannel httpChannel) { + private FakeRestRequest(XContentParserConfiguration config, HttpRequest httpRequest, RequestParams params, HttpChannel httpChannel) { super(config, params, httpRequest.uri(), httpRequest.getHeaders(), httpRequest, httpChannel); } @@ -203,7 +203,7 @@ public static class Builder { private Map> headers = new HashMap<>(); - private ParameterMap params = ParameterMap.empty(); + private RequestParams params = RequestParams.empty(); private HttpBody content = HttpBody.empty(); @@ -227,12 +227,12 @@ public Builder withHeaders(Map> headers) { public Builder withParams(Map params) { if (params != null) { - this.params = ParameterMap.fromSingleValues(params); + this.params = RequestParams.fromSingleValues(params); } return this; } - public Builder withMultiParams(ParameterMap multiParams) { + public Builder withMultiParams(RequestParams multiParams) { this.params = multiParams; return this; } diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java index 6b710883da01c..2d588ff35de64 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Streams; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.idp.action.SamlValidateAuthnRequestResponse; import org.elasticsearch.xpack.idp.saml.idp.SamlIdentityProvider; @@ -124,7 +125,7 @@ public void processQueryString(String queryString, ActionListener parameters) { - var expectedQueryStringMap = ParameterMap.fromQueryString(query); + var expectedQueryStringMap = RequestParams.fromQueryString(query); - var resourceVersionQueryStringMap = ParameterMap.fromQueryString(resourceVersionQueryString()); + var resourceVersionQueryStringMap = RequestParams.fromQueryString(resourceVersionQueryString()); Map actualQueryStringMap = new HashMap<>(); actualQueryStringMap.putAll(resourceVersionQueryStringMap); diff --git a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java index 4d58dfbf9c4ce..a3c3f665eb4be 100644 --- a/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java +++ b/x-pack/plugin/security/qa/microsoft-graph-authz-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphHttpFixture.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.ssl.KeyStoreUtil; import org.elasticsearch.common.ssl.PemUtils; import org.elasticsearch.core.Strings; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; @@ -121,7 +122,7 @@ private void registerGetAccessTokenHandler() { } final var requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), Charset.defaultCharset())); - final var formFields = ParameterMap.fromQueryString(requestBody); + final var formFields = RequestParams.fromQueryString(requestBody); if (formFields.get("grant_type").equals("client_credentials") == false) { graphError(exchange, RestStatus.BAD_REQUEST, Strings.format("Unexpected Grant Type: %s", formFields.get("grant_type"))); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index 1f0faec9683ea..1155264d982a8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -15,6 +15,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Streams; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.xpack.core.security.support.RestorableContextClassLoader; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.io.Unmarshaller; @@ -395,7 +396,7 @@ protected void validateNotOnOrAfter(Instant notOnOrAfter) { protected ParsedQueryString parseQueryStringAndValidateSignature(String queryString, String samlMessageParameterName) { final String signatureInput = queryString.replaceAll("&Signature=.*$", ""); - final var parameters = ParameterMap.fromQueryString(queryString); + final var parameters = RequestParams.fromQueryString(queryString); final String samlMessage = parameters.get(samlMessageParameterName); if (samlMessage == null) { throw samlException("Could not parse {} from query string: [{}]", samlMessageParameterName, queryString); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 1bf6630aa2737..a265262969be1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.license.MockLicenseState; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.xpack.core.security.action.oidc.OpenIdConnectLogoutResponse; import org.elasticsearch.xpack.core.security.action.oidc.OpenIdConnectPrepareAuthenticationResponse; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; @@ -356,7 +357,7 @@ public void testBuildLogoutResponse() throws Exception { final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = ParameterMap.fromUrl(endSessionUrl); + final var parameters = RequestParams.fromUrl(endSessionUrl); assertThat(parameters, aMapWithSize(3)); assertThat(parameters, hasKey("id_token_hint")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -379,7 +380,7 @@ public void testBuildLogoutResponseFromEndsessionEndpointWithExistingParameters( final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = ParameterMap.fromUrl(endSessionUrl); + final var parameters = RequestParams.fromUrl(endSessionUrl); assertThat(parameters, aMapWithSize(4)); assertThat(parameters, hasKey("parameter")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -452,9 +453,9 @@ private void assertEqualUrlStrings(String actual, String expected) { assertThat(endOfPath, greaterThan(-1)); assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1))); - final var actualParams = ParameterMap.fromUrl(actual); + final var actualParams = RequestParams.fromUrl(actual); - final var expectedParams = ParameterMap.fromUrl(expected); + final var expectedParams = RequestParams.fromUrl(expected); assertThat(actualParams, equalTo(expectedParams)); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java index 03105e13c16ca..1e92a78f6937d 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; @@ -559,7 +560,7 @@ public Builder fromUrl(String supposedUrl) { } String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - setParams(ParameterMap.fromQueryString(rawQuery)); + setParams(RequestParams.fromQueryString(rawQuery)); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java index 8bc316f7691e7..66551e86a3698 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequestTemplate.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.util.Maps; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.script.ScriptType; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; @@ -541,7 +542,7 @@ public Builder fromUrl(String supposedUrl) { String rawQuery = uri.getRawQuery(); if (Strings.hasLength(rawQuery)) { - ParameterMap.fromQueryString(rawQuery).forEach((k, v) -> params.put(k, new TextTemplate(v))); + RequestParams.fromQueryString(rawQuery).forEach((k, v) -> params.put(k, new TextTemplate(v))); } } catch (URISyntaxException e) { throw new ElasticsearchParseException("Malformed URL [{}]", supposedUrl); diff --git a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java index 068099a7a8d62..da68609bcdf1b 100644 --- a/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java +++ b/x-pack/qa/oidc-op-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithOidcAuthIT.java @@ -18,6 +18,7 @@ import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.ResponseException; import org.elasticsearch.core.Strings; +import org.elasticsearch.rest.RequestParams; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.TestMatchers; import org.elasticsearch.test.TestSecurityClient; @@ -137,7 +138,7 @@ public void testAuthenticateWithOidcIssuedJwt() throws Exception { * The three-part-encoded JWT id_token will be in the "id_token" field */ final int hashChar = implicitFlowURI.indexOf('#'); - final var hashParams = ParameterMap.fromQueryString(implicitFlowURI.substring(hashChar + 1)); + final var hashParams = RequestParams.fromQueryString(implicitFlowURI.substring(hashChar + 1)); assertThat("Hash value of URI [" + implicitFlowURI + "] should be a JWT with an id Token", hashParams, hasKey("id_token")); String idJwt = hashParams.get("id_token"); From 0b5404b0903e02c72d17714de6b9ce27d51e1c06 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 12:56:25 +0100 Subject: [PATCH 08/29] Make RequestParams immutable Mutation methods (put, remove, clear) now throw UnsupportedOperationException. Value lists are defensively copied via List.copyOf in the constructor, and keySet/entrySet no longer expose mutable views. Tests updated accordingly. --- .../org/elasticsearch/rest/RequestParams.java | 40 ++++++++----------- .../rest/RequestParamsTests.java | 37 +++++------------ 2 files changed, 25 insertions(+), 52 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 8198af6a4db50..6e806d81830fa 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -12,7 +12,6 @@ import java.net.URI; import java.util.AbstractMap; import java.util.AbstractSet; -import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; @@ -28,9 +27,11 @@ * operates on the last value in that list: *

    *
  • {@link #get(Object)} returns the last value for a key, or {@code null} if absent.
    • - *
  • {@link #put(String, String)} replaces all existing values with a single new one.
    • *
* Use {@link #getAll(String)} to retrieve all values for a repeated key. + * + *

Instances are immutable: mutation methods ({@code put}, {@code remove}, {@code clear}) + * throw {@link UnsupportedOperationException}. */ public final class RequestParams extends AbstractMap { @@ -107,9 +108,10 @@ public static RequestParams fromSingleValues(Map singleValues) { } private RequestParams(Map> multiValues) { - this.map = new LinkedHashMap<>(multiValues); - assert map.values().stream().allMatch(list -> list != null && list.isEmpty() == false) - : "RequestParams requires every value list to be non-empty"; + LinkedHashMap> copy = new LinkedHashMap<>(multiValues.size() * 2); + multiValues.forEach((k, v) -> copy.put(k, List.copyOf(v))); + assert copy.values().stream().allMatch(list -> list.isEmpty() == false) : "RequestParams requires every value list to be non-empty"; + this.map = copy; } // ------------------------------------------------------------------------- @@ -125,7 +127,7 @@ private RequestParams(Map> multiValues) { */ public List getAll(String key) { var list = map.get(key); - return list == null ? List.of() : Collections.unmodifiableList(list); + return list == null ? List.of() : list; } /** @@ -160,25 +162,22 @@ public String get(Object key) { return list == null ? null : list.getLast(); } - /** - * Associates {@code key} with {@code value}, replacing all previous values for that key. - * Returns the previous last value, or {@code null}. - */ + /** @throws UnsupportedOperationException always */ @Override public String put(String key, String value) { - var old = map.put(key, new ArrayList<>(List.of(value))); - return old == null ? null : old.getLast(); + throw new UnsupportedOperationException("RequestParams is immutable"); } + /** @throws UnsupportedOperationException always */ @Override public String remove(Object key) { - var old = map.remove(key); - return old == null ? null : old.getLast(); + throw new UnsupportedOperationException("RequestParams is immutable"); } + /** @throws UnsupportedOperationException always */ @Override public void clear() { - map.clear(); + throw new UnsupportedOperationException("RequestParams is immutable"); } @Override @@ -191,15 +190,13 @@ public boolean containsKey(Object key) { return map.containsKey(key); } - /** Returns a live key set backed by the underlying map. */ @Override public Set keySet() { - return map.keySet(); + return Collections.unmodifiableSet(map.keySet()); } /** - * Returns a live entry set where each entry's value is the last value for that key. - * Removal through the iterator is supported and removes the key from the map entirely. + * Returns an unmodifiable entry set where each entry's value is the last value for that key. */ @Override public Set> entrySet() { @@ -218,11 +215,6 @@ public Entry next() { var e = inner.next(); return Map.entry(e.getKey(), e.getValue().getLast()); } - - @Override - public void remove() { - inner.remove(); - } }; } diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index e3e01b18e5a67..cc9ec4318c4af 100644 --- a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -93,40 +93,22 @@ public void testGetSingleThrowsOnMultipleValues() { } // ------------------------------------------------------------------------- - // put / remove / clear + // immutability // ------------------------------------------------------------------------- - public void testPutReplacesAllValues() { + public void testPutThrows() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - var previous = map.put("k", "new"); - assertThat(previous, equalTo("b")); // previous last value - assertThat(map.get("k"), equalTo("new")); - assertThat(map.getAll("k"), equalTo(List.of("new"))); + expectThrows(UnsupportedOperationException.class, () -> map.put("k", "new")); } - public void testPutNewKey() { - var map = RequestParams.empty(); - var previous = map.put("k", "v"); - assertThat(previous, nullValue()); - assertThat(map.get("k"), equalTo("v")); - } - - public void testRemove() { + public void testRemoveThrows() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - var removed = map.remove("k"); - assertThat(removed, equalTo("b")); // last value - assertThat(map.containsKey("k"), is(false)); + expectThrows(UnsupportedOperationException.class, () -> map.remove("k")); } - public void testRemoveAbsentKey() { - var map = RequestParams.empty(); - assertThat(map.remove("missing"), nullValue()); - } - - public void testClear() { + public void testClearThrows() { var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); - map.clear(); - assertThat(map.isEmpty(), is(true)); + expectThrows(UnsupportedOperationException.class, map::clear); } // ------------------------------------------------------------------------- @@ -151,11 +133,10 @@ public void testEntrySetValuesAreLastValues() { assertThat(entry.getValue(), equalTo("last")); } - public void testEntrySetRemove() { + public void testEntrySetIteratorRemoveThrows() { var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); var it = map.entrySet().iterator(); it.next(); - it.remove(); - assertThat(map.size(), equalTo(1)); + expectThrows(UnsupportedOperationException.class, it::remove); } } From 3c6cc063688d99333f737af28fc7056a73cccfb9 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 13:59:02 +0100 Subject: [PATCH 09/29] Address code review findings in RequestParams - Cache empty RequestParams instance as a static constant - Replace assert with IllegalArgumentException for empty value lists in of() - Rename getSingle to requireSingle for clarity - Fix LinkedHashMap initial capacity formula (size/0.75+1 instead of size*2) - Expand get() Javadoc to document last-value-wins semantic - Make RestUtils.decodeQueryString return Map and wrap in RequestParams.of() in factory methods - Remove section-divider comments from source and test files --- .../org/elasticsearch/rest/RequestParams.java | 37 +++++++++++-------- .../org/elasticsearch/rest/RestUtils.java | 8 ++-- .../rest/RequestParamsTests.java | 37 ++++++------------- 3 files changed, 36 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 6e806d81830fa..fdfe24dccb418 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -35,18 +35,18 @@ */ public final class RequestParams extends AbstractMap { + private static final RequestParams EMPTY = new RequestParams(Map.of()); + /** Single backing store: key → non-empty ordered list of all values. */ private final LinkedHashMap> map; - // ------------------------------------------------------------------------- // Factory methods - // ------------------------------------------------------------------------- /** - * Returns a new, empty {@code RequestParams}. + * Returns a shared empty {@code RequestParams} instance. */ public static RequestParams empty() { - return new RequestParams(Map.of()); + return EMPTY; } /** @@ -54,8 +54,9 @@ public static RequestParams empty() { * The last value in each list is what {@link #get(Object)} returns. * * @param multiValues a map from parameter name to all its values, in encounter order + * @throws IllegalArgumentException if any value list is empty */ - public static RequestParams of(Map> multiValues) { + static RequestParams of(Map> multiValues) { return new RequestParams(multiValues); } @@ -67,7 +68,7 @@ public static RequestParams of(Map> multiValues) { * @return a {@code RequestParams} from parameter name to all its values, in encounter order */ public static RequestParams fromQueryString(String queryString) { - return RestUtils.decodeQueryString(queryString, 0); + return of(RestUtils.decodeQueryString(queryString, 0)); } /** @@ -79,7 +80,7 @@ public static RequestParams fromQueryString(String queryString) { */ public static RequestParams fromUrl(String url) { int index = url.indexOf('?'); - return index >= 0 ? RestUtils.decodeQueryString(url, index + 1) : RequestParams.empty(); + return index >= 0 ? of(RestUtils.decodeQueryString(url, index + 1)) : RequestParams.empty(); } /** @@ -102,21 +103,23 @@ public static RequestParams from(URI uri) { * @param singleValues a map whose values are treated as the sole value for each key */ public static RequestParams fromSingleValues(Map singleValues) { - LinkedHashMap> wrapped = new LinkedHashMap<>(singleValues.size() * 2); + LinkedHashMap> wrapped = new LinkedHashMap<>((int) (singleValues.size() / 0.75f) + 1); singleValues.forEach((k, v) -> wrapped.put(k, List.of(v))); return new RequestParams(wrapped); } private RequestParams(Map> multiValues) { - LinkedHashMap> copy = new LinkedHashMap<>(multiValues.size() * 2); - multiValues.forEach((k, v) -> copy.put(k, List.copyOf(v))); - assert copy.values().stream().allMatch(list -> list.isEmpty() == false) : "RequestParams requires every value list to be non-empty"; + LinkedHashMap> copy = new LinkedHashMap<>((int) (multiValues.size() / 0.75f) + 1); + multiValues.forEach((k, v) -> { + if (v.isEmpty()) { + throw new IllegalArgumentException("value list for parameter [" + k + "] must not be empty"); + } + copy.put(k, List.copyOf(v)); + }); this.map = copy; } - // ------------------------------------------------------------------------- // Multi-value API - // ------------------------------------------------------------------------- /** * Returns all values for {@code key} in the order they were added, @@ -138,7 +141,7 @@ public List getAll(String key) { * @return the single value, or {@code null} if absent * @throws IllegalArgumentException if the key has multiple values */ - public String getSingle(String key) { + public String requireSingle(String key) { var list = map.get(key); if (list == null) { return null; @@ -149,12 +152,14 @@ public String getSingle(String key) { return list.getFirst(); } - // ------------------------------------------------------------------------- // Map — single-value view over the backing list map - // ------------------------------------------------------------------------- /** * Returns the last value associated with {@code key}, or {@code null} if absent. + * + *

When a query parameter appears multiple times (e.g. {@code a=1&a=2}), this method returns + * the last value ({@code "2"}). Use {@link #getAll(String)} to retrieve all values, or + * {@link #requireSingle(String)} to assert that only one value is present. */ @Override public String get(Object key) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 64e30a7c08c92..0f122a45c9bec 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -45,20 +45,20 @@ public class RestUtils { public static final UnaryOperator REST_DECODER = RestUtils::decodeComponent; /** - * Parses a URL-encoded query string into a {@link RequestParams}, preserving all values for + * Parses a URL-encoded query string into a multi-value map, preserving all values for * repeated parameters (e.g. {@code match[]=foo&match[]=bar} → {@code ["foo", "bar"]}). * * @param s the full string containing the query string * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) - * @return a {@link RequestParams} from parameter name to all its values, in encounter order + * @return a map from parameter name to all its values, in encounter order */ - static RequestParams decodeQueryString(String s, int fromIndex) { + static Map> decodeQueryString(String s, int fromIndex) { Map> result = new LinkedHashMap<>(); parseQueryStringPairs(s, fromIndex, (name, value) -> { checkReservedParam(name); result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); }); - return RequestParams.of(result); + return result; } private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer consumer) { diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index cc9ec4318c4af..0a033ca4a1b78 100644 --- a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -20,10 +20,6 @@ public class RequestParamsTests extends ESTestCase { - // ------------------------------------------------------------------------- - // Factory methods - // ------------------------------------------------------------------------- - public void testEmpty() { var map = RequestParams.empty(); assertThat(map.isEmpty(), is(true)); @@ -39,6 +35,11 @@ public void testOf() { assertThat(map.getAll("b"), equalTo(List.of("3"))); } + public void testOfEmptyListThrows() { + var ex = expectThrows(IllegalArgumentException.class, () -> RequestParams.of(Map.of("k", List.of()))); + assertThat(ex.getMessage(), equalTo("value list for parameter [k] must not be empty")); + } + public void testFromSingleValues() { var map = RequestParams.fromSingleValues(Map.of("a", "1", "b", "2")); assertThat(map.size(), equalTo(2)); @@ -48,10 +49,6 @@ public void testFromSingleValues() { assertThat(map.getAll("b"), equalTo(List.of("2"))); } - // ------------------------------------------------------------------------- - // get / getAll - // ------------------------------------------------------------------------- - public void testGetReturnsLastValue() { var map = RequestParams.of(Map.of("k", List.of("first", "second", "last"))); assertThat(map.get("k"), equalTo("last")); @@ -72,30 +69,22 @@ public void testGetAllReturnsEmptyListForAbsentKey() { assertThat(map.getAll("missing"), equalTo(List.of())); } - // ------------------------------------------------------------------------- - // getSingle - // ------------------------------------------------------------------------- - - public void testGetSingleReturnsSingleValue() { + public void testRequireSingleReturnsSingleValue() { var map = RequestParams.of(Map.of("k", List.of("only"))); - assertThat(map.getSingle("k"), equalTo("only")); + assertThat(map.requireSingle("k"), equalTo("only")); } - public void testGetSingleReturnsNullForAbsentKey() { + public void testRequireSingleReturnsNullForAbsentKey() { var map = RequestParams.empty(); - assertThat(map.getSingle("missing"), nullValue()); + assertThat(map.requireSingle("missing"), nullValue()); } - public void testGetSingleThrowsOnMultipleValues() { + public void testRequireSingleThrowsOnMultipleValues() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - var ex = expectThrows(IllegalArgumentException.class, () -> map.getSingle("k")); + var ex = expectThrows(IllegalArgumentException.class, () -> map.requireSingle("k")); assertThat(ex.getMessage(), equalTo("parameter [k] must have a single value, but found: [a, b]")); } - // ------------------------------------------------------------------------- - // immutability - // ------------------------------------------------------------------------- - public void testPutThrows() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); expectThrows(UnsupportedOperationException.class, () -> map.put("k", "new")); @@ -111,10 +100,6 @@ public void testClearThrows() { expectThrows(UnsupportedOperationException.class, map::clear); } - // ------------------------------------------------------------------------- - // Map interface — containsKey, keySet, entrySet - // ------------------------------------------------------------------------- - public void testContainsKey() { var map = RequestParams.of(Map.of("present", List.of("v"))); assertThat(map.containsKey("present"), is(true)); From d77afa3d0b7684b30472f6127542d23b2a745a06 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 14:09:23 +0100 Subject: [PATCH 10/29] Throw RestRequest.BadParameterException from requireSingle --- .../java/org/elasticsearch/rest/RequestParams.java | 12 +++++------- .../org/elasticsearch/rest/RequestParamsTests.java | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index fdfe24dccb418..04fb6d6c0f468 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -24,10 +24,7 @@ * per key (e.g. repeated query parameters such as {@code match[]=foo&match[]=bar}). * *

Each key maps to a non-empty ordered list of values. The standard {@link Map} interface - * operates on the last value in that list: - *

    - *
  • {@link #get(Object)} returns the last value for a key, or {@code null} if absent.
    • - *
+ * operates on the last value in that list: {@link #get(Object)} returns the last value for a key, or {@code null} if absent. * Use {@link #getAll(String)} to retrieve all values for a repeated key. * *

Instances are immutable: mutation methods ({@code put}, {@code remove}, {@code clear}) @@ -135,11 +132,10 @@ public List getAll(String key) { /** * Returns the single value for {@code key}, or {@code null} if absent. - * Throws {@link IllegalArgumentException} if the key has more than one value. * * @param key the parameter name * @return the single value, or {@code null} if absent - * @throws IllegalArgumentException if the key has multiple values + * @throws RestRequest.BadParameterException if the key has multiple values */ public String requireSingle(String key) { var list = map.get(key); @@ -147,7 +143,9 @@ public String requireSingle(String key) { return null; } if (list.size() > 1) { - throw new IllegalArgumentException("parameter [" + key + "] must have a single value, but found: " + list); + throw new RestRequest.BadParameterException( + new IllegalArgumentException("parameter [" + key + "] must have a single value, but found: " + list) + ); } return list.getFirst(); } diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index 0a033ca4a1b78..d5fced3ed774d 100644 --- a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -81,7 +81,7 @@ public void testRequireSingleReturnsNullForAbsentKey() { public void testRequireSingleThrowsOnMultipleValues() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - var ex = expectThrows(IllegalArgumentException.class, () -> map.requireSingle("k")); + var ex = expectThrows(RestRequest.BadParameterException.class, () -> map.requireSingle("k")); assertThat(ex.getMessage(), equalTo("parameter [k] must have a single value, but found: [a, b]")); } From a638f860d01ec684b8b90fa451288752da283d69 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 14:11:17 +0100 Subject: [PATCH 11/29] Inline parseQueryStringPairs into decodeQueryString --- .../org/elasticsearch/rest/RestUtils.java | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 0f122a45c9bec..6f5ef56917a68 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -28,7 +28,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.BiConsumer; import java.util.function.UnaryOperator; import java.util.regex.Pattern; @@ -53,20 +52,9 @@ public class RestUtils { * @return a map from parameter name to all its values, in encounter order */ static Map> decodeQueryString(String s, int fromIndex) { - Map> result = new LinkedHashMap<>(); - parseQueryStringPairs(s, fromIndex, (name, value) -> { - checkReservedParam(name); - result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); - }); - return result; - } - - private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer consumer) { - if (fromIndex < 0) { - return; - } - if (fromIndex >= s.length()) { - return; + Map> params = new LinkedHashMap<>(); + if (fromIndex < 0 || fromIndex >= s.length()) { + return params; } int queryStringLength = s.contains("#") ? s.indexOf('#') : s.length(); @@ -87,9 +75,9 @@ private static void parseQueryStringPairs(String s, int fromIndex, BiConsumer= 0 ? of(RestUtils.decodeQueryString(url, index + 1)) : RequestParams.empty(); + public static RequestParams fromUri(String uri) { + int index = uri.indexOf('?'); + return index >= 0 ? of(RestUtils.decodeQueryString(uri, index + 1)) : RequestParams.empty(); } /** diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 80e716741fe9d..c306d219fece2 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -212,7 +212,7 @@ public static RestRequest request(XContentParserConfiguration parserConfig, Http private static RequestParams params(final String uri) { try { - return RequestParams.fromUrl(uri); + return RequestParams.fromUri(uri); } catch (final IllegalArgumentException e) { throw new BadParameterException(e); } diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index 99d286e6de75d..025a48699ca94 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -30,51 +30,51 @@ static char randomDelimiter() { return randomBoolean() ? '&' : ';'; } - public void testDecodeQueryStringFromUrl() { - var params = RequestParams.fromUrl("something?test=value"); + public void testDecodeQueryStringFromUri() { + var params = RequestParams.fromUri("something?test=value"); assertThat(params.size(), equalTo(1)); assertThat(params.get("test"), equalTo("value")); - params = RequestParams.fromUrl(Strings.format("something?test=value%ctest1=value1", randomDelimiter())); + params = RequestParams.fromUri(Strings.format("something?test=value%ctest1=value1", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("test"), equalTo("value")); assertThat(params.get("test1"), equalTo("value1")); // no query string - assertThat(RequestParams.fromUrl("something").isEmpty(), is(true)); + assertThat(RequestParams.fromUri("something").isEmpty(), is(true)); } - public void testDecodeQueryStringFromUrlEdgeCases() { + public void testDecodeQueryStringFromUriEdgeCases() { // empty query string - assertThat(RequestParams.fromUrl("something?").size(), equalTo(0)); + assertThat(RequestParams.fromUri("something?").size(), equalTo(0)); - assertThat(RequestParams.fromUrl(Strings.format("something?%c", randomDelimiter())).size(), equalTo(0)); + assertThat(RequestParams.fromUri(Strings.format("something?%c", randomDelimiter())).size(), equalTo(0)); - var params = RequestParams.fromUrl(Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter())); + var params = RequestParams.fromUri(Strings.format("something?p=v%c%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - assertThat(RequestParams.fromUrl("something?=").size(), equalTo(0)); + assertThat(RequestParams.fromUri("something?=").size(), equalTo(0)); - assertThat(RequestParams.fromUrl(Strings.format("something?%c=", randomDelimiter())).size(), equalTo(0)); + assertThat(RequestParams.fromUri(Strings.format("something?%c=", randomDelimiter())).size(), equalTo(0)); - params = RequestParams.fromUrl("something?a"); + params = RequestParams.fromUri("something?a"); assertThat(params.size(), equalTo(1)); assertThat(params.get("a"), equalTo("")); - params = RequestParams.fromUrl(Strings.format("something?p=v%ca", randomDelimiter())); + params = RequestParams.fromUri(Strings.format("something?p=v%ca", randomDelimiter())); assertThat(params.size(), equalTo(2)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); - params = RequestParams.fromUrl(Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter())); + params = RequestParams.fromUri(Strings.format("something?p=v%ca%cp1=v1", randomDelimiter(), randomDelimiter())); assertThat(params.size(), equalTo(3)); assertThat(params.get("a"), equalTo("")); assertThat(params.get("p"), equalTo("v")); assertThat(params.get("p1"), equalTo("v1")); - params = RequestParams.fromUrl( + params = RequestParams.fromUri( Strings.format("something?p=v%ca%cb%cp1=v1", randomDelimiter(), randomDelimiter(), randomDelimiter()) ); assertThat(params.size(), equalTo(4)); @@ -112,7 +112,7 @@ public void testDecodeQueryStringEdgeCases() { public void testDecodeQueryStringFragment() { // fragment should be excluded - var params = RequestParams.fromUrl("something?a=1#fragment"); + var params = RequestParams.fromUri("something?a=1#fragment"); assertThat(params.getAll("a"), equalTo(List.of("1"))); assertThat(params.containsKey("fragment"), is(false)); } @@ -160,7 +160,7 @@ public void testCrazyURL() { randomDelimiter(), randomDelimiter() ); - var params = RequestParams.fromUrl(uri); + var params = RequestParams.fromUri(uri); assertThat(params.get("/?:@-._~!$'()* ,"), equalTo("/?:@-._~!$'()* ,==")); assertThat(params.size(), equalTo(1)); } @@ -169,7 +169,7 @@ public void testReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> RequestParams.fromUrl("something?" + reservedParam + "=value") + () -> RequestParams.fromUri("something?" + reservedParam + "=value") ); assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index a265262969be1..e142d5fcccaea 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -357,7 +357,7 @@ public void testBuildLogoutResponse() throws Exception { final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RequestParams.fromUrl(endSessionUrl); + final var parameters = RequestParams.fromUri(endSessionUrl); assertThat(parameters, aMapWithSize(3)); assertThat(parameters, hasKey("id_token_hint")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -380,7 +380,7 @@ public void testBuildLogoutResponseFromEndsessionEndpointWithExistingParameters( final JWT idToken = generateIdToken(randomAlphaOfLength(8), randomAlphaOfLength(8), randomAlphaOfLength(8)); final OpenIdConnectLogoutResponse logoutResponse = realm.buildLogoutResponse(idToken); final String endSessionUrl = logoutResponse.getEndSessionUrl(); - final var parameters = RequestParams.fromUrl(endSessionUrl); + final var parameters = RequestParams.fromUri(endSessionUrl); assertThat(parameters, aMapWithSize(4)); assertThat(parameters, hasKey("parameter")); assertThat(parameters, hasKey("post_logout_redirect_uri")); @@ -453,9 +453,9 @@ private void assertEqualUrlStrings(String actual, String expected) { assertThat(endOfPath, greaterThan(-1)); assertThat(actual.substring(0, endOfPath + 1), equalTo(expected.substring(0, endOfPath + 1))); - final var actualParams = RequestParams.fromUrl(actual); + final var actualParams = RequestParams.fromUri(actual); - final var expectedParams = RequestParams.fromUrl(expected); + final var expectedParams = RequestParams.fromUri(expected); assertThat(actualParams, equalTo(expectedParams)); } From 8dd9c25842efc2cf04d1142a5167b788c611d0c1 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 15:03:53 +0100 Subject: [PATCH 18/29] Rename RestRequest paramAsList to repeatedParamAsList --- server/src/main/java/org/elasticsearch/rest/RestRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index c306d219fece2..f9425d986b0bb 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -428,7 +428,7 @@ public RequestParams params() { * @param key the parameter name * @return all values for the parameter, or an empty list if the parameter was not present */ - public List paramAsList(String key) { + public List repeatedParamAsList(String key) { consumedParams.add(key); return params.getAll(key); } From fcfefd281417f1e76d3492b57af44a42e9e9cb4e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 16:59:17 +0100 Subject: [PATCH 19/29] Update GCS fixture to use RequestParams.from(URI) instead of removed decodeQueryString overload --- .../fixture/gcs/GoogleCloudStorageHttpHandler.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java index dfbf5005c05e9..4f0405f74b1c6 100644 --- a/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java +++ b/test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java @@ -285,12 +285,12 @@ public void handle(final HttpExchange exchange) throws IOException { final String srcObject = URLDecoder.decode(matcher.group("srcObject"), UTF_8); final String dstObject = URLDecoder.decode(matcher.group("dstObject"), UTF_8); - final Map params = new HashMap<>(); - RestUtils.decodeQueryString(exchange.getRequestURI(), params); + final RequestParams params = RequestParams.from(exchange.getRequestURI()); final String rewriteToken = params.get("rewriteToken"); - final long maxBytesRewrittenPerCall = Long.parseLong( - params.getOrDefault("maxBytesRewrittenPerCall", String.valueOf(DEFAULT_MAX_BYTES_REWRITTEN_PER_CALL)) - ); + final String maxBytesStr = params.get("maxBytesRewrittenPerCall"); + final long maxBytesRewrittenPerCall = maxBytesStr != null + ? Long.parseLong(maxBytesStr) + : DEFAULT_MAX_BYTES_REWRITTEN_PER_CALL; var rewriteResponse = mockGcsBlobStore.rewrite(srcObject, dstObject, rewriteToken, maxBytesRewrittenPerCall); try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { From 3bea1e0706b9660647cd870ec2bca692ab707494 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 17:00:04 +0100 Subject: [PATCH 20/29] Fix setParamTrueOnceAndConsume to use a mutable overlay instead of mutating immutable RequestParams Add markerParams Set as an overlay for internal marker parameters (serverlessRequest, operatorRequest) so that setParamTrueOnceAndConsume no longer calls params.put() which throws UnsupportedOperationException on the immutable RequestParams. The overlay is propagated via the copy constructor and is visible through hasParam(), param(), and paramAsBoolean() (the ToXContent.Params interface). --- .../org/elasticsearch/rest/RestRequest.java | 21 ++--- .../elasticsearch/rest/RestRequestTests.java | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index f9425d986b0bb..85c64c31801a2 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -87,6 +87,7 @@ public class RestRequest implements ToXContent.Params, Traceable { private final XContentParserConfiguration parserConfig; private final RequestParams params; + private final Set markerParams = new HashSet<>(); private final Map> headers; private final String rawPath; private final Set consumedParams = new HashSet<>(); @@ -170,6 +171,7 @@ protected RestRequest(RestRequest other) { this.httpRequest = other.httpRequest; this.httpChannel = other.httpChannel; this.params = other.params; + this.markerParams.addAll(other.markerParams); this.rawPath = other.rawPath; this.headers = other.headers; this.requestId = other.requestId; @@ -397,23 +399,20 @@ public HttpRequest getHttpRequest() { } public final boolean hasParam(String key) { - return params.containsKey(key); + return params.containsKey(key) || markerParams.contains(key); } @Override public final String param(String key) { consumedParams.add(key); - return params.get(key); + String value = params.get(key); + return value != null ? value : (markerParams.contains(key) ? "true" : null); } @Override public final String param(String key, String defaultValue) { - consumedParams.add(key); - String value = params.get(key); - if (value == null) { - return defaultValue; - } - return value; + String value = param(key); + return value != null ? value : defaultValue; } public RequestParams params() { @@ -728,12 +727,10 @@ public boolean isOperatorRequest() { } private void setParamTrueOnceAndConsume(String param) { - if (params.containsKey(param)) { + if (hasParam(param)) { throw new IllegalArgumentException("The parameter [" + param + "] is already defined."); } - params.put(param, "true"); - // this parameter is intended be consumed via ToXContent.Params.param(..), not this.params(..) so don't require it is consumed here - consumedParams.add(param); + markerParams.add(param); } @Override diff --git a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java index 9cf5a7e7ad9f3..97833e298f812 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; @@ -332,6 +333,67 @@ public void testIsOperatorRequest() { assertThat(exception.getMessage(), is("The parameter [" + OPERATOR_REQUEST + "] is already defined.")); } + public void testSetParamTrueOnceAndConsume() { + // marker is visible via param(), hasParam(), and paramAsBoolean() + RestRequest request = contentRestRequest("content", new HashMap<>()); + assertFalse(request.hasParam(SERVERLESS_REQUEST)); + assertNull(request.param(SERVERLESS_REQUEST)); + assertFalse(request.paramAsBoolean(SERVERLESS_REQUEST, false)); + + request.markAsServerlessRequest(); + + assertTrue(request.hasParam(SERVERLESS_REQUEST)); + assertEquals("true", request.param(SERVERLESS_REQUEST)); + assertTrue(request.paramAsBoolean(SERVERLESS_REQUEST, false)); + assertTrue(request.isServerlessRequest()); + + // marker must NOT mutate the immutable RequestParams + assertFalse(request.params().containsKey(SERVERLESS_REQUEST)); + + // setting the same marker twice throws + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, request::markAsServerlessRequest); + assertThat(ex.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined.")); + + // setting a marker that is already present in the original params throws + RestRequest withParam = contentRestRequest("content", Map.of(SERVERLESS_REQUEST, "true")); + ex = expectThrows(IllegalArgumentException.class, withParam::markAsServerlessRequest); + assertThat(ex.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined.")); + } + + public void testSetParamTrueOnceAndConsumeVisibleAsToXContentParams() { + // verify markers are visible when the request is used as ToXContent.Params (the XContent serialization path) + RestRequest request = contentRestRequest("content", new HashMap<>()); + request.markAsServerlessRequest(); + + ToXContent.Params xContentParams = request; + assertEquals("true", xContentParams.param(SERVERLESS_REQUEST)); + assertEquals("true", xContentParams.param(SERVERLESS_REQUEST, "default")); + assertTrue(xContentParams.paramAsBoolean(SERVERLESS_REQUEST, false)); + + // absent marker still falls back to default + assertEquals("default", xContentParams.param(OPERATOR_REQUEST, "default")); + assertFalse(xContentParams.paramAsBoolean(OPERATOR_REQUEST, false)); + } + + public void testSetParamTrueOnceAndConsumeSerializationViaCopyConstructor() { + // marker set before copy is visible on the copy (serialization path) + RestRequest original = contentRestRequest("content", new HashMap<>()); + original.markAsServerlessRequest(); + original.markAsOperatorRequest(); + + RestRequest copy = new WrappedRestRequest(original); + + assertTrue(copy.hasParam(SERVERLESS_REQUEST)); + assertEquals("true", copy.param(SERVERLESS_REQUEST)); + assertTrue(copy.paramAsBoolean(SERVERLESS_REQUEST, false)); + assertTrue(copy.isServerlessRequest()); + + assertTrue(copy.hasParam(OPERATOR_REQUEST)); + assertEquals("true", copy.param(OPERATOR_REQUEST)); + assertTrue(copy.paramAsBoolean(OPERATOR_REQUEST, false)); + assertTrue(copy.isOperatorRequest()); + } + public static RestRequest contentRestRequest(String content, Map params) { Map> headers = new HashMap<>(); headers.put("Content-Type", Collections.singletonList("application/json")); @@ -346,6 +408,33 @@ private static RestRequest contentRestRequest(String content, Map Date: Tue, 24 Mar 2026 18:47:01 +0100 Subject: [PATCH 21/29] Restore mutable put() semantics on RequestParams, revert markerParams overlay Instead of keeping RequestParams immutable and adding overlay maps to work around callsites that legitimately mutate params (e.g. RestIndexAction, RestClusterRerouteAction, SettingsFilter, test code), restore the pre-PR mutable put() semantics on RequestParams directly. The value lists stored per key remain immutable (List.of()), so a non-null result from getAll() always guarantees getFirst()/getLast() are safe to call. The shared empty() singleton is replaced with a fresh instance per call to avoid shared-state mutation bugs. This also reverts the markerParams overlay commit: setParamTrueOnceAndConsume goes back to calling params.put() directly, and the markerParams field, copy-constructor propagation, and hasParam()/param() checks are all removed. --- .../org/elasticsearch/rest/RequestParams.java | 37 ++++---- .../org/elasticsearch/rest/RestRequest.java | 13 ++- .../elasticsearch/rest/RestRequestTests.java | 89 ------------------- 3 files changed, 22 insertions(+), 117 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index ccf05079baff6..33d5ad8c89fd2 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -24,26 +24,26 @@ * per key (e.g. repeated query parameters such as {@code match[]=foo&match[]=bar}). * *

Each key maps to a non-empty ordered list of values. The standard {@link Map} interface - * operates on the last value in that list: {@link #get(Object)} returns the last value for a key, or {@code null} if absent. + * operates on the last value in that list: {@link #get(Object)} returns the last value for a key, or {@code null} if absent, + * and {@link #put(String, String)} sets a key to a single value (stored as a one-element list, so {@link #getAll(String)} returns + * a singleton list after a {@code put}). * Use {@link #getAll(String)} to retrieve all values for a repeated key. * - *

Instances are immutable: mutation methods ({@code put}, {@code remove}, {@code clear}) - * throw {@link UnsupportedOperationException}. + *

The value lists returned by {@link #getAll(String)} are always non-empty and immutable, so + * {@link List#getFirst()} and {@link List#getLast()} are always safe to call on a non-{@code null} result. */ public final class RequestParams extends AbstractMap { - private static final RequestParams EMPTY = new RequestParams(Map.of()); - /** Single backing store: key → non-empty ordered list of all values. */ private final LinkedHashMap> map; // Factory methods /** - * Returns a shared empty {@code RequestParams} instance. + * Returns an empty {@code RequestParams} instance. */ public static RequestParams empty() { - return EMPTY; + return new RequestParams(Map.of()); } /** @@ -165,22 +165,17 @@ public String get(Object key) { return list == null ? null : list.getLast(); } - /** @throws UnsupportedOperationException always */ + /** + * Sets {@code key} to a single {@code value}, replacing any previous values for that key. + * The value is stored as a one-element immutable list, so {@link #getAll(String)} will return + * a singleton list and {@link List#getFirst()}/{@link List#getLast()} remain safe to call. + * + * @return the previous last value for {@code key}, or {@code null} if absent + */ @Override public String put(String key, String value) { - throw new UnsupportedOperationException("RequestParams is immutable"); - } - - /** @throws UnsupportedOperationException always */ - @Override - public String remove(Object key) { - throw new UnsupportedOperationException("RequestParams is immutable"); - } - - /** @throws UnsupportedOperationException always */ - @Override - public void clear() { - throw new UnsupportedOperationException("RequestParams is immutable"); + var prev = map.put(key, List.of(value)); + return prev == null ? null : prev.getLast(); } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 85c64c31801a2..f7de96121effe 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -87,7 +87,6 @@ public class RestRequest implements ToXContent.Params, Traceable { private final XContentParserConfiguration parserConfig; private final RequestParams params; - private final Set markerParams = new HashSet<>(); private final Map> headers; private final String rawPath; private final Set consumedParams = new HashSet<>(); @@ -171,7 +170,6 @@ protected RestRequest(RestRequest other) { this.httpRequest = other.httpRequest; this.httpChannel = other.httpChannel; this.params = other.params; - this.markerParams.addAll(other.markerParams); this.rawPath = other.rawPath; this.headers = other.headers; this.requestId = other.requestId; @@ -399,14 +397,13 @@ public HttpRequest getHttpRequest() { } public final boolean hasParam(String key) { - return params.containsKey(key) || markerParams.contains(key); + return params.containsKey(key); } @Override public final String param(String key) { consumedParams.add(key); - String value = params.get(key); - return value != null ? value : (markerParams.contains(key) ? "true" : null); + return params.get(key); } @Override @@ -727,10 +724,12 @@ public boolean isOperatorRequest() { } private void setParamTrueOnceAndConsume(String param) { - if (hasParam(param)) { + if (params.containsKey(param)) { throw new IllegalArgumentException("The parameter [" + param + "] is already defined."); } - markerParams.add(param); + params.put(param, "true"); + // this parameter is intended be consumed via ToXContent.Params.param(..), not this.params(..) so don't require it is consumed here + consumedParams.add(param); } @Override diff --git a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java index 97833e298f812..9cf5a7e7ad9f3 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java @@ -19,7 +19,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.xcontent.NamedXContentRegistry; -import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; @@ -333,67 +332,6 @@ public void testIsOperatorRequest() { assertThat(exception.getMessage(), is("The parameter [" + OPERATOR_REQUEST + "] is already defined.")); } - public void testSetParamTrueOnceAndConsume() { - // marker is visible via param(), hasParam(), and paramAsBoolean() - RestRequest request = contentRestRequest("content", new HashMap<>()); - assertFalse(request.hasParam(SERVERLESS_REQUEST)); - assertNull(request.param(SERVERLESS_REQUEST)); - assertFalse(request.paramAsBoolean(SERVERLESS_REQUEST, false)); - - request.markAsServerlessRequest(); - - assertTrue(request.hasParam(SERVERLESS_REQUEST)); - assertEquals("true", request.param(SERVERLESS_REQUEST)); - assertTrue(request.paramAsBoolean(SERVERLESS_REQUEST, false)); - assertTrue(request.isServerlessRequest()); - - // marker must NOT mutate the immutable RequestParams - assertFalse(request.params().containsKey(SERVERLESS_REQUEST)); - - // setting the same marker twice throws - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, request::markAsServerlessRequest); - assertThat(ex.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined.")); - - // setting a marker that is already present in the original params throws - RestRequest withParam = contentRestRequest("content", Map.of(SERVERLESS_REQUEST, "true")); - ex = expectThrows(IllegalArgumentException.class, withParam::markAsServerlessRequest); - assertThat(ex.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined.")); - } - - public void testSetParamTrueOnceAndConsumeVisibleAsToXContentParams() { - // verify markers are visible when the request is used as ToXContent.Params (the XContent serialization path) - RestRequest request = contentRestRequest("content", new HashMap<>()); - request.markAsServerlessRequest(); - - ToXContent.Params xContentParams = request; - assertEquals("true", xContentParams.param(SERVERLESS_REQUEST)); - assertEquals("true", xContentParams.param(SERVERLESS_REQUEST, "default")); - assertTrue(xContentParams.paramAsBoolean(SERVERLESS_REQUEST, false)); - - // absent marker still falls back to default - assertEquals("default", xContentParams.param(OPERATOR_REQUEST, "default")); - assertFalse(xContentParams.paramAsBoolean(OPERATOR_REQUEST, false)); - } - - public void testSetParamTrueOnceAndConsumeSerializationViaCopyConstructor() { - // marker set before copy is visible on the copy (serialization path) - RestRequest original = contentRestRequest("content", new HashMap<>()); - original.markAsServerlessRequest(); - original.markAsOperatorRequest(); - - RestRequest copy = new WrappedRestRequest(original); - - assertTrue(copy.hasParam(SERVERLESS_REQUEST)); - assertEquals("true", copy.param(SERVERLESS_REQUEST)); - assertTrue(copy.paramAsBoolean(SERVERLESS_REQUEST, false)); - assertTrue(copy.isServerlessRequest()); - - assertTrue(copy.hasParam(OPERATOR_REQUEST)); - assertEquals("true", copy.param(OPERATOR_REQUEST)); - assertTrue(copy.paramAsBoolean(OPERATOR_REQUEST, false)); - assertTrue(copy.isOperatorRequest()); - } - public static RestRequest contentRestRequest(String content, Map params) { Map> headers = new HashMap<>(); headers.put("Content-Type", Collections.singletonList("application/json")); @@ -408,33 +346,6 @@ private static RestRequest contentRestRequest(String content, Map Date: Tue, 24 Mar 2026 18:49:50 +0100 Subject: [PATCH 22/29] Move BadParameterException wrapping into RequestParams.fromQueryString Catch IllegalArgumentException in RequestParams.fromQueryString and rethrow as RestRequest.BadParameterException, matching the existing pattern in requireSingle(). This makes the private static RestRequest.params(String) wrapper redundant, so remove it and call RequestParams.fromUri() directly from RestRequest.request(). --- .../java/org/elasticsearch/rest/RequestParams.java | 6 +++++- .../main/java/org/elasticsearch/rest/RestRequest.java | 10 +--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 33d5ad8c89fd2..a52444be954b1 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -65,7 +65,11 @@ static RequestParams of(Map> multiValues) { * @return a {@code RequestParams} from parameter name to all its values, in encounter order */ public static RequestParams fromQueryString(String queryString) { - return of(RestUtils.decodeQueryString(queryString, 0)); + try { + return of(RestUtils.decodeQueryString(queryString, 0)); + } catch (IllegalArgumentException e) { + throw new RestRequest.BadParameterException(e); + } } /** diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index f7de96121effe..3ab35e6594a6f 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -198,7 +198,7 @@ protected RestRequest(RestRequest other) { * @throws MediaTypeHeaderException if the Content-Type or Accept header can not be parsed */ public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) { - RequestParams params = params(httpRequest.uri()); + RequestParams params = RequestParams.fromUri(httpRequest.uri()); return new RestRequest( parserConfig, params, @@ -210,14 +210,6 @@ public static RestRequest request(XContentParserConfiguration parserConfig, Http ); } - private static RequestParams params(final String uri) { - try { - return RequestParams.fromUri(uri); - } catch (final IllegalArgumentException e) { - throw new BadParameterException(e); - } - } - /** * Creates a new REST request. The path is not decoded so this constructor will not throw a * {@link BadParameterException}. From 201b5a52e9602b75c1c99c2ff56ade0e47844728 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 18:51:41 +0100 Subject: [PATCH 23/29] Fix fromUri(String) to delegate to fromQueryString for BadParameterException wrapping fromUri called decodeQueryString directly, bypassing the try/catch added in fromQueryString. Delegate to fromQueryString instead so all three parsing entry points (fromQueryString, fromUri, from(URI)) consistently wrap IllegalArgumentException as BadParameterException. --- server/src/main/java/org/elasticsearch/rest/RequestParams.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index a52444be954b1..eeab8f70e1fa5 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -81,7 +81,7 @@ public static RequestParams fromQueryString(String queryString) { */ public static RequestParams fromUri(String uri) { int index = uri.indexOf('?'); - return index >= 0 ? of(RestUtils.decodeQueryString(uri, index + 1)) : RequestParams.empty(); + return index >= 0 ? fromQueryString(uri.substring(index + 1)) : RequestParams.empty(); } /** From aded23803470ce2e866d4b3d996d0f1ad75c7ffc Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 18:53:21 +0100 Subject: [PATCH 24/29] Add @throws BadParameterException to RequestParams parsing factory methods --- server/src/main/java/org/elasticsearch/rest/RequestParams.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index eeab8f70e1fa5..3998e3b98ebf8 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -63,6 +63,7 @@ static RequestParams of(Map> multiValues) { * * @param queryString the raw query string (the part after {@code ?}, without the {@code ?} itself) * @return a {@code RequestParams} from parameter name to all its values, in encounter order + * @throws RestRequest.BadParameterException if the query string cannot be decoded */ public static RequestParams fromQueryString(String queryString) { try { @@ -78,6 +79,7 @@ public static RequestParams fromQueryString(String queryString) { * * @param uri a URI string, e.g. {@code /index/_search?pretty&size=10} * @return a {@code RequestParams} from parameter name to all its values, in encounter order + * @throws RestRequest.BadParameterException if the query string cannot be decoded */ public static RequestParams fromUri(String uri) { int index = uri.indexOf('?'); @@ -90,6 +92,7 @@ public static RequestParams fromUri(String uri) { * * @param uri the URI whose raw query string is parsed * @return a {@code RequestParams} from parameter name to all its values, in encounter order + * @throws RestRequest.BadParameterException if the query string cannot be decoded */ public static RequestParams from(URI uri) { final var rawQuery = uri.getRawQuery(); From 994884729b936b94c932e8fc47f639f62207a6bd Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 25 Mar 2026 07:48:51 +0100 Subject: [PATCH 25/29] Override remove(), clear(), and entrySet iterator remove() in RequestParams AbstractMap's default implementations of these methods iterate the entry set, which failed with UnsupportedOperationException before this change. Now all three delegate directly to the backing LinkedHashMap, fixing the crash observed via RestController.clear() in integration tests. --- .../org/elasticsearch/rest/RequestParams.java | 24 ++++++++++++- .../rest/RequestParamsTests.java | 36 ++++++++++++++----- 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 3998e3b98ebf8..29445fa484078 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -190,6 +190,17 @@ public int size() { return map.size(); } + @Override + public String remove(Object key) { + var prev = map.remove(key); + return prev == null ? null : prev.getLast(); + } + + @Override + public void clear() { + map.clear(); + } + @Override public boolean containsKey(Object key) { return map.containsKey(key); @@ -201,7 +212,8 @@ public Set keySet() { } /** - * Returns an unmodifiable entry set where each entry's value is the last value for that key. + * Returns an entry set where each entry's value is the last value for that key. + * Supports removal via the iterator, which removes the entire key from the underlying map. */ @Override public Set> entrySet() { @@ -220,6 +232,11 @@ public Entry next() { var e = inner.next(); return Map.entry(e.getKey(), e.getValue().getLast()); } + + @Override + public void remove() { + inner.remove(); + } }; } @@ -227,6 +244,11 @@ public Entry next() { public int size() { return map.size(); } + + @Override + public void clear() { + map.clear(); + } }; } } diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index 20a8e7be1941d..1d53e64934056 100644 --- a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -86,19 +86,36 @@ public void testRequireSingleThrowsOnMultipleValues() { assertThat(ex.getMessage(), containsString("parameter [k] must have a single value, but found: [a, b]")); } - public void testPutThrows() { + public void testPut() { var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - expectThrows(UnsupportedOperationException.class, () -> map.put("k", "new")); + assertThat(map.put("k", "new"), equalTo("b")); // returns previous last value + assertThat(map.get("k"), equalTo("new")); + assertThat(map.getAll("k"), equalTo(List.of("new"))); } - public void testRemoveThrows() { - var map = RequestParams.of(Map.of("k", List.of("a", "b"))); - expectThrows(UnsupportedOperationException.class, () -> map.remove("k")); + public void testPutNewKey() { + var map = RequestParams.of(Map.of("k", List.of("v"))); + assertThat(map.put("new", "val"), nullValue()); + assertThat(map.get("new"), equalTo("val")); + } + + public void testRemove() { + var map = RequestParams.of(Map.of("a", List.of("1", "last"), "b", List.of("2"))); + assertThat(map.remove("a"), equalTo("last")); // returns previous last value + assertThat(map.containsKey("a"), is(false)); + assertThat(map.size(), equalTo(1)); } - public void testClearThrows() { + public void testRemoveAbsentKey() { + var map = RequestParams.of(Map.of("k", List.of("v"))); + assertThat(map.remove("missing"), nullValue()); + assertThat(map.size(), equalTo(1)); + } + + public void testClear() { var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); - expectThrows(UnsupportedOperationException.class, map::clear); + map.clear(); + assertThat(map.isEmpty(), is(true)); } public void testContainsKey() { @@ -119,10 +136,11 @@ public void testEntrySetValuesAreLastValues() { assertThat(entry.getValue(), equalTo("last")); } - public void testEntrySetIteratorRemoveThrows() { + public void testEntrySetIteratorRemove() { var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); var it = map.entrySet().iterator(); it.next(); - expectThrows(UnsupportedOperationException.class, it::remove); + it.remove(); + assertThat(map.size(), equalTo(1)); } } From 144a1f985dc37c9ba47bd67542f00b5a6a0377aa Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 25 Mar 2026 08:01:52 +0100 Subject: [PATCH 26/29] Use Maps.newLinkedHashMapWithExpectedSize --- .../src/main/java/org/elasticsearch/rest/RequestParams.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 29445fa484078..85551d4127254 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -9,6 +9,8 @@ package org.elasticsearch.rest; +import org.elasticsearch.common.util.Maps; + import java.net.URI; import java.util.AbstractMap; import java.util.AbstractSet; @@ -107,13 +109,13 @@ public static RequestParams from(URI uri) { * @param singleValues a map whose values are treated as the sole value for each key */ public static RequestParams fromSingleValues(Map singleValues) { - LinkedHashMap> wrapped = new LinkedHashMap<>((int) (singleValues.size() / 0.75f) + 1); + LinkedHashMap> wrapped = Maps.newLinkedHashMapWithExpectedSize(singleValues.size()); singleValues.forEach((k, v) -> wrapped.put(k, List.of(v))); return new RequestParams(wrapped); } private RequestParams(Map> multiValues) { - LinkedHashMap> copy = new LinkedHashMap<>((int) (multiValues.size() / 0.75f) + 1); + LinkedHashMap> copy = Maps.newLinkedHashMapWithExpectedSize(multiValues.size()); multiValues.forEach((k, v) -> { if (v.isEmpty()) { throw new IllegalArgumentException("value list for parameter [" + k + "] must not be empty"); From 42a651bf316be38885c5b30b44ed69b029283f94 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 25 Mar 2026 09:06:08 +0100 Subject: [PATCH 27/29] Fix fromSingleValues null handling and update stale test assertions fromSingleValues now maps null values to "" to match the previous Map behaviour where null meant a valueless parameter. RestUtilsTests.testReservedParameters updated to expect BadParameterException (wrapping IllegalArgumentException) since fromUri now wraps all parse errors in BadParameterException. --- .../main/java/org/elasticsearch/rest/RequestParams.java | 7 ++++++- .../test/java/org/elasticsearch/rest/RestUtilsTests.java | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 85551d4127254..101bbd8027cde 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -110,7 +110,7 @@ public static RequestParams from(URI uri) { */ public static RequestParams fromSingleValues(Map singleValues) { LinkedHashMap> wrapped = Maps.newLinkedHashMapWithExpectedSize(singleValues.size()); - singleValues.forEach((k, v) -> wrapped.put(k, List.of(v))); + singleValues.forEach((k, v) -> wrapped.put(k, Collections.singletonList(v))); return new RequestParams(wrapped); } @@ -125,6 +125,11 @@ private RequestParams(Map> multiValues) { this.map = copy; } + /** Wraps an already-validated map directly, without copying. Used by {@link #fromSingleValues}. */ + private RequestParams(LinkedHashMap> validatedMap) { + this.map = validatedMap; + } + // Multi-value API /** diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index 025a48699ca94..c81a25e31512b 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -167,11 +167,11 @@ public void testCrazyURL() { public void testReservedParameters() { for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) { - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, + RestRequest.BadParameterException exception = expectThrows( + RestRequest.BadParameterException.class, () -> RequestParams.fromUri("something?" + reservedParam + "=value") ); - assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); + assertEquals(exception.getCause().getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set"); } } From de9bbf85c9caa696df23c9992d8fd5749c595ff0 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 25 Mar 2026 09:51:31 +0100 Subject: [PATCH 28/29] Use RequestParams in RestUtils.decodeQueryString and addParam - decodeQueryString now returns RequestParams directly, built via the new package-private addValue() method, eliminating the intermediate Map> - addParam takes RequestParams instead of Map> - fromQueryString no longer needs the of() wrapping step - Backing field widened to Map>; copy logic moved into of() - getAll() returns Collections.unmodifiableList to prevent callers mutating the backing list - put() uses Collections.singletonList (no ArrayList needed for replacements) --- .../org/elasticsearch/rest/RequestParams.java | 46 +++++++++---------- .../org/elasticsearch/rest/RestUtils.java | 14 ++---- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java index 101bbd8027cde..d8bc1a5f16b3d 100644 --- a/server/src/main/java/org/elasticsearch/rest/RequestParams.java +++ b/server/src/main/java/org/elasticsearch/rest/RequestParams.java @@ -14,6 +14,7 @@ import java.net.URI; import java.util.AbstractMap; import java.util.AbstractSet; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; @@ -37,7 +38,7 @@ public final class RequestParams extends AbstractMap { /** Single backing store: key → non-empty ordered list of all values. */ - private final LinkedHashMap> map; + private final Map> map; // Factory methods @@ -45,7 +46,7 @@ public final class RequestParams extends AbstractMap { * Returns an empty {@code RequestParams} instance. */ public static RequestParams empty() { - return new RequestParams(Map.of()); + return new RequestParams(new LinkedHashMap<>()); } /** @@ -56,7 +57,14 @@ public static RequestParams empty() { * @throws IllegalArgumentException if any value list is empty */ static RequestParams of(Map> multiValues) { - return new RequestParams(multiValues); + LinkedHashMap> copy = Maps.newLinkedHashMapWithExpectedSize(multiValues.size()); + multiValues.forEach((k, v) -> { + if (v.isEmpty()) { + throw new IllegalArgumentException("value list for parameter [" + k + "] must not be empty"); + } + copy.put(k, List.copyOf(v)); + }); + return new RequestParams(copy); } /** @@ -69,7 +77,7 @@ static RequestParams of(Map> multiValues) { */ public static RequestParams fromQueryString(String queryString) { try { - return of(RestUtils.decodeQueryString(queryString, 0)); + return RestUtils.decodeQueryString(queryString, 0); } catch (IllegalArgumentException e) { throw new RestRequest.BadParameterException(e); } @@ -114,34 +122,28 @@ public static RequestParams fromSingleValues(Map singleValues) { return new RequestParams(wrapped); } - private RequestParams(Map> multiValues) { - LinkedHashMap> copy = Maps.newLinkedHashMapWithExpectedSize(multiValues.size()); - multiValues.forEach((k, v) -> { - if (v.isEmpty()) { - throw new IllegalArgumentException("value list for parameter [" + k + "] must not be empty"); - } - copy.put(k, List.copyOf(v)); - }); - this.map = copy; + private RequestParams(Map> map) { + this.map = map; } - /** Wraps an already-validated map directly, without copying. Used by {@link #fromSingleValues}. */ - private RequestParams(LinkedHashMap> validatedMap) { - this.map = validatedMap; + /** + * Appends {@code value} to the list of values for {@code key}. + * If the key is not yet present, a new entry is created. + */ + void addValue(String key, String value) { + map.computeIfAbsent(key, k -> new ArrayList<>(1)).add(value); } - // Multi-value API - /** * Returns all values for {@code key} in the order they were added, * or an empty list if the key is absent. * * @param key the parameter name - * @return a non-empty list of all values, or an empty list if absent; never {@code null} + * @return an unmodifiable non-empty list of all values, or an empty list if absent; never {@code null} */ public List getAll(String key) { var list = map.get(key); - return list == null ? List.of() : list; + return list == null ? List.of() : Collections.unmodifiableList(list); } /** @@ -164,8 +166,6 @@ public String requireSingle(String key) { return list.getFirst(); } - // Map — single-value view over the backing list map - /** * Returns the last value associated with {@code key}, or {@code null} if absent. * @@ -188,7 +188,7 @@ public String get(Object key) { */ @Override public String put(String key, String value) { - var prev = map.put(key, List.of(value)); + var prev = map.put(key, Collections.singletonList(value)); return prev == null ? null : prev.getLast(); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 751d8b22bc2ed..744612fbca374 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -22,11 +22,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.function.UnaryOperator; import java.util.regex.Pattern; @@ -49,10 +45,10 @@ public class RestUtils { * * @param s the full string containing the query string * @param fromIndex the index at which the query string begins (i.e. one past the {@code ?}) - * @return a map from parameter name to all its values, in encounter order + * @return a {@link RequestParams} from parameter name to all its values, in encounter order */ - static Map> decodeQueryString(String s, int fromIndex) { - Map> params = new LinkedHashMap<>(); + static RequestParams decodeQueryString(String s, int fromIndex) { + RequestParams params = RequestParams.empty(); if (fromIndex < 0 || fromIndex >= s.length()) { return params; } @@ -101,13 +97,13 @@ private static String decodeQueryStringParam(final String s) { return decodeComponent(s, StandardCharsets.UTF_8, true); } - private static void addParam(Map> result, String name, String value) { + private static void addParam(RequestParams result, String name, String value) { for (var reservedParameter : INTERNAL_MARKER_REQUEST_PARAMETERS) { if (reservedParameter.equalsIgnoreCase(name)) { throw new IllegalArgumentException("parameter [" + name + "] is reserved and may not be set"); } } - result.computeIfAbsent(name, k -> new ArrayList<>()).add(value); + result.addValue(name, value); } /** From e708c6478795671877c25644bd953c340d95f352 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 27 Mar 2026 12:58:33 +0100 Subject: [PATCH 29/29] Add missing test coverage per review feedback - Cover hasNext() in entrySet iterator test - Add testEntrySetClear() for entrySet().clear() - Add testAddValue() for the package-private addValue() method - Add testFromUri() and testFrom() for the URI factory methods - Add testDecodeQueryStringMultipleValuesUnadorned() for bare repeated params --- .../rest/RequestParamsTests.java | 37 ++++++++++++++++++- .../elasticsearch/rest/RestUtilsTests.java | 6 +++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java index 1d53e64934056..276b44a93cb4d 100644 --- a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.test.ESTestCase; +import java.net.URI; import java.util.List; import java.util.Map; @@ -131,9 +132,12 @@ public void testKeySet() { public void testEntrySetValuesAreLastValues() { var map = RequestParams.of(Map.of("k", List.of("first", "last"))); - var entry = map.entrySet().iterator().next(); + var iterator = map.entrySet().iterator(); + assertThat(iterator.hasNext(), is(true)); + var entry = iterator.next(); assertThat(entry.getKey(), equalTo("k")); assertThat(entry.getValue(), equalTo("last")); + assertThat(iterator.hasNext(), is(false)); } public void testEntrySetIteratorRemove() { @@ -143,4 +147,35 @@ public void testEntrySetIteratorRemove() { it.remove(); assertThat(map.size(), equalTo(1)); } + + public void testEntrySetClear() { + var map = RequestParams.of(Map.of("a", List.of("1"), "b", List.of("2"))); + map.entrySet().clear(); + assertThat(map.isEmpty(), is(true)); + } + + public void testAddValue() { + var map = RequestParams.empty(); + map.addValue("k", "first"); + assertThat(map.getAll("k"), equalTo(List.of("first"))); + map.addValue("k", "second"); + assertThat(map.getAll("k"), equalTo(List.of("first", "second"))); + assertThat(map.get("k"), equalTo("second")); + } + + public void testFromUri() { + var map = RequestParams.fromUri("something?a=1&b=2"); + assertThat(map.size(), equalTo(2)); + assertThat(map.get("a"), equalTo("1")); + assertThat(map.get("b"), equalTo("2")); + assertThat(RequestParams.fromUri("something").isEmpty(), is(true)); + } + + public void testFrom() throws Exception { + var map = RequestParams.from(new URI("http://example.com/path?a=1&b=2")); + assertThat(map.size(), equalTo(2)); + assertThat(map.get("a"), equalTo("1")); + assertThat(map.get("b"), equalTo("2")); + assertThat(RequestParams.from(new URI("http://example.com/path")).isEmpty(), is(true)); + } } diff --git a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java index c81a25e31512b..5b500ca8a6a60 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java @@ -96,6 +96,12 @@ public void testDecodeQueryStringMultipleValues() { assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); } + public void testDecodeQueryStringMultipleValuesUnadorned() { + var params = RequestParams.fromQueryString("match=up&match=http_requests_total&start=1609746000"); + assertThat(params.getAll("match"), equalTo(List.of("up", "http_requests_total"))); + assertThat(params.getAll("start"), equalTo(List.of("1609746000"))); + } + public void testDecodeQueryStringDelimiters() { var params = RequestParams.fromQueryString(Strings.format("a=1%cb=2", randomDelimiter())); assertThat(params.getAll("a"), equalTo(List.of("1")));