From 9c8beb822079f0c9bdea0df983e8063c4a0f5e8f Mon Sep 17 00:00:00 2001
From: Lee Hinman <lee@writequit.org>
Date: Wed, 1 Oct 2014 10:23:47 +0200
Subject: [PATCH] Be stricter parsing ids for ids query

Adds a check to make sure that all ids in the query are either strings
or numbers. This is to prevent the case where a user accidentally
specifies:

"ids": [["1", "2"]]

(note the double array)

With this change, an exception will be thrown since the second "[" is
not a string or number, it is a Token.START_ARRAY.

Fixes #7686
---
 .../index/query/IdsQueryParser.java           | 15 +++++++++-----
 .../search/query/SimpleQueryTests.java        | 20 +++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java b/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java
index 455b5187f0f..fc8f2b7103a 100644
--- a/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java
+++ b/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java
@@ -70,11 +70,17 @@ public class IdsQueryParser implements QueryParser {
                 if ("values".equals(currentFieldName)) {
                     idsProvided = true;
                     while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                        BytesRef value = parser.utf8BytesOrNull();
-                        if (value == null) {
-                            throw new QueryParsingException(parseContext.index(), "No value specified for term filter");
+                        if ((token == XContentParser.Token.VALUE_STRING) ||
+                                (token == XContentParser.Token.VALUE_NUMBER)) {
+                            BytesRef value = parser.utf8BytesOrNull();
+                            if (value == null) {
+                                throw new QueryParsingException(parseContext.index(), "No value specified for term filter");
+                            }
+                            ids.add(value);
+                        } else {
+                            throw new QueryParsingException(parseContext.index(),
+                                    "Illegal value for id, expecting a string or number, got: " + token);
                         }
-                        ids.add(value);
                     }
                 } else if ("types".equals(currentFieldName) || "type".equals(currentFieldName)) {
                     types = new ArrayList<>();
@@ -125,4 +131,3 @@ public class IdsQueryParser implements QueryParser {
         return query;
     }
 }
-
diff --git a/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java b/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java
index 30038ce18cf..f1ebede4f0d 100644
--- a/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java
+++ b/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java
@@ -2700,4 +2700,24 @@ public class SimpleQueryTests extends ElasticsearchIntegrationTest {
         }
     }
 
+    @Test // see #7686.
+    public void testIdsQueryWithInvalidValues() throws Exception {
+        createIndex("test");
+        indexRandom(true, false, client().prepareIndex("test", "type", "1").setSource("body", "foo"));
+        try {
+            client().prepareSearch("test")
+                    .setTypes("type")
+                    .setQuery("{\n" +
+                            "  \"ids\": {\n" +
+                            "    \"values\": [[\"1\"]]\n" +
+                            "  }\n" +
+                            "}")
+                    .get();
+            fail("query is invalid and should have produced a parse exception");
+        } catch (Exception e) {
+            assertThat("query could not be parsed due to bad format: " + e.getMessage(),
+                    e.getMessage().contains("Illegal value for id, expecting a string or number, got: START_ARRAY"),
+                    equalTo(true));
+        }
+    }
 }