From fc29b99c82747412950e46a132182b62b0023882 Mon Sep 17 00:00:00 2001 From: olcbean Date: Mon, 20 Nov 2017 10:13:56 +0100 Subject: [PATCH 1/3] verify `op_type` for `_create` --- .../rest/action/document/RestIndexAction.java | 8 ++++ .../action/index/IndexRequestTests.java | 19 +++----- .../action/document/RestIndexActionTests.java | 43 +++++++++++++++++++ 3 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index 68b09a4d86716..ef6e3923bb20d 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -31,6 +31,7 @@ import org.elasticsearch.rest.action.RestStatusToXContentListener; import java.io.IOException; +import java.util.Locale; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -63,9 +64,16 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { + validateOpType(request.params().get("op_type")); request.params().put("op_type", "create"); return RestIndexAction.this.prepareRequest(request, client); } + + public void validateOpType(String opType) { + if (null != opType && !"create".equals(opType.toLowerCase(Locale.US))) { + throw new IllegalArgumentException("opType must be 'create', found: [" + opType + "]"); + } + } } @Override diff --git a/core/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java b/core/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java index 6191184ef3f7a..640d6a84e960d 100644 --- a/core/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java @@ -55,23 +55,18 @@ public void testIndexRequestOpTypeFromString() throws Exception { IndexRequest indexRequest = new IndexRequest(""); indexRequest.opType(create); - assertThat(indexRequest.opType() , equalTo(DocWriteRequest.OpType.CREATE)); + assertThat(indexRequest.opType(), equalTo(DocWriteRequest.OpType.CREATE)); indexRequest.opType(createUpper); - assertThat(indexRequest.opType() , equalTo(DocWriteRequest.OpType.CREATE)); + assertThat(indexRequest.opType(), equalTo(DocWriteRequest.OpType.CREATE)); indexRequest.opType(index); - assertThat(indexRequest.opType() , equalTo(DocWriteRequest.OpType.INDEX)); + assertThat(indexRequest.opType(), equalTo(DocWriteRequest.OpType.INDEX)); indexRequest.opType(indexUpper); - assertThat(indexRequest.opType() , equalTo(DocWriteRequest.OpType.INDEX)); + assertThat(indexRequest.opType(), equalTo(DocWriteRequest.OpType.INDEX)); } - public void testReadBogusString() { - try { - IndexRequest indexRequest = new IndexRequest(""); - indexRequest.opType("foobar"); - fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), equalTo("opType must be 'create' or 'index', found: [foobar]")); - } + public void testReadIncorrectOpType() { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IndexRequest("").opType("foobar")); + assertThat(e.getMessage(), equalTo("opType must be 'create' or 'index', found: [foobar]")); } public void testCreateOperationRejectsVersions() { diff --git a/core/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java b/core/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java new file mode 100644 index 0000000000000..d0c7210db2beb --- /dev/null +++ b/core/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java @@ -0,0 +1,43 @@ +/* + * 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.document; + +import org.elasticsearch.Version; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; + +public class RestIndexActionTests extends ESTestCase { + + public void testCreateOpTypeValidation() throws Exception { + Settings settings = settings(Version.CURRENT).build(); + RestIndexAction.CreateHandler create = new RestIndexAction(settings, mock(RestController.class)).new CreateHandler(settings); + + String opType = randomFrom("CREATE", null); + create.validateOpType(opType); + + String illegalOpType = randomFrom("index", "unknown", ""); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> create.validateOpType(illegalOpType)); + assertThat(e.getMessage(), equalTo("opType must be 'create', found: [" + illegalOpType + "]")); + } +} From dee361c9730b7676e12551128eda38effe30d7a1 Mon Sep 17 00:00:00 2001 From: olcbean Date: Tue, 21 Nov 2017 17:55:20 +0100 Subject: [PATCH 2/3] using root locale and false== instead of ![not] --- .../org/elasticsearch/rest/action/document/RestIndexAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index ef6e3923bb20d..da4aaaa44a6b1 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -70,7 +70,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient } public void validateOpType(String opType) { - if (null != opType && !"create".equals(opType.toLowerCase(Locale.US))) { + if (null != opType && false == "create".equals(opType.toLowerCase(Locale.ROOT))) { throw new IllegalArgumentException("opType must be 'create', found: [" + opType + "]"); } } From 7a2a35a0a6912725198abc8ad6bb2797566f0eca Mon Sep 17 00:00:00 2001 From: olcbean Date: Wed, 22 Nov 2017 18:41:10 +0100 Subject: [PATCH 3/3] changing method access modifier --- .../org/elasticsearch/rest/action/document/RestIndexAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index da4aaaa44a6b1..b29df7a281a86 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -69,7 +69,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient return RestIndexAction.this.prepareRequest(request, client); } - public void validateOpType(String opType) { + void validateOpType(String opType) { if (null != opType && false == "create".equals(opType.toLowerCase(Locale.ROOT))) { throw new IllegalArgumentException("opType must be 'create', found: [" + opType + "]"); }