From 7a55cca1224e53e587f46b25aa874fd0ed769d9a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 19 Oct 2016 22:01:17 -0400 Subject: [PATCH] Whitelist node stats indices level parameter When indices stats are requested via the node stats API, there is a level parameter to request stats at the index, node, or shards level. This parameter was not whitelisted when URL parsing was made strict. This commit whitelists this parameter. Additionally, there was some leniency in the parsing of this parameter that has been removed. Relates #21024 --- .../indices/NodeIndicesStats.java | 7 ++-- .../admin/cluster/RestNodesStatsAction.java | 9 ++++ .../indices/NodeIndicesStatsTests.java | 42 +++++++++++++++++++ .../rest-api-spec/api/nodes.stats.json | 4 +- .../test/nodes.stats/10_basic.yaml | 14 +++++++ 5 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/indices/NodeIndicesStatsTests.java diff --git a/core/src/main/java/org/elasticsearch/indices/NodeIndicesStats.java b/core/src/main/java/org/elasticsearch/indices/NodeIndicesStats.java index 6c251d3bf1c..9133ca81e28 100644 --- a/core/src/main/java/org/elasticsearch/indices/NodeIndicesStats.java +++ b/core/src/main/java/org/elasticsearch/indices/NodeIndicesStats.java @@ -188,10 +188,11 @@ public class NodeIndicesStats implements Streamable, ToXContent { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - String level = params.param("level", "node"); - boolean isLevelValid = "node".equalsIgnoreCase(level) || "indices".equalsIgnoreCase(level) || "shards".equalsIgnoreCase(level); + final String level = params.param("level", "node"); + final boolean isLevelValid = + "indices".equalsIgnoreCase(level) || "node".equalsIgnoreCase(level) || "shards".equalsIgnoreCase(level); if (!isLevelValid) { - return builder; + throw new IllegalArgumentException("level parameter must be one of [indices] or [node] or [shards] but was [" + level + "]"); } // "node" level diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesStatsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesStatsAction.java index 6b353fc6e44..917f5b2c5b1 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesStatsAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesStatsAction.java @@ -32,6 +32,7 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestActions.NodesResponseRestListener; import java.io.IOException; +import java.util.Collections; import java.util.Set; import static org.elasticsearch.rest.RestRequest.Method.GET; @@ -114,8 +115,16 @@ public class RestNodesStatsAction extends BaseRestHandler { return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); } + private final Set RESPONSE_PARAMS = Collections.singleton("level"); + + @Override + protected Set responseParams() { + return RESPONSE_PARAMS; + } + @Override public boolean canTripCircuitBreaker() { return false; } + } diff --git a/core/src/test/java/org/elasticsearch/indices/NodeIndicesStatsTests.java b/core/src/test/java/org/elasticsearch/indices/NodeIndicesStatsTests.java new file mode 100644 index 00000000000..f712a5ba843 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/indices/NodeIndicesStatsTests.java @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.indices; + +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.test.ESTestCase; + +import java.util.Collections; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.object.HasToString.hasToString; + +public class NodeIndicesStatsTests extends ESTestCase { + + public void testInvalidLevel() { + final NodeIndicesStats stats = new NodeIndicesStats(); + final String level = randomAsciiOfLength(16); + final ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap("level", level)); + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> stats.toXContent(null, params)); + assertThat( + e, + hasToString(containsString("level parameter must be one of [indices] or [node] or [shards] but was [" + level + "]"))); + } + +} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.stats.json b/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.stats.json index fb9ef094f0b..665d6bd7a2c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.stats.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/nodes.stats.json @@ -52,8 +52,8 @@ }, "level": { "type" : "enum", - "description": "Return indices stats aggregated at node, index or shard level", - "options" : ["node", "indices", "shards"], + "description": "Return indices stats aggregated at index, node or shard level", + "options" : ["indices", "node", "shards"], "default" : "node" }, "types" : { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/10_basic.yaml index 977d002748a..1cd9fef0258 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/10_basic.yaml @@ -6,3 +6,17 @@ - is_true: cluster_name - is_true: nodes + +--- +"Nodes stats level": + - do: + cluster.state: {} + + - set: { master_node: master } + + - do: + nodes.stats: + metric: [ indices ] + level: "indices" + + - is_true: nodes.$master.indices.indices