From 08d1749de75ab6ede491a3f9bff859055ac9197d Mon Sep 17 00:00:00 2001 From: Alex Benusovich Date: Thu, 1 Jun 2017 11:30:04 -0700 Subject: [PATCH] Fixed NPEs caused by requests without content. REST handlers that require a body will throw an an ElasticsearchParseException "request body required". REST handlers that require a body OR source param will throw an ElasticsearchParseException "request body or source param required". Replaced asserts in BulkRequest parsing code with a more descriptive IllegalArgumentException if the line contains an empty object. Updated bulk REST test to verify an empty action line is rejected properly. Updated BulkRequestTests with randomized testing for an empty action line. Used try-with-resouces for XContentParser in AbstractBulkByQueryRestHandler. --- .../noop/action/bulk/RestNoopBulkAction.java | 4 +- .../action/bulk/BulkRequest.java | 11 +++- .../org/elasticsearch/rest/RestRequest.java | 63 ++++++++++--------- .../cluster/RestPutStoredScriptAction.java | 2 +- .../indices/RestPutIndexTemplateAction.java | 2 +- .../admin/indices/RestPutMappingAction.java | 2 +- .../indices/RestUpdateSettingsAction.java | 22 +++---- .../rest/action/document/RestBulkAction.java | 2 +- .../rest/action/document/RestIndexAction.java | 2 +- .../action/bulk/BulkRequestTests.java | 25 ++++++++ .../elasticsearch/rest/RestRequestTests.java | 35 +++++++++-- .../rest-api-spec/test/ingest/20_crud.yml | 13 ++++ .../rest-api-spec/test/ingest/90_simulate.yml | 13 ++++ .../RestMultiSearchTemplateAction.java | 4 -- .../mustache/RestPutSearchTemplateAction.java | 2 +- .../mustache/RestSearchTemplateAction.java | 4 -- .../test/lang_mustache/10_basic.yml | 13 ++++ .../test/lang_mustache/30_search_template.yml | 13 ++++ .../50_multi_search_template.yml | 12 ++++ .../AbstractBulkByQueryRestHandler.java | 34 +++------- .../reindex/RestDeleteByQueryAction.java | 3 - .../rest-api-spec/test/reindex/10_basic.yml | 13 ++++ .../rest-api-spec/test/bulk/10_basic.yml | 30 +++++++++ .../test/cluster.put_script/10_basic.yml | 12 ++++ .../test/cluster.put_settings/10_basic.yml | 12 ++++ .../rest-api-spec/test/index/10_with_id.yml | 13 ++++ .../test/indices.put_mapping/10_basic.yml | 13 ++++ .../test/indices.put_template/10_basic.yml | 13 ++++ .../rest-api-spec/test/msearch/10_basic.yml | 12 ++++ 29 files changed, 306 insertions(+), 93 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/cluster.put_script/10_basic.yml 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