Further improve robustness of geo shape parser for malformed shapes

Continuation of the work in elastic#31449. Ensures that malformed geoshapes are reliably ignored if "ignore_malformed" is set to true instead of failing the entire document by making sure that xcontent parse is left in a coherent state even if a data format parsing error occurred. Fixes elastic#34047
imotov · Oct 15, 2018 · a7aedf5 · a7aedf5
1 parent 23ece92
commit a7aedf5
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 10 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java b/server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java
@@ -166,13 +166,22 @@ private static CoordinateNode parseCoordinates(XContentParser parser, boolean ig
         }
 
         List<CoordinateNode> nodes = new ArrayList<>();
-        while (token != XContentParser.Token.END_ARRAY) {
-            CoordinateNode node = parseCoordinates(parser, ignoreZValue);
-            if (nodes.isEmpty() == false && nodes.get(0).numDimensions() != node.numDimensions()) {
-                throw new ElasticsearchParseException("Exception parsing coordinates: number of dimensions do not match");
+        try {
+            while (token != XContentParser.Token.END_ARRAY) {
+                CoordinateNode node = parseCoordinates(parser, ignoreZValue);
+                if (nodes.isEmpty() == false && nodes.get(0).numDimensions() != node.numDimensions()) {
+                    throw new ElasticsearchParseException("Exception parsing coordinates: number of dimensions do not match");
+                }
+                nodes.add(node);
+                token = parser.nextToken();
+            }
+        } catch (Exception ex) {
+            // Skip all other fields until the end of the array
+            while (parser.currentToken() != XContentParser.Token.END_ARRAY && parser.currentToken() != null) {
+                parser.nextToken();
+                parser.skipChildren();
             }
-            nodes.add(node);
-            token = parser.nextToken();
+            throw ex;
         }
 
         return new CoordinateNode(nodes);
@@ -216,10 +225,20 @@ static GeometryCollectionBuilder parseGeometries(XContentParser parser, GeoShape
 
         XContentParser.Token token = parser.nextToken();
         GeometryCollectionBuilder geometryCollection = new GeometryCollectionBuilder();
-        while (token != XContentParser.Token.END_ARRAY) {
-            ShapeBuilder shapeBuilder = ShapeParser.parse(parser);
-            geometryCollection.shape(shapeBuilder);
-            token = parser.nextToken();
+        try {
+
+            while (token != XContentParser.Token.END_ARRAY) {
+                ShapeBuilder shapeBuilder = ShapeParser.parse(parser);
+                geometryCollection.shape(shapeBuilder);
+                token = parser.nextToken();
+            }
+        } catch (Exception ex) {
+            // Skip all other fields until the end of the array
+            while (parser.currentToken() != XContentParser.Token.END_ARRAY && parser.currentToken() != null) {
+                parser.nextToken();
+                parser.skipChildren();
+            }
+            throw ex;
         }
 
         return geometryCollection;

diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java
@@ -1213,4 +1213,33 @@ public void testParseInvalidShapes() throws IOException {
             assertNull(parser.nextToken());
         }
     }
+
+    public void testParseInvalidGeometryCollectionShapes() throws IOException {
+        // single dimensions point
+        XContentBuilder invalidPoints = XContentFactory.jsonBuilder()
+            .startObject()
+            .startObject("foo")
+            .field("type", "geometrycollection")
+            .startArray("geometries")
+            .startObject()
+            .field("type", "polygon")
+            .startArray("coordinates")
+            .startArray().value("46.6022226498514").value("24.7237442867977").endArray()
+            .startArray().value("46.6031857243798").value("24.722968774929").endArray()
+            .endArray() // coordinates
+            .endObject()
+            .endArray() // geometries
+            .endObject()
+            .endObject();
+
+
+        try (XContentParser parser = createParser(invalidPoints)) {
+            parser.nextToken(); // foo
+            parser.nextToken(); // start object
+            parser.nextToken(); // start object
+            ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
+            assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); // end of the document
+            assertNull(parser.nextToken()); // no more elements afterwards
+        }
+    }
 }