Fix OOM in AbstractXContentParser

This commit fixes an OOM error that happens when the XContentParser.readList() method is asked to parse a single value instead of an array. It fixes the UpdateRequest parsing as well as remove some leniency in the readList() method so that it expect to be in an array before parsing values.

closes #15338

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<UpdateRequest>
                 } else if ("detect_noop".equals(currentFieldName)) {
                     detectNoop(parser.booleanValue());
                 } else if ("fields".equals(currentFieldName)) {
-                    List<Object> values = parser.list();
-                    String[] fields = values.toArray(new String[values.size()]);
-                    fields(fields);
+                    List<Object> 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);

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<Object> 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<Object> 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;

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"));
+    }
 }

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 <T> List<T> 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<T>) (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"));
+        }
+    }
+}