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 14c127c0703..0877ea1c66b 100644 --- a/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java @@ -44,6 +44,7 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptService.ScriptType; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -671,9 +672,15 @@ public class UpdateRequest extends InstanceShardOperationRequest } else if ("detect_noop".equals(currentFieldName)) { detectNoop(parser.booleanValue()); } else if ("fields".equals(currentFieldName)) { - List values = parser.list(); - String[] fields = values.toArray(new String[values.size()]); - fields(fields); + List fields = null; + if (token == XContentParser.Token.START_ARRAY) { + fields = (List) parser.list(); + } else if (token.isValue()) { + fields = Collections.singletonList(parser.text()); + } + if (fields != null) { + fields(fields.toArray(new String[fields.size()])); + } } else { //here we don't have settings available, unable to throw deprecation exceptions scriptParameterParser.token(currentFieldName, token, parser, ParseFieldMatcher.EMPTY); diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index fd4aa782302..e994b81832f 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.xcontent.support; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.xcontent.XContentParser; @@ -286,14 +287,21 @@ public abstract class AbstractXContentParser implements XContentParser { static List readList(XContentParser parser, MapFactory mapFactory) throws IOException { XContentParser.Token token = parser.currentToken(); + if (token == null) { + token = parser.nextToken(); + } if (token == XContentParser.Token.FIELD_NAME) { token = parser.nextToken(); } if (token == XContentParser.Token.START_ARRAY) { token = parser.nextToken(); + } else { + throw new ElasticsearchParseException("Failed to parse list: expecting " + + XContentParser.Token.START_ARRAY + " but got " + token); } + ArrayList list = new ArrayList<>(); - for (; token != XContentParser.Token.END_ARRAY; token = parser.nextToken()) { + for (; token != null && token != XContentParser.Token.END_ARRAY; token = parser.nextToken()) { list.add(readValue(parser, mapFactory, token)); } return list; 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 bcb26613388..d105a4bf63b 100644 --- a/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.action.update; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.io.stream.Streamable; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -36,6 +37,7 @@ import org.elasticsearch.test.ESTestCase; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -179,4 +181,17 @@ public class UpdateRequestTests extends ESTestCase { assertThat(e.getMessage(), equalTo("Failed to derive xcontent")); } } + + // Related to issue 15338 + public void testFieldsParsing() throws Exception { + UpdateRequest request = new UpdateRequest("test", "type1", "1") + .source(new BytesArray("{\"doc\": {\"field1\": \"value1\"}, \"fields\": \"_source\"}")); + assertThat(request.doc().sourceAsMap().get("field1").toString(), equalTo("value1")); + assertThat(request.fields(), arrayContaining("_source")); + + request = new UpdateRequest("test", "type2", "2") + .source(new BytesArray("{\"doc\": {\"field2\": \"value2\"}, \"fields\": [\"field1\", \"field2\"]}")); + assertThat(request.doc().sourceAsMap().get("field2").toString(), equalTo("value2")); + assertThat(request.fields(), arrayContaining("field1", "field2")); + } } diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/XContentParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/XContentParserTests.java new file mode 100644 index 00000000000..cce349f417c --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/xcontent/XContentParserTests.java @@ -0,0 +1,78 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.xcontent; + +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.nullValue; + +public class XContentParserTests extends ESTestCase { + + public void testReadList() throws IOException { + assertThat(readList("{\"foo\": [\"bar\"]}"), contains("bar")); + assertThat(readList("{\"foo\": [\"bar\",\"baz\"]}"), contains("bar", "baz")); + assertThat(readList("{\"foo\": [1, 2, 3], \"bar\": 4}"), contains(1, 2, 3)); + assertThat(readList("{\"foo\": [{\"bar\":1},{\"baz\":2},{\"qux\":3}]}"), hasSize(3)); + assertThat(readList("{\"foo\": [null]}"), contains(nullValue())); + assertThat(readList("{\"foo\": []}"), hasSize(0)); + assertThat(readList("{\"foo\": [1]}"), contains(1)); + assertThat(readList("{\"foo\": [1,2]}"), contains(1, 2)); + assertThat(readList("{\"foo\": [{},{},{},{}]}"), hasSize(4)); + } + + public void testReadListThrowsException() throws IOException { + // Calling XContentParser.list() or listOrderedMap() to read a simple + // value or object should throw an exception + assertReadListThrowsException("{\"foo\": \"bar\"}"); + assertReadListThrowsException("{\"foo\": 1, \"bar\": 2}"); + assertReadListThrowsException("{\"foo\": {\"bar\":\"baz\"}}"); + } + + @SuppressWarnings("unchecked") + private static List readList(String source) throws IOException { + try (XContentParser parser = XContentType.JSON.xContent().createParser(source)) { + XContentParser.Token token = parser.nextToken(); + assertThat(token, equalTo(XContentParser.Token.START_OBJECT)); + token = parser.nextToken(); + assertThat(token, equalTo(XContentParser.Token.FIELD_NAME)); + assertThat(parser.currentName(), equalTo("foo")); + return (List) (randomBoolean() ? parser.listOrderedMap() : parser.list()); + } + } + + private void assertReadListThrowsException(String source) { + try { + readList(source); + fail("should have thrown a parse exception"); + } catch (Exception e) { + assertThat(e, instanceOf(ElasticsearchParseException.class)); + assertThat(e.getMessage(), containsString("Failed to parse list")); + } + } +}