From a7aedf5d6f62ba8ab81f3c5c0574fa8fd0f8a85d Mon Sep 17 00:00:00 2001
From: Igor Motov <igor@motovs.org>
Date: Mon, 15 Oct 2018 14:18:02 -0400
Subject: [PATCH] Further improve robustness of geo shape parser for malformed
 shapes

Continuation of the work in #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 #34047
---
 .../common/geo/parsers/GeoJsonParser.java     | 39 ++++++++++++++-----
 .../common/geo/GeoJsonShapeParserTests.java   | 29 ++++++++++++++
 2 files changed, 58 insertions(+), 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
index 45ce2b610ca0c..1ca4f36113b11 100644
--- 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
index 17f25d1556d48..e2f5135d4542b 100644
--- 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
+        }
+    }
 }