[GEO] Fix for NPE enclosed in SearchParseException for a "geo_shape" filter or query

This fix adds better error handling for parsing multipoint, linestring, and polygon GeoJSONs.  Current logic throws a NPE when parsing a multipoint, linestring, or polygon that does not comply with the GeoJSON specification. That is, if a user provides a single coordinate instead of an array of coordinates, or array of linestrings, the ShapeParser throws a NPE wrapped in a SearchParseException instead of a more useful error message.

Closes #8432

src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java

@@ -346,6 +346,10 @@ public abstract class ShapeBuilder implements ToXContent {
             this.coordinate = null;
         }
 
+        protected boolean isEmpty() {
+            return (coordinate == null && (children == null || children.isEmpty()));
+        }
+
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
             if (children == null) {
@@ -607,7 +611,19 @@ public abstract class ShapeBuilder implements ToXContent {
             }
         }
         
+        protected static void validatePointNode(CoordinateNode node) {
+            if (node.isEmpty()) {
+                throw new ElasticsearchParseException("Invalid number of points (0) provided when expecting a single coordinate "
+                        + "([lat, lng])");
+            } else if (node.coordinate == null) {
+                if (node.children.isEmpty() == false) {
+                    throw new ElasticsearchParseException("multipoint data provided when single point data expected.");
+                }
+            }
+        }
+
         protected static PointBuilder parsePoint(CoordinateNode node) {
+            validatePointNode(node);
             return newPoint(node.coordinate);
         }
 
@@ -619,7 +635,24 @@ public abstract class ShapeBuilder implements ToXContent {
             return newEnvelope().topLeft(coordinates.children.get(0).coordinate).bottomRight(coordinates.children.get(1).coordinate);
         }
 
+        protected static void validateMultiPointNode(CoordinateNode coordinates) {
+            if (coordinates.children == null || coordinates.children.isEmpty()) {
+                if (coordinates.coordinate != null) {
+                    throw new ElasticsearchParseException("single coordinate found when expecting an array of " +
+                            "coordinates. change type to point or change data to an array of >0 coordinates");
+                }
+                throw new ElasticsearchParseException("No data provided for multipoint object when expecting " +
+                        ">0 points (e.g., [[lat, lng]] or [[lat, lng], ...])");
+            } else {
+                for (CoordinateNode point : coordinates.children) {
+                    validatePointNode(point);
+                }
+            }
+        }
+
         protected static MultiPointBuilder parseMultiPoint(CoordinateNode coordinates) {
+            validateMultiPointNode(coordinates);
+
             MultiPointBuilder points = new MultiPointBuilder();
             for (CoordinateNode node : coordinates.children) {
                 points.point(node.coordinate);
@@ -671,6 +704,11 @@ public abstract class ShapeBuilder implements ToXContent {
         }
 
         protected static PolygonBuilder parsePolygon(CoordinateNode coordinates) {
+            if (coordinates.children == null || coordinates.children.isEmpty()) {
+                throw new ElasticsearchParseException("Invalid LinearRing provided for type polygon. Linear ring must be an array of " +
+                        "coordinates");
+            }
+
             LineStringBuilder shell = parseLinearRing(coordinates.children.get(0));
             PolygonBuilder polygon = new PolygonBuilder(shell.points);
             for (int i = 1; i < coordinates.children.size(); i++) {

src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java

@@ -157,6 +157,58 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
         assertGeometryEquals(jtsGeom(expected), polygonGeoJson);
     }
 
+    @Test
+    public void testParse_invalidPoint() throws IOException {
+        // test case 1: create an invalid point object with multipoint data format
+        String invalidPoint1 = XContentFactory.jsonBuilder().startObject().field("type", "point")
+                .startArray("coordinates")
+                .startArray().value(-74.011).value(40.753).endArray()
+                .endArray()
+                .endObject().string();
+        XContentParser parser = JsonXContent.jsonXContent.createParser(invalidPoint1);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+
+        // test case 2: create an invalid point object with an empty number of coordinates
+        String invalidPoint2 = XContentFactory.jsonBuilder().startObject().field("type", "point")
+                .startArray("coordinates")
+                .endArray()
+                .endObject().string();
+        parser = JsonXContent.jsonXContent.createParser(invalidPoint2);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+    }
+
+    @Test
+    public void testParse_invalidMultipoint() throws IOException {
+        // test case 1: create an invalid multipoint object with single coordinate
+        String invalidMultipoint1 = XContentFactory.jsonBuilder().startObject().field("type", "multipoint")
+                .startArray("coordinates").value(-74.011).value(40.753).endArray()
+                .endObject().string();
+        XContentParser parser = JsonXContent.jsonXContent.createParser(invalidMultipoint1);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+
+        // test case 2: create an invalid multipoint object with null coordinate
+        String invalidMultipoint2 = XContentFactory.jsonBuilder().startObject().field("type", "multipoint")
+                .startArray("coordinates")
+                .endArray()
+                .endObject().string();
+        parser = JsonXContent.jsonXContent.createParser(invalidMultipoint2);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+
+        // test case 3: create a valid formatted multipoint object with invalid number (0) of coordinates
+        String invalidMultipoint3 = XContentFactory.jsonBuilder().startObject().field("type", "multipoint")
+                .startArray("coordinates")
+                .startArray().endArray()
+                .endArray()
+                .endObject().string();
+        parser = JsonXContent.jsonXContent.createParser(invalidMultipoint3);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+    }
+
     @Test
     public void testParse_invalidPolygon() throws IOException {
         /**
@@ -225,6 +277,15 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
         parser = JsonXContent.jsonXContent.createParser(invalidPoly5);
         parser.nextToken();
         ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchIllegalArgumentException.class);
+
+        // test case 6: create an invalid polygon with 0 LinearRings
+        String invalidPoly6 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
+                .startArray("coordinates").endArray()
+                .endObject().string();
+
+        parser = JsonXContent.jsonXContent.createParser(invalidPoly6);
+        parser.nextToken();
+        ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
     }
 
     @Test