Throw error if query element doesn't end with END_OBJECT (#20528)

* Throw error if query element doesn't end with END_OBJECT

Followup to #20515 where we added validation that after we parse a query within a query element, we should not get a field name. Truth is that the only token allowed at that point is END_OBJECT, as our DSL allows only one single query within the query object:

```
{
  "query" : {
    "term" : { "field" : "value" }
  }
}
```

We can then check that after parsing of the query we have an end_object that closes the query itself (which we already do). Following that we can check that the query object is immediately closed, as there are no other tokens that can be present in that position.

Relates to #20515

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

@@ -338,10 +338,6 @@ public static Optional<BoolQueryBuilder> fromXContent(QueryParseContext parseCon
                 default:
                     throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]");
                 }
-                if (parser.currentToken() != XContentParser.Token.END_OBJECT) {
-                    throw new ParsingException(parser.getTokenLocation(),
-                            "expected [END_OBJECT] but got [{}], possibly too many query clauses", parser.currentToken());
-                }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
                     switch (currentFieldName) {

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

@@ -25,11 +25,9 @@
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.logging.Loggers;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.indices.query.IndicesQueriesRegistry;
 import org.elasticsearch.script.Script;
-import org.elasticsearch.script.ScriptSettings;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -95,16 +93,12 @@ public QueryBuilder parseTopLevelQueryBuilder() {
      * Parses a query excluding the query element that wraps it
      */
     public Optional<QueryBuilder> parseInnerQueryBuilder() throws IOException {
-        // move to START object
-        XContentParser.Token token;
         if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
-            token = parser.nextToken();
-            if (token != XContentParser.Token.START_OBJECT) {
+            if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
                 throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, must start with start_object");
             }
         }
-        token = parser.nextToken();
-        if (token == XContentParser.Token.END_OBJECT) {
+        if (parser.nextToken() == XContentParser.Token.END_OBJECT) {
             // we encountered '{}' for a query clause
             String msg = "query malformed, empty clause found at [" + parser.getTokenLocation() +"]";
             DEPRECATION_LOGGER.deprecated(msg);
@@ -113,26 +107,26 @@ public Optional<QueryBuilder> parseInnerQueryBuilder() throws IOException {
             }
             return Optional.empty();
         }
-        if (token != XContentParser.Token.FIELD_NAME) {
+        if (parser.currentToken() != XContentParser.Token.FIELD_NAME) {
             throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object");
         }
         String queryName = parser.currentName();
         // move to the next START_OBJECT
-        token = parser.nextToken();
-        if (token != XContentParser.Token.START_OBJECT) {
+        if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
             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);
+        //end_object of the specific query (e.g. match, multi_match etc.) element
         if (parser.currentToken() != XContentParser.Token.END_OBJECT) {
             throw new ParsingException(parser.getTokenLocation(),
                     "[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]");
         }
-        parser.nextToken();
-        if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
+        //end_object of the query object
+        if (parser.nextToken() != XContentParser.Token.END_OBJECT) {
             throw new ParsingException(parser.getTokenLocation(),
-                    "[" + queryName + "] malformed query, unexpected [FIELD_NAME] found [" + parser.currentName() + "]");
+                    "[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]");
         }
         return result;
     }

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

@@ -365,12 +365,22 @@ public void testUnknownQueryName() throws IOException {
      * test that two queries in object throws error
      */
     public void testTooManyQueriesInObject() throws IOException {
-        String clauseType = randomFrom(new String[] {"must", "should", "must_not", "filter"});
+        String clauseType = randomFrom("must", "should", "must_not", "filter");
         // should also throw error if invalid query is preceded by a valid one
-        String query = "{\"bool\" : {\"" + clauseType
-                + "\" : { \"match\" : { \"foo\" : \"bar\" } , \"match\" : { \"baz\" : \"buzz\" } } } }";
+        String query = "{\n" +
+                "  \"bool\": {\n" +
+                "    \"" + clauseType + "\": {\n" +
+                "      \"match\": {\n" +
+                "        \"foo\": \"bar\"\n" +
+                "      },\n" +
+                "      \"match\": {\n" +
+                "        \"baz\": \"buzz\"\n" +
+                "      }\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
         ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY));
-        assertEquals("[match] malformed query, unexpected [FIELD_NAME] found [match]", ex.getMessage());
+        assertEquals("[match] malformed query, expected [END_OBJECT] but found [FIELD_NAME]", ex.getMessage());
     }
 
     public void testRewrite() throws IOException {

core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java

@@ -700,7 +700,7 @@ public void testMalformedQueryFunctionFieldNotSupported() throws IOException {
         expectParsingException(json, "field [not_supported] is not supported");
     }
 
-    public void testMalformedQuery() throws IOException {
+    public void testMalformedQueryMultipleQueryObjects() throws IOException {
         //verify that an error is thrown rather than setting the query twice (https://github.com/elastic/elasticsearch/issues/16583)
         String json = "{\n" +
                 "    \"function_score\":{\n" +
@@ -715,15 +715,34 @@ public void testMalformedQuery() throws IOException {
                 "        }\n" +
                 "    }\n" +
                 "}";
-        expectParsingException(json, equalTo("[bool] malformed query, unexpected [FIELD_NAME] found [ignored_field_name]"));
+        expectParsingException(json, equalTo("[bool] malformed query, expected [END_OBJECT] but found [FIELD_NAME]"));
     }
 
-    private void expectParsingException(String json, Matcher<String> messageMatcher) {
+    public void testMalformedQueryMultipleQueryElements() throws IOException {
+        String json = "{\n" +
+                "    \"function_score\":{\n" +
+                "        \"query\":{\n" +
+                "            \"bool\":{\n" +
+                "                \"must\":{\"match\":{\"field\":\"value\"}}" +
+                "             }\n" +
+                "            },\n" +
+                "        \"query\":{\n" +
+                "            \"bool\":{\n" +
+                "                \"must\":{\"match\":{\"field\":\"value\"}}" +
+                "             }\n" +
+                "            }\n" +
+                "        }\n" +
+                "    }\n" +
+                "}";
+        expectParsingException(json, "[query] is already defined.");
+    }
+
+    private static void expectParsingException(String json, Matcher<String> messageMatcher) {
         ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json));
         assertThat(e.getMessage(), messageMatcher);
     }
 
-    private void expectParsingException(String json, String message) {
+    private static void expectParsingException(String json, String message) {
         expectParsingException(json, equalTo("failed to parse [function_score] query. " + message));
     }
 

core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java

@@ -423,8 +423,9 @@ public void testParseIncludeExclude() throws IOException {
         }
     }
 
-    public void testInvalid() throws Exception {
-        String restContent = " { \"query\": {\n" +
+    public void testMultipleQueryObjectsAreRejected() throws Exception {
+        String restContent =
+                " { \"query\": {\n" +
                 "    \"multi_match\": {\n" +
                 "      \"query\": \"workd\",\n" +
                 "      \"fields\": [\"title^5\", \"plain_body\"]\n" +
@@ -436,11 +437,9 @@ public void testInvalid() throws Exception {
                 "    }\n" +
                 "  } }";
         try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
-            SearchSourceBuilder.fromXContent(createParseContext(parser),
-                    searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers);
-            fail("invalid query syntax multiple keys under query");
-        } catch (ParsingException e) {
-            assertThat(e.getMessage(), containsString("filters"));
+            ParsingException e = expectThrows(ParsingException.class, () -> SearchSourceBuilder.fromXContent(createParseContext(parser),
+                    searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers));
+            assertEquals("[multi_match] malformed query, expected [END_OBJECT] but found [FIELD_NAME]", e.getMessage());
         }
     }