Preserve multi-value request params through PathTrie iteration

felixbarny · felixbarny · commit 8d185e6aece2 · 2026-03-30T18:41:08.000+02:00
RestController#getAllHandlers was using Map.copyOf(requestParamsRef) to
snapshot the original params before PathTrie iterations. Map.copyOf only
sees the last value per key (via entrySet), silently dropping earlier
values for repeated parameters like ?format=json&amp;format=yaml.

- Add RequestParams.copyOf(RequestParams) to deep-copy all multi-values
- Add RequestParams.putAll(RequestParams) overload that copies every
  value in each key's list, unlike the inherited putAll(Map) which only
  sees the last value per key via entrySet
- Change getAllHandlers to accept RequestParams instead of Map&lt;String,String&gt;
  and use the new methods
- Add tests for copyOf, putAll(RequestParams), and a regression test in
  RestControllerTests
diff --git a/server/src/main/java/org/elasticsearch/rest/RequestParams.java b/server/src/main/java/org/elasticsearch/rest/RequestParams.java
@@ -122,10 +122,31 @@ public static RequestParams fromSingleValues(Map<String, String> singleValues) {
         return new RequestParams(wrapped);
     }
 
+    /**
+     * Returns an independent copy of {@code source}, preserving all multi-values.
+     *
+     * @param source the {@code RequestParams} to copy
+     * @return a new mutable {@code RequestParams} with the same contents as {@code source}
+     */
+    public static RequestParams copyOf(RequestParams source) {
+        LinkedHashMap<String, List<String>> copy = Maps.newLinkedHashMapWithExpectedSize(source.map.size());
+        source.map.forEach((k, v) -> copy.put(k, List.copyOf(v)));
+        return new RequestParams(copy);
+    }
+
     private RequestParams(Map<String, List<String>> map) {
         this.map = map;
     }
 
+    /**
+     * Copies all entries from {@code source} into this map, preserving all multi-values.
+     * Unlike the inherited {@link #putAll(Map)}, which only sees the last value per key,
+     * this overload copies every value in each key's list.
+     */
+    public void putAll(RequestParams source) {
+        source.map.forEach(map::put);
+    }
+
     /**
      * Appends {@code value} to the list of values for {@code key}.
      * If the key is not yet present, a new entry is created.
diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java
@@ -650,14 +650,14 @@ private static void validateErrorTrace(RestRequest request, RestChannel channel)
         }
     }
 
-    Iterator<MethodHandlers> getAllHandlers(@Nullable Map<String, String> requestParamsRef, String rawPath) {
+    Iterator<MethodHandlers> getAllHandlers(@Nullable RequestParams requestParamsRef, String rawPath) {
         final Supplier<Map<String, String>> paramsSupplier;
         if (requestParamsRef == null) {
             paramsSupplier = () -> null;
         } else {
             // Between retrieving the correct path, we need to reset the parameters,
             // otherwise parameters are parsed out of the URI that aren't actually handled.
-            final Map<String, String> originalParams = Map.copyOf(requestParamsRef);
+            final RequestParams originalParams = RequestParams.copyOf(requestParamsRef);
             paramsSupplier = () -> {
                 // PathTrie modifies the request, so reset the params between each iteration
                 requestParamsRef.clear();
diff --git a/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java b/server/src/test/java/org/elasticsearch/rest/RequestParamsTests.java
@@ -178,4 +178,43 @@ public void testFrom() throws Exception {
         assertThat(map.get("b"), equalTo("2"));
         assertThat(RequestParams.from(new URI("http://example.com/path")).isEmpty(), is(true));
     }
+
+    public void testCopyOfPreservesMultiValues() {
+        var original = RequestParams.of(Map.of("format", List.of("json", "yaml"), "size", List.of("10")));
+        var copy = RequestParams.copyOf(original);
+        assertThat(copy.getAll("format"), equalTo(List.of("json", "yaml")));
+        assertThat(copy.get("format"), equalTo("yaml"));
+        assertThat(copy.getAll("size"), equalTo(List.of("10")));
+        assertThat(copy.size(), equalTo(2));
+    }
+
+    public void testCopyOfIsIndependentOfSource() {
+        var original = RequestParams.of(Map.of("format", List.of("json", "yaml")));
+        var copy = RequestParams.copyOf(original);
+        original.clear();
+        assertThat(copy.getAll("format"), equalTo(List.of("json", "yaml")));
+        assertThat(original.isEmpty(), is(true));
+    }
+
+    public void testCopyOfSourceDoesNotReflectMutationsToTheCopy() {
+        var original = RequestParams.of(Map.of("format", List.of("json", "yaml")));
+        var copy = RequestParams.copyOf(original);
+        copy.put("format", "cbor");
+        assertThat(original.getAll("format"), equalTo(List.of("json", "yaml")));
+    }
+
+    public void testPutAllRequestParamsPreservesMultiValues() {
+        var source = RequestParams.of(Map.of("format", List.of("json", "yaml")));
+        var target = RequestParams.empty();
+        target.putAll(source);
+        assertThat(target.getAll("format"), equalTo(List.of("json", "yaml")));
+    }
+
+    public void testPutAllRequestParamsMergesIntoExistingEntries() {
+        var source = RequestParams.of(Map.of("format", List.of("json", "yaml")));
+        var target = RequestParams.of(Map.of("size", List.of("10")));
+        target.putAll(source);
+        assertThat(target.getAll("format"), equalTo(List.of("json", "yaml")));
+        assertThat(target.getAll("size"), equalTo(List.of("10")));
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java
@@ -1188,6 +1188,19 @@ public void testApiProtectionWithServerlessEnabledAsEndUser() {
         checkUnprotected.accept(inaccessiblePaths);
     }
 
+    public void testGetAllHandlersPreservesMultiValueParams() {
+        restController.registerHandler(new Route(GET, "/{index}"), (request, channel, client) -> {});
+
+        var params = RequestParams.of(Map.of("format", List.of("json", "yaml")));
+        var it = restController.getAllHandlers(params, "/my-index");
+        while (it.hasNext()) {
+            it.next();
+        }
+
+        // Multi-values must survive the per-iteration reset that PathTrie triggers
+        assertThat(params.getAll("format"), equalTo(List.of("json", "yaml")));
+    }
+
     @ServerlessScope(Scope.PUBLIC)
     private static final class PublicRestHandler extends BaseRestHandler {
         @Override
