Skip to content

Commit a7aedf5

Browse files
committed
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
1 parent 23ece92 commit a7aedf5

File tree

2 files changed

+58
-10
lines changed

2 files changed

+58
-10
lines changed

server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,22 @@ private static CoordinateNode parseCoordinates(XContentParser parser, boolean ig
166166
}
167167

168168
List<CoordinateNode> nodes = new ArrayList<>();
169-
while (token != XContentParser.Token.END_ARRAY) {
170-
CoordinateNode node = parseCoordinates(parser, ignoreZValue);
171-
if (nodes.isEmpty() == false && nodes.get(0).numDimensions() != node.numDimensions()) {
172-
throw new ElasticsearchParseException("Exception parsing coordinates: number of dimensions do not match");
169+
try {
170+
while (token != XContentParser.Token.END_ARRAY) {
171+
CoordinateNode node = parseCoordinates(parser, ignoreZValue);
172+
if (nodes.isEmpty() == false && nodes.get(0).numDimensions() != node.numDimensions()) {
173+
throw new ElasticsearchParseException("Exception parsing coordinates: number of dimensions do not match");
174+
}
175+
nodes.add(node);
176+
token = parser.nextToken();
177+
}
178+
} catch (Exception ex) {
179+
// Skip all other fields until the end of the array
180+
while (parser.currentToken() != XContentParser.Token.END_ARRAY && parser.currentToken() != null) {
181+
parser.nextToken();
182+
parser.skipChildren();
173183
}
174-
nodes.add(node);
175-
token = parser.nextToken();
184+
throw ex;
176185
}
177186

178187
return new CoordinateNode(nodes);
@@ -216,10 +225,20 @@ static GeometryCollectionBuilder parseGeometries(XContentParser parser, GeoShape
216225

217226
XContentParser.Token token = parser.nextToken();
218227
GeometryCollectionBuilder geometryCollection = new GeometryCollectionBuilder();
219-
while (token != XContentParser.Token.END_ARRAY) {
220-
ShapeBuilder shapeBuilder = ShapeParser.parse(parser);
221-
geometryCollection.shape(shapeBuilder);
222-
token = parser.nextToken();
228+
try {
229+
230+
while (token != XContentParser.Token.END_ARRAY) {
231+
ShapeBuilder shapeBuilder = ShapeParser.parse(parser);
232+
geometryCollection.shape(shapeBuilder);
233+
token = parser.nextToken();
234+
}
235+
} catch (Exception ex) {
236+
// Skip all other fields until the end of the array
237+
while (parser.currentToken() != XContentParser.Token.END_ARRAY && parser.currentToken() != null) {
238+
parser.nextToken();
239+
parser.skipChildren();
240+
}
241+
throw ex;
223242
}
224243

225244
return geometryCollection;

server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,4 +1213,33 @@ public void testParseInvalidShapes() throws IOException {
12131213
assertNull(parser.nextToken());
12141214
}
12151215
}
1216+
1217+
public void testParseInvalidGeometryCollectionShapes() throws IOException {
1218+
// single dimensions point
1219+
XContentBuilder invalidPoints = XContentFactory.jsonBuilder()
1220+
.startObject()
1221+
.startObject("foo")
1222+
.field("type", "geometrycollection")
1223+
.startArray("geometries")
1224+
.startObject()
1225+
.field("type", "polygon")
1226+
.startArray("coordinates")
1227+
.startArray().value("46.6022226498514").value("24.7237442867977").endArray()
1228+
.startArray().value("46.6031857243798").value("24.722968774929").endArray()
1229+
.endArray() // coordinates
1230+
.endObject()
1231+
.endArray() // geometries
1232+
.endObject()
1233+
.endObject();
1234+
1235+
1236+
try (XContentParser parser = createParser(invalidPoints)) {
1237+
parser.nextToken(); // foo
1238+
parser.nextToken(); // start object
1239+
parser.nextToken(); // start object
1240+
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
1241+
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); // end of the document
1242+
assertNull(parser.nextToken()); // no more elements afterwards
1243+
}
1244+
}
12161245
}

0 commit comments

Comments
 (0)