Improve robustness of geo shape parser for malformed shapes (#31449)

Ensures that malformed geoshapes are more reliably ignored if
"ignore_malformed" is set to true instead of failing the entire
document.

Closes #31428
This commit is contained in:
Igor Motov 2018-06-26 06:44:10 -07:00 committed by GitHub
parent 232c71b6bf
commit 544e473908
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 116 additions and 48 deletions

View File

@ -55,57 +55,66 @@ abstract class GeoJsonParser {
String malformedException = null;
XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
String fieldName = parser.currentName();
try {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
String fieldName = parser.currentName();
if (ShapeParser.FIELD_TYPE.match(fieldName, parser.getDeprecationHandler())) {
parser.nextToken();
final GeoShapeType type = GeoShapeType.forName(parser.text());
if (shapeType != null && shapeType.equals(type) == false) {
malformedException = ShapeParser.FIELD_TYPE + " already parsed as ["
+ shapeType + "] cannot redefine as [" + type + "]";
if (ShapeParser.FIELD_TYPE.match(fieldName, parser.getDeprecationHandler())) {
parser.nextToken();
final GeoShapeType type = GeoShapeType.forName(parser.text());
if (shapeType != null && shapeType.equals(type) == false) {
malformedException = ShapeParser.FIELD_TYPE + " already parsed as ["
+ shapeType + "] cannot redefine as [" + type + "]";
} else {
shapeType = type;
}
} else if (ShapeParser.FIELD_COORDINATES.match(fieldName, parser.getDeprecationHandler())) {
parser.nextToken();
CoordinateNode tempNode = parseCoordinates(parser, ignoreZValue.value());
if (coordinateNode != null && tempNode.numDimensions() != coordinateNode.numDimensions()) {
throw new ElasticsearchParseException("Exception parsing coordinates: " +
"number of dimensions do not match");
}
coordinateNode = tempNode;
} else if (ShapeParser.FIELD_GEOMETRIES.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType == null) {
shapeType = GeoShapeType.GEOMETRYCOLLECTION;
} else if (shapeType.equals(GeoShapeType.GEOMETRYCOLLECTION) == false) {
malformedException = "cannot have [" + ShapeParser.FIELD_GEOMETRIES + "] with type set to ["
+ shapeType + "]";
}
parser.nextToken();
geometryCollections = parseGeometries(parser, shapeMapper);
} else if (CircleBuilder.FIELD_RADIUS.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType == null) {
shapeType = GeoShapeType.CIRCLE;
} else if (shapeType != null && shapeType.equals(GeoShapeType.CIRCLE) == false) {
malformedException = "cannot have [" + CircleBuilder.FIELD_RADIUS + "] with type set to ["
+ shapeType + "]";
}
parser.nextToken();
radius = DistanceUnit.Distance.parseDistance(parser.text());
} else if (ShapeParser.FIELD_ORIENTATION.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType != null
&& (shapeType.equals(GeoShapeType.POLYGON) || shapeType.equals(GeoShapeType.MULTIPOLYGON)) == false) {
malformedException = "cannot have [" + ShapeParser.FIELD_ORIENTATION + "] with type set to [" + shapeType + "]";
}
parser.nextToken();
requestedOrientation = ShapeBuilder.Orientation.fromString(parser.text());
} else {
shapeType = type;
parser.nextToken();
parser.skipChildren();
}
} else if (ShapeParser.FIELD_COORDINATES.match(fieldName, parser.getDeprecationHandler())) {
parser.nextToken();
CoordinateNode tempNode = parseCoordinates(parser, ignoreZValue.value());
if (coordinateNode != null && tempNode.numDimensions() != coordinateNode.numDimensions()) {
throw new ElasticsearchParseException("Exception parsing coordinates: " +
"number of dimensions do not match");
}
coordinateNode = tempNode;
} else if (ShapeParser.FIELD_GEOMETRIES.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType == null) {
shapeType = GeoShapeType.GEOMETRYCOLLECTION;
} else if (shapeType.equals(GeoShapeType.GEOMETRYCOLLECTION) == false) {
malformedException = "cannot have [" + ShapeParser.FIELD_GEOMETRIES + "] with type set to ["
+ shapeType + "]";
}
parser.nextToken();
geometryCollections = parseGeometries(parser, shapeMapper);
} else if (CircleBuilder.FIELD_RADIUS.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType == null) {
shapeType = GeoShapeType.CIRCLE;
} else if (shapeType != null && shapeType.equals(GeoShapeType.CIRCLE) == false) {
malformedException = "cannot have [" + CircleBuilder.FIELD_RADIUS + "] with type set to ["
+ shapeType + "]";
}
parser.nextToken();
radius = DistanceUnit.Distance.parseDistance(parser.text());
} else if (ShapeParser.FIELD_ORIENTATION.match(fieldName, parser.getDeprecationHandler())) {
if (shapeType != null
&& (shapeType.equals(GeoShapeType.POLYGON) || shapeType.equals(GeoShapeType.MULTIPOLYGON)) == false) {
malformedException = "cannot have [" + ShapeParser.FIELD_ORIENTATION + "] with type set to [" + shapeType + "]";
}
parser.nextToken();
requestedOrientation = ShapeBuilder.Orientation.fromString(parser.text());
} else {
parser.nextToken();
parser.skipChildren();
}
}
} catch (Exception ex) {
// Skip all other fields until the end of the object
while (parser.currentToken() != XContentParser.Token.END_OBJECT && parser.currentToken() != null) {
parser.nextToken();
parser.skipChildren();
}
throw ex;
}
if (malformedException != null) {
@ -144,6 +153,12 @@ abstract class GeoJsonParser {
* XContentParser
*/
private static CoordinateNode parseCoordinates(XContentParser parser, boolean ignoreZValue) throws IOException {
if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
parser.skipChildren();
parser.nextToken();
throw new ElasticsearchParseException("coordinates cannot be specified as objects");
}
XContentParser.Token token = parser.nextToken();
// Base cases
if (token != XContentParser.Token.START_ARRAY &&
@ -168,8 +183,13 @@ abstract class GeoJsonParser {
}
private static Coordinate parseCoordinate(XContentParser parser, boolean ignoreZValue) throws IOException {
if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) {
throw new ElasticsearchParseException("geo coordinates must be numbers");
}
double lon = parser.doubleValue();
parser.nextToken();
if (parser.nextToken() != XContentParser.Token.VALUE_NUMBER) {
throw new ElasticsearchParseException("geo coordinates must be numbers");
}
double lat = parser.doubleValue();
XContentParser.Token token = parser.nextToken();
// alt (for storing purposes only - future use includes 3d shapes)

View File

@ -145,6 +145,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
XContentParser parser = createParser(pointGeoJson);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
// multi dimension linestring
XContentBuilder lineGeoJson = XContentFactory.jsonBuilder()
@ -159,6 +160,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
parser = createParser(lineGeoJson);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
@Override
@ -196,6 +198,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(multilinesGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test #4: "envelope" with empty coordinates
@ -206,6 +209,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(multilinesGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
@ -291,6 +295,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(polygonGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
@ -306,6 +311,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(invalidPoint1)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 2: create an invalid point object with an empty number of coordinates
@ -318,6 +324,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(invalidPoint2)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
@ -331,6 +338,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(invalidMultipoint1)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 2: create an invalid multipoint object with null coordinate
@ -343,6 +351,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(invalidMultipoint2)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 3: create a valid formatted multipoint object with invalid number (0) of coordinates
@ -356,6 +365,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(invalidMultipoint3)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
@ -392,6 +402,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, multiPolygonGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, InvalidShapeException.class);
assertNull(parser.nextToken());
}
}
@ -432,8 +443,9 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, multiPolygonGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
}
public void testParseOGCPolygonWithoutHoles() throws IOException {
@ -650,6 +662,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 2: create an invalid polygon with only 1 point
@ -664,6 +677,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 3: create an invalid polygon with 0 points
@ -678,6 +692,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 4: create an invalid polygon with null value points
@ -692,6 +707,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, IllegalArgumentException.class);
assertNull(parser.nextToken());
}
// test case 5: create an invalid polygon with 1 invalid LinearRing
@ -704,6 +720,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, IllegalArgumentException.class);
assertNull(parser.nextToken());
}
// test case 6: create an invalid polygon with 0 LinearRings
@ -714,6 +731,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// test case 7: create an invalid polygon with 0 LinearRings
@ -726,6 +744,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidPoly)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
@ -794,6 +813,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, polygonGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, InvalidShapeException.class);
assertNull(parser.nextToken());
}
}
@ -1165,4 +1185,32 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
ElasticsearchGeoAssertions.assertMultiPolygon(shape);
}
}
public void testParseInvalidShapes() throws IOException {
// single dimensions point
XContentBuilder tooLittlePointGeoJson = XContentFactory.jsonBuilder()
.startObject()
.field("type", "Point")
.startArray("coordinates").value(10.0).endArray()
.endObject();
try (XContentParser parser = createParser(tooLittlePointGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
// zero dimensions point
XContentBuilder emptyPointGeoJson = XContentFactory.jsonBuilder()
.startObject()
.field("type", "Point")
.startObject("coordinates").field("foo", "bar").endObject()
.endObject();
try (XContentParser parser = createParser(emptyPointGeoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
assertNull(parser.nextToken());
}
}
}