diff --git a/client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java b/client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java index d8a5e4fa0ea47..df712f537e43e 100644 --- a/client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java +++ b/client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java @@ -73,8 +73,8 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC } bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT)); bulkRequest.setRefreshPolicy(request.param("refresh")); - bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, null, defaultPipeline, null, true, - request.getXContentType()); + bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting, defaultFields, + null, defaultPipeline, null, true, request.getXContentType()); // short circuit the call to the transport layer return channel -> { 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 b60728b9d4587..5836da3b8c49a 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -41,7 +41,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; @@ -300,10 +299,16 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null if (token == null) { continue; } - assert token == XContentParser.Token.START_OBJECT; + if (token != XContentParser.Token.START_OBJECT) { + throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected " + + XContentParser.Token.START_OBJECT + " but found [" + token + "]"); + } // Move to FIELD_NAME, that's the action token = parser.nextToken(); - assert token == XContentParser.Token.FIELD_NAME; + if (token != XContentParser.Token.FIELD_NAME) { + throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected " + + XContentParser.Token.FIELD_NAME + " but found [" + token + "]"); + } String action = parser.currentName(); String index = defaultIndex; diff --git a/core/src/main/java/org/elasticsearch/rest/RestRequest.java b/core/src/main/java/org/elasticsearch/rest/RestRequest.java index 509bfa7a3c0eb..1a74a6c68111b 100644 --- a/core/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/core/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -136,6 +136,18 @@ public final String path() { public abstract BytesReference content(); + /** + * @return content of the request body or throw an exception if the body or content type is missing + */ + public final BytesReference requiredContent() { + if (hasContent() == false) { + throw new ElasticsearchParseException("request body is required"); + } else if (xContentType.get() == null) { + throw new IllegalStateException("unknown content type"); + } + return content(); + } + /** * Get the value of the header or {@code null} if not found. This method only retrieves the first header value if multiple values are * sent. Use of {@link #getAllHeaderValues(String)} should be preferred @@ -329,12 +341,7 @@ public NamedXContentRegistry getXContentRegistry() { * {@link #contentOrSourceParamParser()} for requests that support specifying the request body in the {@code source} param. */ public final XContentParser contentParser() throws IOException { - BytesReference content = content(); - if (content.length() == 0) { - throw new ElasticsearchParseException("Body required"); - } else if (xContentType.get() == null) { - throw new IllegalStateException("unknown content type"); - } + BytesReference content = requiredContent(); // will throw exception if body or content type missing return xContentType.get().xContent().createParser(xContentRegistry, content); } @@ -364,11 +371,7 @@ public final boolean hasContentOrSourceParam() { */ public final XContentParser contentOrSourceParamParser() throws IOException { Tuple tuple = contentOrSourceParam(); - BytesReference content = tuple.v2(); - if (content.length() == 0) { - throw new ElasticsearchParseException("Body required"); - } - return tuple.v1().xContent().createParser(xContentRegistry, content); + return tuple.v1().xContent().createParser(xContentRegistry, tuple.v2()); } /** @@ -377,10 +380,10 @@ public final XContentParser contentOrSourceParamParser() throws IOException { * back to the user when there isn't request content. */ public final void withContentOrSourceParamParserOrNull(CheckedConsumer withParser) throws IOException { - Tuple tuple = contentOrSourceParam(); - BytesReference content = tuple.v2(); - XContentType xContentType = tuple.v1(); - if (content.length() > 0) { + if (hasContentOrSourceParam()) { + Tuple tuple = contentOrSourceParam(); + BytesReference content = tuple.v2(); + XContentType xContentType = tuple.v1(); try (XContentParser parser = xContentType.xContent().createParser(xContentRegistry, content)) { withParser.accept(parser); } @@ -390,28 +393,26 @@ public final void withContentOrSourceParamParserOrNull(CheckedConsumer contentOrSourceParam() { - if (hasContent()) { - if (xContentType.get() == null) { - throw new IllegalStateException("unknown content type"); - } - return new Tuple<>(xContentType.get(), content()); + if (hasContentOrSourceParam() == false) { + throw new ElasticsearchParseException("request body or source parameter is required"); + } else if (hasContent()) { + return new Tuple<>(xContentType.get(), requiredContent()); } - String source = param("source"); String typeParam = param("source_content_type"); - if (source != null && typeParam != null) { - BytesArray bytes = new BytesArray(source); - final XContentType xContentType = parseContentType(Collections.singletonList(typeParam)); - if (xContentType == null) { - throw new IllegalStateException("Unknown value for source_content_type [" + typeParam + "]"); - } - return new Tuple<>(xContentType, bytes); + if (source == null || typeParam == null) { + throw new IllegalStateException("source and source_content_type parameters are required"); + } + BytesArray bytes = new BytesArray(source); + final XContentType xContentType = parseContentType(Collections.singletonList(typeParam)); + if (xContentType == null) { + throw new IllegalStateException("Unknown value for source_content_type [" + typeParam + "]"); } - return new Tuple<>(XContentType.JSON, BytesArray.EMPTY); + return new Tuple<>(xContentType, bytes); } /** diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java index 358c3656ceddf..8f3077d047596 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java @@ -58,7 +58,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client lang = null; } - BytesReference content = request.content(); + BytesReference content = request.requiredContent(); if (lang != null) { deprecationLogger.deprecated( diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java index b376f3dab30a2..c91c9fc3986fc 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java @@ -57,7 +57,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC putRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putRequest.masterNodeTimeout())); putRequest.create(request.paramAsBoolean("create", false)); putRequest.cause(request.param("cause", "")); - putRequest.source(request.content(), request.getXContentType()); + putRequest.source(request.requiredContent(), request.getXContentType()); return channel -> client.admin().indices().putTemplate(putRequest, new AcknowledgedRestListener<>(channel)); } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java index 06fe67926455f..023f83d43e367 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java @@ -64,7 +64,7 @@ public RestPutMappingAction(Settings settings, RestController controller) { public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index"))); putMappingRequest.type(request.param("type")); - putMappingRequest.source(request.content(), request.getXContentType()); + putMappingRequest.source(request.requiredContent(), request.getXContentType()); putMappingRequest.updateAllTypes(request.paramAsBoolean("update_all_types", false)); putMappingRequest.timeout(request.paramAsTime("timeout", putMappingRequest.timeout())); putMappingRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putMappingRequest.masterNodeTimeout())); diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestUpdateSettingsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestUpdateSettingsAction.java index 9a168e84dd683..d4c7619db6baa 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestUpdateSettingsAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestUpdateSettingsAction.java @@ -54,18 +54,16 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC updateSettingsRequest.indicesOptions(IndicesOptions.fromRequest(request, updateSettingsRequest.indicesOptions())); Map settings = new HashMap<>(); - if (request.hasContent()) { - try (XContentParser parser = request.contentParser()) { - Map bodySettings = parser.map(); - Object innerBodySettings = bodySettings.get("settings"); - // clean up in case the body is wrapped with "settings" : { ... } - if (innerBodySettings instanceof Map) { - @SuppressWarnings("unchecked") - Map innerBodySettingsMap = (Map) innerBodySettings; - settings.putAll(innerBodySettingsMap); - } else { - settings.putAll(bodySettings); - } + try (XContentParser parser = request.contentParser()) { + Map bodySettings = parser.map(); + Object innerBodySettings = bodySettings.get("settings"); + // clean up in case the body is wrapped with "settings" : { ... } + if (innerBodySettings instanceof Map) { + @SuppressWarnings("unchecked") + Map innerBodySettingsMap = (Map) innerBodySettings; + settings.putAll(innerBodySettingsMap); + } else { + settings.putAll(bodySettings); } } updateSettingsRequest.settings(settings); diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java index d6af84d947299..8745c4bfe2d98 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java @@ -86,7 +86,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC } bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT)); bulkRequest.setRefreshPolicy(request.param("refresh")); - bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, + bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting, defaultFields, defaultFetchSourceContext, defaultPipeline, null, allowExplicitIndex, request.getXContentType()); return channel -> client.bulk(bulkRequest, new RestStatusToXContentListener<>(channel)); 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 a32848841a462..cc82fd681e6a1 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 @@ -64,7 +64,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC indexRequest.routing(request.param("routing")); indexRequest.parent(request.param("parent")); indexRequest.setPipeline(request.param("pipeline")); - indexRequest.source(request.content(), request.getXContentType()); + indexRequest.source(request.requiredContent(), request.getXContentType()); indexRequest.timeout(request.paramAsTime("timeout", IndexRequest.DEFAULT_TIMEOUT)); indexRequest.setRefreshPolicy(request.param("refresh")); indexRequest.version(RestActions.parseVersion(request)); 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 76810056485ac..170b0f143a31f 100644 --- a/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java @@ -177,6 +177,31 @@ public void testSimpleBulk10() throws Exception { assertThat(bulkRequest.numberOfActions(), equalTo(9)); } + public void testBulkEmptyObject() throws Exception { + String bulkIndexAction = "{ \"index\":{\"_index\":\"test\",\"_type\":\"type1\",\"_id\":\"1\"} }\r\n"; + String bulkIndexSource = "{ \"field1\" : \"value1\" }\r\n"; + String emptyObject = "{}\r\n"; + StringBuilder bulk = new StringBuilder(); + int emptyLine; + if (randomBoolean()) { + bulk.append(emptyObject); + emptyLine = 1; + } else { + int actions = randomIntBetween(1, 10); + int emptyAction = randomIntBetween(1, actions); + emptyLine = emptyAction * 2 - 1; + for (int i = 1; i <= actions; i++) { + bulk.append(i == emptyAction ? emptyObject : bulkIndexAction); + bulk.append(randomBoolean() ? emptyObject : bulkIndexSource); + } + } + String bulkAction = bulk.toString(); + BulkRequest bulkRequest = new BulkRequest(); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, + () -> bulkRequest.add(bulkAction.getBytes(StandardCharsets.UTF_8), 0, bulkAction.length(), null, null, XContentType.JSON)); + assertThat(exc.getMessage(), containsString("Malformed action/metadata line [" + emptyLine + "], expected FIELD_NAME but found [END_OBJECT]")); + } + // issue 7361 public void testBulkRequestWithRefresh() throws Exception { BulkRequest bulkRequest = new BulkRequest(); diff --git a/core/src/test/java/org/elasticsearch/rest/RestRequestTests.java b/core/src/test/java/org/elasticsearch/rest/RestRequestTests.java index 14eb413de083b..d1c7d03e1b174 100644 --- a/core/src/test/java/org/elasticsearch/rest/RestRequestTests.java +++ b/core/src/test/java/org/elasticsearch/rest/RestRequestTests.java @@ -43,11 +43,14 @@ public class RestRequestTests extends ESTestCase { public void testContentParser() throws IOException { Exception e = expectThrows(ElasticsearchParseException.class, () -> new ContentRestRequest("", emptyMap()).contentParser()); - assertEquals("Body required", e.getMessage()); + assertEquals("request body is required", e.getMessage()); e = expectThrows(ElasticsearchParseException.class, () -> new ContentRestRequest("", singletonMap("source", "{}")).contentParser()); - assertEquals("Body required", e.getMessage()); + assertEquals("request body is required", e.getMessage()); assertEquals(emptyMap(), new ContentRestRequest("{}", emptyMap()).contentParser().map()); + e = expectThrows(ElasticsearchParseException.class, () -> + new ContentRestRequest("", emptyMap(), emptyMap()).contentParser()); + assertEquals("request body is required", e.getMessage()); } public void testApplyContentParser() throws IOException { @@ -59,7 +62,9 @@ public void testApplyContentParser() throws IOException { } public void testContentOrSourceParam() throws IOException { - assertEquals(BytesArray.EMPTY, new ContentRestRequest("", emptyMap()).contentOrSourceParam().v2()); + Exception e = expectThrows(ElasticsearchParseException.class, () -> + new ContentRestRequest("", emptyMap()).contentOrSourceParam()); + assertEquals("request body or source parameter is required", e.getMessage()); assertEquals(new BytesArray("stuff"), new ContentRestRequest("stuff", emptyMap()).contentOrSourceParam().v2()); assertEquals(new BytesArray("stuff"), new ContentRestRequest("stuff", MapBuilder.newMapBuilder() @@ -68,6 +73,10 @@ public void testContentOrSourceParam() throws IOException { new ContentRestRequest("", MapBuilder.newMapBuilder() .put("source", "{\"foo\": \"stuff\"}").put("source_content_type", "application/json").immutableMap()) .contentOrSourceParam().v2()); + e = expectThrows(IllegalStateException.class, () -> + new ContentRestRequest("", MapBuilder.newMapBuilder() + .put("source", "stuff2").immutableMap()).contentOrSourceParam()); + assertEquals("source and source_content_type parameters are required", e.getMessage()); } public void testHasContentOrSourceParam() throws IOException { @@ -80,7 +89,7 @@ public void testHasContentOrSourceParam() throws IOException { public void testContentOrSourceParamParser() throws IOException { Exception e = expectThrows(ElasticsearchParseException.class, () -> new ContentRestRequest("", emptyMap()).contentOrSourceParamParser()); - assertEquals("Body required", e.getMessage()); + assertEquals("request body or source parameter is required", e.getMessage()); assertEquals(emptyMap(), new ContentRestRequest("{}", emptyMap()).contentOrSourceParamParser().map()); assertEquals(emptyMap(), new ContentRestRequest("{}", singletonMap("source", "stuff2")).contentOrSourceParamParser().map()); assertEquals(emptyMap(), new ContentRestRequest("", MapBuilder.newMapBuilder() @@ -138,6 +147,24 @@ public void testMultipleContentTypeHeaders() { assertEquals("only one Content-Type header should be provided", e.getMessage()); } + public void testRequiredContent() { + Exception e = expectThrows(ElasticsearchParseException.class, () -> + new ContentRestRequest("", emptyMap()).requiredContent()); + assertEquals("request body is required", e.getMessage()); + assertEquals(new BytesArray("stuff"), new ContentRestRequest("stuff", emptyMap()).requiredContent()); + assertEquals(new BytesArray("stuff"), + new ContentRestRequest("stuff", MapBuilder.newMapBuilder() + .put("source", "stuff2").put("source_content_type", "application/json").immutableMap()).requiredContent()); + e = expectThrows(ElasticsearchParseException.class, () -> + new ContentRestRequest("", MapBuilder.newMapBuilder() + .put("source", "{\"foo\": \"stuff\"}").put("source_content_type", "application/json").immutableMap()) + .requiredContent()); + assertEquals("request body is required", e.getMessage()); + e = expectThrows(IllegalStateException.class, () -> + new ContentRestRequest("test", null, Collections.emptyMap()).requiredContent()); + assertEquals("unknown content type", e.getMessage()); + } + private static final class ContentRestRequest extends RestRequest { private final BytesArray content; diff --git a/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/20_crud.yml b/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/20_crud.yml index b041e0664bb6c..10a47bb2b811c 100644 --- a/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/20_crud.yml +++ b/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/20_crud.yml @@ -203,3 +203,16 @@ teardown: catch: missing ingest.get_pipeline: id: "my_pipeline" + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body or source parameter is required/ + raw: + method: PUT + path: _ingest/pipeline/my_pipeline diff --git a/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/90_simulate.yml b/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/90_simulate.yml index 8b08535c12494..747f132f9031f 100644 --- a/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/90_simulate.yml +++ b/modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/90_simulate.yml @@ -605,3 +605,16 @@ teardown: - length: { docs.0.processor_results.1: 2 } - match: { docs.0.processor_results.1.tag: "rename-1" } - match: { docs.0.processor_results.1.doc._source.new_status: 200 } + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body or source parameter is required/ + raw: + method: POST + path: _ingest/pipeline/my_pipeline/_simulate diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java index f8bac3236d40b..f995d0c06f7de 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java @@ -57,10 +57,6 @@ public RestMultiSearchTemplateAction(Settings settings, RestController controlle @Override public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - if (request.hasContentOrSourceParam() == false) { - throw new ElasticsearchException("request body is required"); - } - MultiSearchTemplateRequest multiRequest = parseRequest(request, allowExplicitIndex); return channel -> client.execute(MultiSearchTemplateAction.INSTANCE, multiRequest, new RestToXContentListener<>(channel)); } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java index 83925f0ec03b6..37e6c516252d9 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java @@ -45,7 +45,7 @@ public RestPutSearchTemplateAction(Settings settings, RestController controller) @Override public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { String id = request.param("id"); - BytesReference content = request.content(); + BytesReference content = request.requiredContent(); PutStoredScriptRequest put = new PutStoredScriptRequest(id, Script.DEFAULT_TEMPLATE_LANG, content, request.getXContentType()); return channel -> client.admin().cluster().putStoredScript(put, new AcknowledgedRestListener<>(channel)); diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java index 3e967fa4286c1..f6d5f3fe97b51 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java @@ -87,10 +87,6 @@ public RestSearchTemplateAction(Settings settings, RestController controller) { @Override public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - if (request.hasContentOrSourceParam() == false) { - throw new ElasticsearchException("request body is required"); - } - // Creates the search request with all required params SearchRequest searchRequest = new SearchRequest(); RestSearchAction.parseSearchRequest(searchRequest, request, null); diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yml index 644319d50ec3c..f0c4e2ca2710b 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yml @@ -58,3 +58,16 @@ put_template: id: "1" body: { "template": { "query": { "match{{}}_all": {}}, "size": "{{my_size}}" } } + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: _search/template/1 diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml index d796731bdb4b2..40a16dc4f4a78 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml @@ -122,3 +122,16 @@ - match: { hits.total: 1 } - length: { hits.hits: 1 } - length: { profile: 1 } + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body or source parameter is required/ + raw: + method: POST + path: _search/template diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml index 658f2368d4e89..bc1b295c62c38 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml @@ -157,3 +157,15 @@ setup: - match: { responses.1.hits.total: 1 } - match: { responses.2.hits.total: 1 } +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body or source parameter is required/ + raw: + method: POST + path: _msearch/template diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java index 32a252ccc4b19..64209653c6f4f 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.reindex; -import org.apache.lucene.util.IOUtils; import org.elasticsearch.action.GenericAction; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.settings.Settings; @@ -55,18 +54,9 @@ protected void parseInternalRequest(Request internal, RestRequest restRequest, int scrollSize = searchRequest.source().size(); searchRequest.source().size(SIZE_ALL_MATCHES); - restRequest.withContentOrSourceParamParserOrNull(parser -> { - XContentParser searchRequestParser = extractRequestSpecificFieldsAndReturnSearchCompatibleParser(parser, bodyConsumers); - /* searchRequestParser might be parser or it might be a new parser built from parser's contents. If it is parser then - * withContentOrSourceParamParserOrNull will close it for us but if it isn't then we should close it. Technically close on - * the generated parser probably is a noop but we should do the accounting just in case. It doesn't hurt to close twice but it - * really hurts not to close if by some miracle we have to. */ - try { - RestSearchAction.parseSearchRequest(searchRequest, restRequest, searchRequestParser); - } finally { - IOUtils.close(searchRequestParser); - } - }); + try (XContentParser parser = extractRequestSpecificFields(restRequest, bodyConsumers)) { + RestSearchAction.parseSearchRequest(searchRequest, restRequest, parser); + } internal.setSize(searchRequest.source().size()); searchRequest.source().size(restRequest.paramAsInt("scroll_size", scrollSize)); @@ -89,12 +79,13 @@ protected void parseInternalRequest(Request internal, RestRequest restRequest, * should get better when SearchRequest has full ObjectParser support * then we can delegate and stuff. */ - private XContentParser extractRequestSpecificFieldsAndReturnSearchCompatibleParser(XContentParser parser, - Map> bodyConsumers) throws IOException { - if (parser == null) { - return parser; + private XContentParser extractRequestSpecificFields(RestRequest restRequest, + Map> bodyConsumers) throws IOException { + if (restRequest.hasContentOrSourceParam() == false) { + return null; // body is optional } - try { + try (XContentParser parser = restRequest.contentOrSourceParamParser(); + XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { Map body = parser.map(); for (Map.Entry> consumer : bodyConsumers.entrySet()) { @@ -103,12 +94,7 @@ private XContentParser extractRequestSpecificFieldsAndReturnSearchCompatiblePars consumer.getValue().accept(value); } } - - try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { - return parser.contentType().xContent().createParser(parser.getXContentRegistry(), builder.map(body).bytes()); - } - } finally { - parser.close(); + return parser.contentType().xContent().createParser(parser.getXContentRegistry(), builder.map(body).bytes()); } } } diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestDeleteByQueryAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestDeleteByQueryAction.java index f906ef7660da8..a32bea5ed3c7b 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestDeleteByQueryAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestDeleteByQueryAction.java @@ -47,9 +47,6 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client @Override protected DeleteByQueryRequest buildRequest(RestRequest request) throws IOException { - if (false == request.hasContent()) { - throw new ElasticsearchException("_delete_by_query requires a request body"); - } /* * Passing the search request through DeleteByQueryRequest first allows * it to set its own defaults which differ from SearchRequest's diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml index 3557cf9bad7f3..247da75e80678 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml @@ -299,3 +299,16 @@ index: source metric: search - match: {indices.source.total.search.open_contexts: 0} + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: _reindex diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/10_basic.yml index 3573f8ba75b93..29a565ba0f784 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/10_basic.yml @@ -58,3 +58,33 @@ index: test - match: { count: 2 } + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: _bulk + +--- +"empty action": + + - skip: + version: " - 5.99.99" + reason: confusing exception messaged caused by empty object fixed in 6.0.0 + + - do: + catch: /Malformed action\/metadata line \[3\], expected FIELD_NAME but found \[END_OBJECT\]/ + headers: + Content-Type: application/json + bulk: + body: | + {"index": {"_index": "test_index", "_type": "test_type", "_id": "test_id"}} + {"f1": "v1", "f2": 42} + {} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_script/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_script/10_basic.yml new file mode 100644 index 0000000000000..bfab22fd65880 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_script/10_basic.yml @@ -0,0 +1,12 @@ +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: _scripts/lang diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_settings/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_settings/10_basic.yml index 083466f94a55b..31f504a510cf9 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_settings/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_settings/10_basic.yml @@ -75,3 +75,15 @@ - match: {defaults.node.attr.testattr: "test"} +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: PUT + path: _settings diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/10_with_id.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/10_with_id.yml index 8ac55ec79f626..b15aad5aaa1a1 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/10_with_id.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/index/10_with_id.yml @@ -32,3 +32,16 @@ type: type id: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa body: { foo: bar } + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: idx/type/123 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml index 1d33f2d31bb15..10253fbcef7cf 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml @@ -67,3 +67,16 @@ properties: "": type: keyword + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: POST + path: test_index/test_type/_mapping diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml index 01bd7afc582b8..face649ea4e41 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml @@ -210,3 +210,16 @@ catch: missing indices.get_template: name: "my_template" + +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body is required/ + raw: + method: PUT + path: _template/my_template diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/msearch/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/msearch/10_basic.yml index 6e6b0cd3b4f81..5c78e6824ca1d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/msearch/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/msearch/10_basic.yml @@ -62,3 +62,15 @@ setup: - match: { responses.3.error.root_cause.0.index: index_3 } - match: { responses.4.hits.total: 4 } +--- +"missing body": + + - skip: + version: " - 5.99.99" + reason: NPE caused by missing body fixed in 6.0.0 + + - do: + catch: /request body or source parameter is required/ + raw: + method: POST + path: _msearch