From 42134cca4d8e61b79b7362f975d092890eecba3a Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 11 Jan 2016 17:04:20 -0500 Subject: [PATCH] Fixes an issue where, if the content type of the request body could not be determined, the UpdateRequest would still try to parse the content instead of throwing the standard ElasticsearchParseException. This manifests when passing illegal JSON in the request body that does not begin with a '{'. By trying to parse the content from an unknown request body content type, the UpdateRequest was throwing a null pointer exception. This has been fixed to throw an ElasticsearchParseException, to be consistent with the behavior of all other requests in the face of undecipherable request content types. Closes #15822 --- .../elasticsearch/action/update/UpdateRequest.java | 5 +++-- .../common/xcontent/XContentFactory.java | 3 +++ .../action/update/UpdateRequestTests.java | 12 ++++++++++++ .../common/xcontent/XContentFactoryTests.java | 8 ++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java index 9e061d29500..6bc69ed4d9c 100644 --- a/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java @@ -639,8 +639,7 @@ public class UpdateRequest extends InstanceShardOperationRequest ScriptParameterParser scriptParameterParser = new ScriptParameterParser(); Map scriptParams = null; Script script = null; - XContentType xContentType = XContentFactory.xContentType(source); - try (XContentParser parser = XContentFactory.xContent(xContentType).createParser(source)) { + try (XContentParser parser = XContentFactory.xContent(source).createParser(source)) { XContentParser.Token token = parser.nextToken(); if (token == null) { return this; @@ -657,10 +656,12 @@ public class UpdateRequest extends InstanceShardOperationRequest } else if ("scripted_upsert".equals(currentFieldName)) { scriptedUpsert = parser.booleanValue(); } else if ("upsert".equals(currentFieldName)) { + XContentType xContentType = XContentFactory.xContentType(source); XContentBuilder builder = XContentFactory.contentBuilder(xContentType); builder.copyCurrentStructure(parser); safeUpsertRequest().source(builder); } else if ("doc".equals(currentFieldName)) { + XContentType xContentType = XContentFactory.xContentType(source); XContentBuilder docBuilder = XContentFactory.contentBuilder(xContentType); docBuilder.copyCurrentStructure(parser); safeDoc().source(docBuilder); diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/XContentFactory.java b/core/src/main/java/org/elasticsearch/common/xcontent/XContentFactory.java index 627486548cd..47dd1249d4a 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/XContentFactory.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/XContentFactory.java @@ -133,6 +133,9 @@ public class XContentFactory { * Returns the {@link org.elasticsearch.common.xcontent.XContent} for the provided content type. */ public static XContent xContent(XContentType type) { + if (type == null) { + throw new IllegalArgumentException("Cannot get xcontent for unknown type"); + } return type.xContent(); } diff --git a/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java b/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java index 51053f63a01..55260957105 100644 --- a/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.update; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.common.io.stream.Streamable; @@ -166,4 +167,15 @@ public class UpdateRequestTests extends ESTestCase { indexAction = (IndexRequest) action; assertThat(indexAction.ttl(), is(providedTTLValue)); } + + // Related to issue #15822 + public void testInvalidBodyThrowsParseException() throws Exception { + UpdateRequest request = new UpdateRequest("test", "type", "1"); + try { + request.source(new byte[] { (byte) '"' }); + fail("Should have thrown a ElasticsearchParseException"); + } catch (ElasticsearchParseException e) { + assertThat(e.getMessage(), equalTo("Failed to derive xcontent")); + } + } } diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/XContentFactoryTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/XContentFactoryTests.java index 9b1cfb64573..583234461b3 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/XContentFactoryTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/XContentFactoryTests.java @@ -97,6 +97,14 @@ public class XContentFactoryTests extends ESTestCase { assertNull(XContentFactory.xContentType(is)); } + public void testInvalidStream() throws Exception { + byte[] bytes = new byte[] { (byte) '"' }; + assertNull(XContentFactory.xContentType(bytes)); + + bytes = new byte[] { (byte) 'x' }; + assertNull(XContentFactory.xContentType(bytes)); + } + public void testJsonFromBytesOptionallyPrecededByUtf8Bom() throws Exception { byte[] bytes = new byte[] {(byte) '{', (byte) '}'}; assertThat(XContentFactory.xContentType(bytes), equalTo(XContentType.JSON));