From bdad62a2f8fe1c1455656471e769df11c067cbf6 Mon Sep 17 00:00:00 2001
From: Areek Zillur <areek.zillur@elasticsearch.com>
Date: Thu, 15 Sep 2016 16:32:28 -0400
Subject: [PATCH] Fix silently accepting malformed queries

Currently, we silently accept malformed query where more
than one key is defined at the top-level for query object.
If all the keys have a valid query body, only the last query
is executed, besides throwing off parsing for additional suggest,
aggregation or highlighting defined in the search request.

This commit throws a parsing exception when we encounter a query
with multiple keys.

closes #20500
---
 .../index/query/QueryParseContext.java        |  4 ++++
 .../index/query/BoolQueryBuilderTests.java    |  2 +-
 .../FunctionScoreQueryBuilderTests.java       |  2 +-
 .../builder/SearchSourceBuilderTests.java     | 22 +++++++++++++++++++
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java
index 9ed374db212..7b5fa97825f 100644
--- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java
+++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java
@@ -130,6 +130,10 @@ public class QueryParseContext implements ParseFieldMatcherSupplier {
                     "[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]");
         }
         parser.nextToken();
+        if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
+            throw new ParsingException(parser.getTokenLocation(),
+                    "[" + queryName + "] malformed query, unexpected [FIELD_NAME] found [" + parser.currentName() + "]");
+        }
         return result;
     }
 
diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
index 5e911261171..5f9c7e0881d 100644
--- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
@@ -370,7 +370,7 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
         String query = "{\"bool\" : {\"" + clauseType
                 + "\" : { \"match\" : { \"foo\" : \"bar\" } , \"match\" : { \"baz\" : \"buzz\" } } } }";
         ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY));
-        assertEquals("expected [END_OBJECT] but got [FIELD_NAME], possibly too many query clauses", ex.getMessage());
+        assertEquals("[match] malformed query, unexpected [FIELD_NAME] found [match]", ex.getMessage());
     }
 
     public void testRewrite() throws IOException {
diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
index c5b31eb6a92..c280bc4a49e 100644
--- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
@@ -715,7 +715,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
                 "        }\n" +
                 "    }\n" +
                 "}";
-        expectParsingException(json, "[query] is already defined.");
+        expectParsingException(json, equalTo("[bool] malformed query, unexpected [FIELD_NAME] found [ignored_field_name]"));
     }
 
     private void expectParsingException(String json, Matcher<String> messageMatcher) {
diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java
index 4d2b0d394a8..fe324190106 100644
--- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java
@@ -21,6 +21,7 @@ package org.elasticsearch.search.builder;
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.ParseFieldMatcher;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -422,6 +423,27 @@ public class SearchSourceBuilderTests extends ESTestCase {
         }
     }
 
+    public void testInvalid() throws Exception {
+        String restContent = " { \"query\": {\n" +
+                "    \"multi_match\": {\n" +
+                "      \"query\": \"workd\",\n" +
+                "      \"fields\": [\"title^5\", \"plain_body\"]\n" +
+                "    },\n" +
+                "    \"filters\": {\n" +
+                "      \"terms\": {\n" +
+                "        \"status\": [ 3 ]\n" +
+                "      }\n" +
+                "    }\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"));
+        }
+    }
+
     public void testParseSort() throws IOException {
         {
             String restContent = " { \"sort\": \"foo\"}";