Geo Point parse error fix

When geo point parsing threw a parse exception, it did not consume
remaining tokens from the parser. This in turn meant that
indexing documents with malformed geo points into mappings with
ignore_malformed=true would fail in some cases, since DocumentParser
expects geo_point parsing to end on the END_OBJECT token.

Related to #17617

Author: henningandersen

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java

@@ -39,10 +39,8 @@ public class XContentSubParser implements XContentParser {
 
     public XContentSubParser(XContentParser parser) {
         this.parser = parser;
-        if (parser.currentToken() != Token.START_OBJECT) {
-            throw new IllegalStateException("The sub parser has to be created on the start of an object");
-        }
-        level = 1;
+        // we allow a non-object, non-array with semantics that it is the single item only.
+        this.level = parser.currentToken() == Token.START_OBJECT || parser.currentToken() == Token.START_ARRAY ? 1 : 0;
     }
 
     @Override

server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -31,6 +31,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.common.xcontent.XContentSubParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.fielddata.FieldData;
@@ -429,11 +430,18 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
      */
     public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, final boolean ignoreZValue, EffectivePoint effectivePoint)
             throws IOException, ElasticsearchParseException {
+        try (XContentSubParser subParser = new XContentSubParser(parser)) {
+            return parseGeoPointUnsafe(subParser, point, ignoreZValue, effectivePoint);
+        }
+    }
+
+    private static GeoPoint parseGeoPointUnsafe(XContentParser parser, GeoPoint point, final boolean ignoreZValue,
+                                                EffectivePoint effectivePoint)
+        throws IOException, ElasticsearchParseException {
         double lat = Double.NaN;
         double lon = Double.NaN;
         String geohash = null;
         NumberFormatException numberFormatException = null;
-        ParseExceptionConsumer exceptionConsumer = new ParseExceptionConsumer();
 
         if(parser.currentToken() == Token.START_OBJECT) {
             while(parser.nextToken() != Token.END_OBJECT) {
@@ -451,7 +459,7 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
                                 }
                                 break;
                             default:
-                                exceptionConsumer.consume(new ElasticsearchParseException("latitude must be a number"));
+                                throw new ElasticsearchParseException("latitude must be a number");
                         }
                     } else if (LONGITUDE.equals(field)) {
                         parser.nextToken();
@@ -465,25 +473,22 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
                                 }
                                 break;
                             default:
-                                exceptionConsumer.consume(new ElasticsearchParseException("longitude must be a number"));
+                                throw new ElasticsearchParseException("longitude must be a number");
                         }
                     } else if (GEOHASH.equals(field)) {
                         if(parser.nextToken() == Token.VALUE_STRING) {
                             geohash = parser.text();
                         } else {
-                            exceptionConsumer.consume(new ElasticsearchParseException("geohash must be a string"));
+                            throw new ElasticsearchParseException("geohash must be a string");
                         }
                     } else {
-                        exceptionConsumer.consume(new ElasticsearchParseException("field must be either [{}], [{}] or [{}]",
-                            LATITUDE, LONGITUDE, GEOHASH));
+                        throw new ElasticsearchParseException("field must be either [{}], [{}] or [{}]", LATITUDE, LONGITUDE, GEOHASH);
                     }
                 } else {
-                    exceptionConsumer.consume(new ElasticsearchParseException("token [{}] not allowed", parser.currentToken()));
+                    throw new ElasticsearchParseException("token [{}] not allowed", parser.currentToken());
                 }
             }
 
-            exceptionConsumer.doThrow();
-
             if (geohash != null) {
                 if(!Double.isNaN(lat) || !Double.isNaN(lon)) {
                     throw new ElasticsearchParseException("field must be either lat/lon or geohash");
@@ -511,17 +516,12 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
                     } else if(element == 2) {
                         lat = parser.doubleValue();
                     } else {
-                        try {
-                            GeoPoint.assertZValue(ignoreZValue, parser.doubleValue());
-                        } catch (ElasticsearchParseException e) {
-                            exceptionConsumer.consume(e);
-                        }
+                        GeoPoint.assertZValue(ignoreZValue, parser.doubleValue());
                     }
                 } else {
-                    exceptionConsumer.consume(new ElasticsearchParseException("numeric value expected"));
+                    throw new ElasticsearchParseException("numeric value expected");
                 }
             }
-            exceptionConsumer.doThrow();
             return point.reset(lat, lon);
         } else if(parser.currentToken() == Token.VALUE_STRING) {
             String val = parser.text();
@@ -704,20 +704,6 @@ public boolean advanceExact(int target) throws IOException {
         }
     }
 
-    private static class ParseExceptionConsumer {
-        private ElasticsearchParseException exception;
-
-        public void consume(ElasticsearchParseException e) {
-            if (exception == null)
-                exception = e;
-        }
-
-        public void doThrow() {
-            if (exception != null)
-                throw exception;
-        }
-    }
-
     private GeoUtils() {
     }
 }