Improve error detection in geo_filter parsing

Relates to #5370

Author: bleskes

src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java

@@ -53,7 +53,7 @@
 public class GeoPolygonFilterParser implements FilterParser {
 
     public static final String NAME = "geo_polygon";
-    public static final String POINTS = "points"; 
+    public static final String POINTS = "points";
 
     @Inject
     public GeoPolygonFilterParser() {
@@ -91,13 +91,15 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
                     if (token == XContentParser.Token.FIELD_NAME) {
                         currentFieldName = parser.currentName();
                     } else if (token == XContentParser.Token.START_ARRAY) {
-                        if(POINTS.equals(currentFieldName)) {
-                            while((token = parser.nextToken()) != Token.END_ARRAY) {
+                        if (POINTS.equals(currentFieldName)) {
+                            while ((token = parser.nextToken()) != Token.END_ARRAY) {
                                 shell.add(GeoPoint.parse(parser));
                             }
                         } else {
                             throw new QueryParsingException(parseContext.index(), "[geo_polygon] filter does not support [" + currentFieldName + "]");
                         }
+                    } else {
+                        throw new QueryParsingException(parseContext.index(), "[geo_polygon] filter does not support token type [" + token.name() + "] under [" + currentFieldName + "]");
                     }
                 }
             } else if (token.isValue()) {
@@ -113,20 +115,22 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
                 } else {
                     throw new QueryParsingException(parseContext.index(), "[geo_polygon] filter does not support [" + currentFieldName + "]");
                 }
+            } else {
+                throw new QueryParsingException(parseContext.index(), "[geo_polygon] unexpected token type [" + token.name() + "]");
             }
         }
 
         if (shell.isEmpty()) {
             throw new QueryParsingException(parseContext.index(), "no points defined for geo_polygon filter");
         } else {
-            if(shell.size() < 3) {
+            if (shell.size() < 3) {
                 throw new QueryParsingException(parseContext.index(), "to few points defined for geo_polygon filter");
             }
             GeoPoint start = shell.get(0);
-            if(!start.equals(shell.get(shell.size()-1))) {
+            if (!start.equals(shell.get(shell.size() - 1))) {
                 shell.add(start);
             }
-            if(shell.size() < 4) {
+            if (shell.size() < 4) {
                 throw new QueryParsingException(parseContext.index(), "to few points defined for geo_polygon filter");
             }
         }

src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java

@@ -2038,6 +2038,29 @@ public void testGeoPolygonNamedFilter() throws IOException {
         assertThat(filter.points()[2].lon(), closeTo(-90, 0.00001));
     }
 
+
+    @Test
+    public void testGeoPolygonFilterParsingExceptions() throws IOException {
+        String[] brokenFiles = new String[]{
+                "/org/elasticsearch/index/query/geo_polygon_exception_1.json",
+                "/org/elasticsearch/index/query/geo_polygon_exception_2.json",
+                "/org/elasticsearch/index/query/geo_polygon_exception_3.json",
+                "/org/elasticsearch/index/query/geo_polygon_exception_4.json",
+                "/org/elasticsearch/index/query/geo_polygon_exception_5.json"
+        };
+        for (String brokenFile : brokenFiles) {
+            IndexQueryParserService queryParser = queryParser();
+            String query = copyToStringFromClasspath(brokenFile);
+            try {
+                queryParser.parse(query).query();
+                fail("parsing a broken geo_polygon filter didn't fail as expected while parsing: " + brokenFile);
+            } catch (QueryParsingException e) {
+                // success!
+            }
+        }
+    }
+
+
     @Test
     public void testGeoPolygonFilter1() throws IOException {
         IndexQueryParserService queryParser = queryParser();
@@ -2181,7 +2204,7 @@ public void testFilterParsing() throws IOException {
         Query parsedQuery = queryParser.parse(query).query();
         assertThat((double) (parsedQuery.getBoost()), Matchers.closeTo(3.0, 1.e-7));
     }
-    
+
     @Test
     public void testBadTypeMatchQuery() throws Exception {
         IndexQueryParserService queryParser = queryParser();
@@ -2193,7 +2216,7 @@ public void testBadTypeMatchQuery() throws Exception {
             expectedException = qpe;
         }
         assertThat(expectedException, notNullValue());
-    }     
+    }
 
     @Test
     public void testMultiMatchQuery() throws Exception {

src/test/java/org/elasticsearch/index/query/geo_polygon_exception_1.json

@@ -0,0 +1,20 @@
+{
+    "filtered": {
+        "query": {
+            "match_all": {}
+        },
+        "filter": {
+            "geo_polygon": {
+                "location": {
+                    "points": {
+                        "points": [
+                            [-70, 40],
+                            [-80, 30],
+                            [-90, 20]
+                        ]
+                    }
+                }
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/geo_polygon_exception_2.json

@@ -0,0 +1,22 @@
+{
+    "filtered": {
+        "query": {
+            "match_all": {}
+        },
+        "filter": {
+            "geo_polygon": {
+                "location": {
+                    "points": [
+                        [-70, 40],
+                        [-80, 30],
+                        [-90, 20]
+                    ],
+                    "something_else": {
+
+                    }
+
+                }
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/geo_polygon_exception_3.json

@@ -0,0 +1,12 @@
+{
+    "filtered": {
+        "query": {
+            "match_all": {}
+        },
+        "filter": {
+            "geo_polygon": {
+                "location": ["WRONG"]
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/geo_polygon_exception_4.json

@@ -0,0 +1,19 @@
+{
+    "filtered": {
+        "query": {
+            "match_all": {}
+        },
+        "filter": {
+            "geo_polygon": {
+                "location": {
+                    "points": [
+                        [-70, 40],
+                        [-80, 30],
+                        [-90, 20]
+                    ]
+                },
+                "bla": true
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/geo_polygon_exception_5.json

@@ -0,0 +1,19 @@
+{
+    "filtered": {
+        "query": {
+            "match_all": {}
+        },
+        "filter": {
+            "geo_polygon": {
+                "location": {
+                    "points": [
+                        [-70, 40],
+                        [-80, 30],
+                        [-90, 20]
+                    ]
+                },
+                "bla": ["array"]
+            }
+        }
+    }
+}