From 1c0f0dbe6bb4419a6f9ccb8c4562108e7a6291e4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Jan 2019 15:03:00 -0500 Subject: [PATCH 1/5] Reject delete index requests with a body Delete index requests do not take a body, and it is misleading and arguably a bug that they currently do. We abhor this sort of leniency in Elasticsearch, so this commit removes the possibility for a delete index request to have a body. --- .../admin/indices/RestDeleteIndexAction.java | 3 + .../indices/RestDeleteIndexActionTests.java | 66 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java index f6c4c17857287..1193c497618a6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java @@ -45,6 +45,9 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + if (request.hasContent()) { + throw new IllegalArgumentException("delete requests can not have a request body"); + } DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(Strings.splitStringByCommaToArray(request.param("index"))); deleteIndexRequest.timeout(request.paramAsTime("timeout", deleteIndexRequest.timeout())); deleteIndexRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteIndexRequest.masterNodeTimeout())); diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java new file mode 100644 index 0000000000000..d39c1bb20e522 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java @@ -0,0 +1,66 @@ +/* + * 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.rest.action.admin.indices; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.action.document.RestDeleteAction; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; + +public class RestDeleteIndexActionTests extends ESTestCase { + + public void testBodyRejection() throws Exception { + final RestDeleteIndexAction handler = new RestDeleteIndexAction(Settings.EMPTY, mock(RestController.class)); + try (XContentBuilder builder = JsonXContent.contentBuilder()) { + builder.startObject(); + { + builder.startObject("query"); + { + builder.startObject("term"); + { + builder.field("user", ""); + } + builder.endObject(); + } + builder.endObject(); + } + builder.endObject(); + final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withContent(new BytesArray(builder.toString()), XContentType.JSON) + .build(); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> handler.prepareRequest(request, mock(NodeClient.class))); + assertThat(e.getMessage(), equalTo("delete requests can not have a request body")); + } + } + +} \ No newline at end of file From 918c8d3f37d0c79aeb0185a0c4eae215ce43d3e9 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Jan 2019 15:06:12 -0500 Subject: [PATCH 2/5] Add missing newline --- .../rest/action/admin/indices/RestDeleteIndexActionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java index d39c1bb20e522..6fcd3fe800d98 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java @@ -63,4 +63,4 @@ public void testBodyRejection() throws Exception { } } -} \ No newline at end of file +} From 457ed18d2acf6764639a97647f6693f29171858e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Jan 2019 15:06:44 -0500 Subject: [PATCH 3/5] Fix imports --- .../rest/action/admin/indices/RestDeleteIndexActionTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java index 6fcd3fe800d98..8ae2ca07b5882 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java @@ -27,12 +27,10 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.action.document.RestDeleteAction; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.*; import static org.mockito.Mockito.mock; public class RestDeleteIndexActionTests extends ESTestCase { From 834daf61dc80cfb8c4904a1e9a6f7baeac6842ab Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Jan 2019 15:07:18 -0500 Subject: [PATCH 4/5] Better wording --- .../rest/action/admin/indices/RestDeleteIndexAction.java | 2 +- .../rest/action/admin/indices/RestDeleteIndexActionTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java index 1193c497618a6..fe796bc9050cf 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java @@ -46,7 +46,7 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { if (request.hasContent()) { - throw new IllegalArgumentException("delete requests can not have a request body"); + throw new IllegalArgumentException("delete index requests can not have a request body"); } DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(Strings.splitStringByCommaToArray(request.param("index"))); deleteIndexRequest.timeout(request.paramAsTime("timeout", deleteIndexRequest.timeout())); diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java index 8ae2ca07b5882..625433a47609c 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexActionTests.java @@ -57,7 +57,7 @@ public void testBodyRejection() throws Exception { IllegalArgumentException e = expectThrows( IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class))); - assertThat(e.getMessage(), equalTo("delete requests can not have a request body")); + assertThat(e.getMessage(), equalTo("delete index requests can not have a request body")); } } From 3a8549a7b7e0aa61101efb753d19d779e87bf7cf Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Jan 2019 15:11:01 -0500 Subject: [PATCH 5/5] Formatting while we are here --- .../rest/action/admin/indices/RestDeleteIndexAction.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java index fe796bc9050cf..0232f8e7062bd 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestDeleteIndexAction.java @@ -32,6 +32,7 @@ import java.io.IOException; public class RestDeleteIndexAction extends BaseRestHandler { + public RestDeleteIndexAction(Settings settings, RestController controller) { super(settings); controller.registerHandler(RestRequest.Method.DELETE, "/", this); @@ -48,10 +49,11 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC if (request.hasContent()) { throw new IllegalArgumentException("delete index requests can not have a request body"); } - DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(Strings.splitStringByCommaToArray(request.param("index"))); + final DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(Strings.splitStringByCommaToArray(request.param("index"))); deleteIndexRequest.timeout(request.paramAsTime("timeout", deleteIndexRequest.timeout())); deleteIndexRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteIndexRequest.masterNodeTimeout())); deleteIndexRequest.indicesOptions(IndicesOptions.fromRequest(request, deleteIndexRequest.indicesOptions())); return channel -> client.admin().indices().delete(deleteIndexRequest, new RestToXContentListener<>(channel)); } + }