Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/RequestParams.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RequestParams;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.Scope;
import org.elasticsearch.rest.ServerlessScope;
Expand Down Expand Up @@ -74,10 +73,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
String labelName = PrometheusLabelNameUtils.decodeLabelName(rawName);
String index = request.param(INDEX_PARAM, "*");

// Consume the parameter; re-parse from the raw URI to handle repeated match[] params,
// since request processing currently keeps only the last value for repeated parameters.
request.repeatedParamAsList(MATCH_PARAM);
List<String> matchSelectors = RequestParams.fromUri(request.uri()).getAll(MATCH_PARAM);
List<String> matchSelectors = request.repeatedParamAsList(MATCH_PARAM);

// Time range
String endParam = request.param(END_PARAM);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RequestParams;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.Scope;
import org.elasticsearch.rest.ServerlessScope;
Expand Down Expand Up @@ -60,10 +59,7 @@ public List<Route> routes() {

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
// Consume the parameter; re-parse from the raw URI to handle repeated match[] params,
// since request processing currently keeps only the last value for repeated parameters.
request.repeatedParamAsList(MATCH_PARAM);
List<String> matchSelectors = RequestParams.fromUri(request.uri()).getAll(MATCH_PARAM);
List<String> matchSelectors = request.repeatedParamAsList(MATCH_PARAM);

// Time range
String endParam = request.param(END_PARAM);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RequestParams;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.Scope;
import org.elasticsearch.rest.ServerlessScope;
Expand Down Expand Up @@ -58,10 +57,7 @@ public List<Route> routes() {

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
// Consume the parameter; re-parse from the raw URI to handle repeated match[] params,
// since request processing currently keeps only the last value for repeated parameters.
request.repeatedParamAsList(MATCH_PARAM);
List<String> matchSelectors = RequestParams.fromUri(request.uri()).getAll(MATCH_PARAM);
List<String> matchSelectors = request.repeatedParamAsList(MATCH_PARAM);
if (matchSelectors.isEmpty()) {
throw new IllegalArgumentException("At least one [match[]] selector is required");
}
Expand Down
Loading