From 5c4c1c5e157cabace5124c327427e2cf644db9d6 Mon Sep 17 00:00:00 2001 From: olcbean Date: Tue, 8 Aug 2017 10:45:44 +0200 Subject: [PATCH] Verify that _bulk and _msearch requests are terminated by a newline (#25740) --- .../action/bulk/BulkRequest.java | 3 +++ .../action/search/RestMultiSearchAction.java | 3 +++ .../action/bulk/BulkRequestTests.java | 12 ++++++++++ .../action/bulk/BulkWithUpdatesIT.java | 24 ++++++++----------- .../search/MultiSearchRequestTests.java | 16 +++++++++++++ .../action/bulk/simple-bulk11.json | 5 ++++ .../action/search/simple-msearch5.json | 6 +++++ .../join/query/LegacyChildQuerySearchIT.java | 22 ++++++++--------- 8 files changed, 66 insertions(+), 25 deletions(-) create mode 100644 core/src/test/resources/org/elasticsearch/action/bulk/simple-bulk11.json create mode 100644 core/src/test/resources/org/elasticsearch/action/search/simple-msearch5.json diff --git a/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index 5836da3b8c4..1b0d89222dd 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -518,6 +518,9 @@ public class BulkRequest extends ActionRequest implements CompositeIndicesReques return i; } } + if (from != length) { + throw new IllegalArgumentException("The bulk request must be terminated by a newline [\n]"); + } return -1; } diff --git a/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java index c8550d40875..7c2cc4b33d4 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java @@ -213,6 +213,9 @@ public class RestMultiSearchAction extends BaseRestHandler { return i; } } + if (from != length) { + throw new IllegalArgumentException("The msearch request must be terminated by a newline [\n]"); + } return -1; } diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java index 170b0f143a3..d52087db0e7 100644 --- a/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java @@ -316,4 +316,16 @@ public class BulkRequestTests extends ESTestCase { "can't provide version in upsert request")); } + public void testBulkTerminatedByNewline() throws Exception { + String bulkAction = copyToStringFromClasspath("/org/elasticsearch/action/bulk/simple-bulk11.json"); + IllegalArgumentException expectThrows = expectThrows(IllegalArgumentException.class, () -> new BulkRequest() + .add(bulkAction.getBytes(StandardCharsets.UTF_8), 0, bulkAction.length(), null, null, XContentType.JSON)); + assertEquals("The bulk request must be terminated by a newline [\n]", expectThrows.getMessage()); + + String bulkActionWithNewLine = bulkAction + "\n"; + BulkRequest bulkRequestWithNewLine = new BulkRequest(); + bulkRequestWithNewLine.add(bulkActionWithNewLine.getBytes(StandardCharsets.UTF_8), 0, bulkActionWithNewLine.length(), null, null, + XContentType.JSON); + assertEquals(3, bulkRequestWithNewLine.numberOfActions()); + } } diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java index 615ed7db5db..2dfc0e5a031 100644 --- a/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.action.bulk; import org.elasticsearch.Version; +import org.elasticsearch.action.DocWriteRequest.OpType; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.delete.DeleteRequest; @@ -31,17 +32,18 @@ import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.action.update.UpdateRequestBuilder; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.Requests; -import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; -import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptException; +import org.elasticsearch.script.ScriptType; import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.InternalSettingsPlugin; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -51,16 +53,10 @@ import java.util.Map; import java.util.concurrent.CyclicBarrier; import java.util.function.Function; -import static org.elasticsearch.action.DocWriteRequest.OpType; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; - -import org.elasticsearch.script.ScriptType; -import org.elasticsearch.test.InternalSettingsPlugin; - import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -468,7 +464,7 @@ public class BulkWithUpdatesIT extends ESIntegTestCase { BulkRequestBuilder builder = client().prepareBulk(); - byte[] addParent = new BytesArray( + byte[] addParent = ( "{" + " \"index\" : {" + " \"_index\" : \"test\"," + @@ -480,9 +476,9 @@ public class BulkWithUpdatesIT extends ESIntegTestCase { "{" + " \"field1\" : \"value1\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); - byte[] addChildOK = new BytesArray( + byte[] addChildOK = ( "{" + " \"index\" : {" + " \"_index\" : \"test\"," + @@ -495,9 +491,9 @@ public class BulkWithUpdatesIT extends ESIntegTestCase { "{" + " \"field1\" : \"value1\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); - byte[] addChildMissingRouting = new BytesArray( + byte[] addChildMissingRouting = ( "{" + " \"index\" : {" + " \"_index\" : \"test\"," + @@ -509,7 +505,7 @@ public class BulkWithUpdatesIT extends ESIntegTestCase { "{" + " \"field1\" : \"value1\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); builder.add(addParent, 0, addParent.length, XContentType.JSON); builder.add(addChildOK, 0, addChildOK.length, XContentType.JSON); diff --git a/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index b2903e24de7..3a162f302bc 100644 --- a/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.test.StreamsUtils; import org.elasticsearch.test.rest.FakeRestRequest; import java.io.IOException; +import java.nio.charset.StandardCharsets; import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.equalTo; @@ -171,6 +172,21 @@ public class MultiSearchRequestTests extends ESTestCase { request.maxConcurrentSearchRequests(randomIntBetween(Integer.MIN_VALUE, 0))); } + public void testMsearchTerminatedByNewline() throws Exception { + String mserchAction = StreamsUtils.copyToStringFromClasspath("/org/elasticsearch/action/search/simple-msearch5.json"); + RestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withContent(new BytesArray(mserchAction.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build(); + IllegalArgumentException expectThrows = expectThrows(IllegalArgumentException.class, + () -> RestMultiSearchAction.parseRequest(restRequest, true)); + assertEquals("The msearch request must be terminated by a newline [\n]", expectThrows.getMessage()); + + String mserchActionWithNewLine = mserchAction + "\n"; + RestRequest restRequestWithNewLine = new FakeRestRequest.Builder(xContentRegistry()) + .withContent(new BytesArray(mserchActionWithNewLine.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build(); + MultiSearchRequest msearchRequest = RestMultiSearchAction.parseRequest(restRequestWithNewLine, true); + assertEquals(3, msearchRequest.requests().size()); + } + private MultiSearchRequest parseMultiSearchRequest(String sample) throws IOException { byte[] data = StreamsUtils.copyToBytesFromClasspath(sample); RestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) diff --git a/core/src/test/resources/org/elasticsearch/action/bulk/simple-bulk11.json b/core/src/test/resources/org/elasticsearch/action/bulk/simple-bulk11.json new file mode 100644 index 00000000000..9be3c130612 --- /dev/null +++ b/core/src/test/resources/org/elasticsearch/action/bulk/simple-bulk11.json @@ -0,0 +1,5 @@ +{ "index":{"_index":"test","_type":"type1","_id":"1"} } +{ "field1" : "value1" } +{ "delete" : { "_index" : "test", "_type" : "type1", "_id" : "2" } } +{ "create" : { "_index" : "test", "_type" : "type1", "_id" : "3" } } +{ "field1" : "value3" } \ No newline at end of file diff --git a/core/src/test/resources/org/elasticsearch/action/search/simple-msearch5.json b/core/src/test/resources/org/elasticsearch/action/search/simple-msearch5.json new file mode 100644 index 00000000000..f2cc4e0e965 --- /dev/null +++ b/core/src/test/resources/org/elasticsearch/action/search/simple-msearch5.json @@ -0,0 +1,6 @@ +{"index":["test0", "test1"], "request_cache": true} +{"query" : {"match_all" : {}}} +{"index" : "test2,test3", "type" : "type1", "preference": "_local"} +{"query" : {"match_all" : {}}} +{"index" : ["test4", "test1"], "type" : [ "type2", "type1" ], "routing": "123"} +{"query" : {"match_all" : {}}} \ No newline at end of file diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/LegacyChildQuerySearchIT.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/LegacyChildQuerySearchIT.java index a227d9e44be..c13a1d76732 100644 --- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/LegacyChildQuerySearchIT.java +++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/LegacyChildQuerySearchIT.java @@ -25,12 +25,12 @@ import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -169,7 +169,7 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { BulkRequestBuilder builder = client().prepareBulk(); // It's important to use JSON parsing here and request objects: issue 3444 is related to incomplete option parsing - byte[] addParent = new BytesArray( + byte[] addParent = ( "{" + " \"index\" : {" + " \"_index\" : \"test\"," + @@ -181,9 +181,9 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { "{" + " \"field1\" : \"value1\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); - byte[] addChild = new BytesArray( + byte[] addChild = ( "{" + " \"update\" : {" + " \"_index\" : \"test\"," + @@ -199,7 +199,7 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { " }," + " \"doc_as_upsert\" : \"true\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); builder.add(addParent, 0, addParent.length, XContentType.JSON); builder.add(addChild, 0, addChild.length, XContentType.JSON); @@ -231,7 +231,7 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { BulkRequestBuilder builder = client().prepareBulk(); - byte[] addParent = new BytesArray( + byte[] addParent = ( "{" + " \"index\" : {" + " \"_index\" : \"test\"," + @@ -243,9 +243,9 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { "{" + " \"field1\" : \"value1\"" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); - byte[] addChild1 = new BytesArray( + byte[] addChild1 = ( "{" + " \"update\" : {" + " \"_index\" : \"test\"," + @@ -264,9 +264,9 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { " \"field1\" : \"value1'\"" + " }" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); - byte[] addChild2 = new BytesArray( + byte[] addChild2 = ( "{" + " \"update\" : {" + " \"_index\" : \"test\"," + @@ -282,7 +282,7 @@ public class LegacyChildQuerySearchIT extends ChildQuerySearchIT { " \"field1\" : \"value1'\"" + " }" + "}" + - "\n").array(); + "\n").getBytes(StandardCharsets.UTF_8); builder.add(addParent, 0, addParent.length, XContentType.JSON); builder.add(addChild1, 0, addChild1.length, XContentType.JSON);