Browse Source

[8.x] Assert that REST params are consumed iff supported (#114040) (#114087)

* Assert that REST params are consumed iff supported (#114040)

REST APIs which declare their supported parameters must consume exactly
those parameters: consuming an unsupported parameter means that requests
including that parameter will be rejected, whereas failing to consume a
supported parameter means that this parameter has no effect and should
be removed.

This commit adds an assertion to verify that we are consuming the
correct parameters.

Closes #113854

* CI poke
David Turner 1 year ago
parent
commit
5a5b14dde6

+ 3 - 2
modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestGetDataStreamsAction.java

@@ -11,6 +11,7 @@ package org.elasticsearch.datastreams.rest;
 import org.elasticsearch.action.datastreams.GetDataStreamAction;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.internal.node.NodeClient;
+import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.util.set.Sets;
@@ -35,14 +36,14 @@ public class RestGetDataStreamsAction extends BaseRestHandler {
             Set.of(
                 "name",
                 "include_defaults",
-                "timeout",
                 "master_timeout",
                 IndicesOptions.WildcardOptions.EXPAND_WILDCARDS,
                 IndicesOptions.ConcreteTargetOptions.IGNORE_UNAVAILABLE,
                 IndicesOptions.WildcardOptions.ALLOW_NO_INDICES,
                 IndicesOptions.GatekeeperOptions.IGNORE_THROTTLED,
                 "verbose"
-            )
+            ),
+            DataStream.isFailureStoreFeatureFlagEnabled() ? Set.of(IndicesOptions.FailureStoreOptions.FAILURE_STORE) : Set.of()
         )
     );
 

+ 17 - 0
server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java

@@ -16,6 +16,7 @@ import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.core.CheckedConsumer;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.RefCounted;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.RestApiVersion;
@@ -104,6 +105,8 @@ public abstract class BaseRestHandler implements RestHandler {
         // prepare the request for execution; has the side effect of touching the request parameters
         try (var action = prepareRequest(request, client)) {
 
+            assert assertConsumesSupportedParams(supported, request);
+
             // validate unconsumed params, but we must exclude params used to format the response
             // use a sorted set so the unconsumed parameters appear in a reliable sorted order
             final SortedSet<String> unconsumedParams = request.unconsumedParams()
@@ -148,6 +151,20 @@ public abstract class BaseRestHandler implements RestHandler {
         }
     }
 
+    private boolean assertConsumesSupportedParams(@Nullable Set<String> supported, RestRequest request) {
+        if (supported != null) {
+            final var supportedAndCommon = new TreeSet<>(supported);
+            supportedAndCommon.add("error_trace");
+            supportedAndCommon.addAll(ALWAYS_SUPPORTED);
+            supportedAndCommon.removeAll(RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS);
+            final var consumed = new TreeSet<>(request.consumedParams());
+            consumed.removeAll(RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS);
+            assert supportedAndCommon.equals(consumed)
+                : getName() + ": consumed params " + consumed + " while supporting " + supportedAndCommon;
+        }
+        return true;
+    }
+
     protected static String unrecognized(RestRequest request, Set<String> invalids, Set<String> candidates, String detail) {
         StringBuilder message = new StringBuilder().append("request [")
             .append(request.path())