Merge pull request #19750 from javanna/fix/npe_parse_field_array

Throw ParsingException if a query is wrapped in an array

core/src/main/java/org/elasticsearch/common/ParseField.java

@@ -23,6 +23,7 @@ import org.elasticsearch.common.logging.Loggers;
 
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -108,6 +109,7 @@ public class ParseField {
      *         names for this {@link ParseField}.
      */
     boolean match(String fieldName, boolean strict) {
+        Objects.requireNonNull(fieldName, "fieldName cannot be null");
         // if this parse field has not been completely deprecated then try to
         // match the preferred name
         if (allReplacedWith == null && fieldName.equals(name)) {

core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java

@@ -109,13 +109,13 @@ public class QueryParseContext implements ParseFieldMatcherSupplier {
         String queryName = parser.currentName();
         // move to the next START_OBJECT
         token = parser.nextToken();
-        if (token != XContentParser.Token.START_OBJECT && token != XContentParser.Token.START_ARRAY) {
+        if (token != XContentParser.Token.START_OBJECT) {
-            throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no start_object after query name");
+            throw new ParsingException(parser.getTokenLocation(), "[" + queryName + "] query malformed, no start_object after query name");
         }
         @SuppressWarnings("unchecked")
         Optional<QueryBuilder> result = (Optional<QueryBuilder>) indicesQueriesRegistry.lookup(queryName, parseFieldMatcher,
                 parser.getTokenLocation()).fromXContent(this);
-        if (parser.currentToken() == XContentParser.Token.END_OBJECT || parser.currentToken() == XContentParser.Token.END_ARRAY) {
+        if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
             // if we are at END_OBJECT, move to the next one...
             parser.nextToken();
         }

core/src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTests.java

@@ -24,8 +24,6 @@ import org.apache.lucene.search.Query;
 import org.elasticsearch.test.AbstractQueryTestCase;
 
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
 
@@ -36,16 +34,6 @@ public class MatchAllQueryBuilderTests extends AbstractQueryTestCase<MatchAllQue
         return new MatchAllQueryBuilder();
     }
 
-    @Override
-    protected Map<String, MatchAllQueryBuilder> getAlternateVersions() {
-        Map<String, MatchAllQueryBuilder> alternateVersions = new HashMap<>();
-        String queryAsString = "{\n" +
-                "    \"match_all\": []\n" +
-                "}";
-        alternateVersions.put(queryAsString, new MatchAllQueryBuilder());
-        return alternateVersions;
-    }
-
     @Override
     protected void doAssertLuceneQuery(MatchAllQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
         assertThat(query, instanceOf(MatchAllDocsQuery.class));

core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java

@@ -113,7 +113,7 @@ public class QueryParseContextTests extends ESTestCase {
         try (XContentParser parser = JsonXContent.jsonXContent.createParser(source)) {
             QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT);
             ParsingException exception = expectThrows(ParsingException.class, () ->  context.parseInnerQueryBuilder());
-            assertEquals("[_na] query malformed, no start_object after query name", exception.getMessage());
+            assertEquals("[foo] query malformed, no start_object after query name", exception.getMessage());
         }
 
         source = "{ \"foo\" : {} }";

test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java

@@ -122,6 +122,7 @@ import static java.util.Collections.emptyList;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.either;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 
@@ -313,6 +314,44 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         }
     }
 
+    /**
+     * Test that wraps the randomly generated query into an array as follows: { "query_name" : [{}]}
+     * This causes unexpected situations in parser code that may not be handled properly.
+     */
+    public void testQueryWrappedInArray() throws IOException {
+        QB queryBuilder = createTestQueryBuilder();
+        String validQuery = queryBuilder.toString();
+        String queryName = queryBuilder.getName();
+        int i = validQuery.indexOf("\"" + queryName + "\"");
+        assertThat(i, greaterThan(0));
+
+        int insertionPosition;
+        for (insertionPosition = i; insertionPosition < validQuery.length(); insertionPosition++) {
+            if (validQuery.charAt(insertionPosition) == ':') {
+                break;
+            }
+        }
+        insertionPosition++;
+
+        int endArrayPosition;
+        for (endArrayPosition = validQuery.length() - 1; endArrayPosition >= 0; endArrayPosition--) {
+            if (validQuery.charAt(endArrayPosition) == '}') {
+                break;
+            }
+        }
+
+        String testQuery = validQuery.substring(0, insertionPosition) + "[" +
+                validQuery.substring(insertionPosition, endArrayPosition) + "]" +
+                validQuery.substring(endArrayPosition, validQuery.length());
+
+        try {
+            parseQuery(testQuery);
+            fail("some parsing exception expected for query: " + testQuery);
+        } catch (ParsingException e) {
+            assertEquals("[" + queryName + "] query malformed, no start_object after query name", e.getMessage());
+        }
+    }
+
     /**
      * Returns alternate string representation of the query that need to be tested as they are never used as output
      * of {@link QueryBuilder#toXContent(XContentBuilder, ToXContent.Params)}. By default there are no alternate versions.