[GEO] NPE parsing GeoJSON polygon with single LinearRing

ShapeBuilder threw a NPE when a polygon coordinate array consisted of a single LinearRing. This PR fixes the error handling to throw a more useful ElasticsearchParseException to provide the user with better insight into the problem.
This commit is contained in:
Nicholas Knize 2015-01-12 13:41:41 -06:00
parent 6ab21e5b1b
commit e69b5c3424
2 changed files with 30 additions and 33 deletions

View File

@ -861,7 +861,12 @@ public abstract class ShapeBuilder implements ToXContent {
* are equivalent (they represent equivalent points). Though a LinearRing is not explicitly
* represented as a GeoJSON geometry type, it is referred to in the Polygon geometry type definition.
*/
if (coordinates.children.size() < 4) {
if (coordinates.children == null) {
String error = "Invalid LinearRing found.";
error += (coordinates.coordinate == null) ?
" No coordinate array provided" : " Found a single coordinate when expecting a coordinate array";
throw new ElasticsearchParseException(error);
} else if (coordinates.children.size() < 4) {
throw new ElasticsearchParseException("Invalid number of points in LinearRing (found " +
coordinates.children.size() + " - must be >= 4)");
} else if (!coordinates.children.get(0).coordinate.equals(

View File

@ -52,7 +52,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
private final static GeometryFactory GEOMETRY_FACTORY = SPATIAL_CONTEXT.getGeometryFactory();
@Test
public void testParse_simplePoint() throws IOException {
String pointGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Point")
.startArray("coordinates").value(100.0).value(0.0).endArray()
@ -62,7 +61,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(new JtsPoint(expected, SPATIAL_CONTEXT), pointGeoJson);
}
@Test
public void testParse_lineString() throws IOException {
String lineGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "LineString")
.startArray("coordinates")
@ -80,7 +78,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(jtsGeom(expected), lineGeoJson);
}
@Test
public void testParse_multiLineString() throws IOException {
String multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiLineString")
.startArray("coordinates")
@ -108,7 +105,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(jtsGeom(expected), multilinesGeoJson);
}
@Test
public void testParse_circle() throws IOException {
String multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "circle")
.startArray("coordinates").value(100.0).value(0.0).endArray()
@ -119,7 +115,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(expected, multilinesGeoJson);
}
@Test
public void testParse_envelope() throws IOException {
// test #1: envelope with expected coordinate order (TopLeft, BottomRight)
String multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "envelope")
@ -165,7 +160,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_polygonNoHoles() throws IOException {
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
.startArray("coordinates")
@ -191,7 +185,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(jtsGeom(expected), polygonGeoJson);
}
@Test
public void testParse_invalidPoint() throws IOException {
// test case 1: create an invalid point object with multipoint data format
String invalidPoint1 = XContentFactory.jsonBuilder().startObject().field("type", "point")
@ -213,7 +206,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_invalidMultipoint() throws IOException {
// test case 1: create an invalid multipoint object with single coordinate
String invalidMultipoint1 = XContentFactory.jsonBuilder().startObject().field("type", "multipoint")
@ -243,7 +235,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_invalidMultiPolygon() throws IOException {
// test invalid multipolygon (an "accidental" polygon with inner rings outside outer ring)
String multiPolygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPolygon")
@ -279,7 +270,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_OGCPolygonWithoutHoles() throws IOException {
// test 1: ccw poly not crossing dateline
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
@ -362,7 +352,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertMultiPolygon(shape);
}
@Test
public void testParse_OGCPolygonWithHoles() throws IOException {
// test 1: ccw poly not crossing dateline
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
@ -468,15 +457,14 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertMultiPolygon(shape);
}
@Test
public void testParse_invalidPolygon() throws IOException {
/**
* The following 3 test cases ensure proper error handling of invalid polygons
* per the GeoJSON specification
*/
// test case 1: create an invalid polygon with only 2 points
String invalidPoly1 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
String invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.startArray()
.startArray().value(-74.011).value(40.753).endArray()
@ -484,12 +472,12 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
.endArray()
.endArray()
.endObject().string();
XContentParser parser = JsonXContent.jsonXContent.createParser(invalidPoly1);
XContentParser parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
// test case 2: create an invalid polygon with only 1 point
String invalidPoly2 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.startArray()
.startArray().value(-74.011).value(40.753).endArray()
@ -497,12 +485,12 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
.endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly2);
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
// test case 3: create an invalid polygon with 0 points
String invalidPoly3 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.startArray()
.startArray().endArray()
@ -510,12 +498,12 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
.endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly3);
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
// test case 4: create an invalid polygon with null value points
String invalidPoly4 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.startArray()
.startArray().nullValue().nullValue().endArray()
@ -523,32 +511,42 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
.endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly4);
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchIllegalArgumentException.class);
// test case 5: create an invalid polygon with 1 invalid LinearRing
String invalidPoly5 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.nullValue().nullValue()
.endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly5);
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchIllegalArgumentException.class);
// test case 6: create an invalid polygon with 0 LinearRings
String invalidPoly6 = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates").endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly6);
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
// test case 7: create an invalid polygon with 0 LinearRings
invalidPoly = XContentFactory.jsonBuilder().startObject().field("type", "polygon")
.startArray("coordinates")
.startArray().value(-74.011).value(40.753).endArray()
.endArray()
.endObject().string();
parser = JsonXContent.jsonXContent.createParser(invalidPoly);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_polygonWithHole() throws IOException {
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
.startArray("coordinates")
@ -592,7 +590,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(jtsGeom(expected), polygonGeoJson);
}
@Test
public void testParse_selfCrossingPolygon() throws IOException {
// test self crossing ccw poly not crossing dateline
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
@ -614,7 +611,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, InvalidShapeException.class);
}
@Test
public void testParse_multiPoint() throws IOException {
String multiPointGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPoint")
.startArray("coordinates")
@ -629,7 +625,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(expected, multiPointGeoJson);
}
@Test
public void testParse_multiPolygon() throws IOException {
// test #1: two polygons; one without hole, one with hole
String multiPolygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPolygon")
@ -742,7 +737,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(jtsGeom(withHoles), multiPolygonGeoJson);
}
@Test
public void testParse_geometryCollection() throws IOException {
String geometryCollectionGeoJson = XContentFactory.jsonBuilder().startObject()
.field("type", "GeometryCollection")
@ -775,7 +769,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(shapeCollection(expected), geometryCollectionGeoJson);
}
@Test
public void testThatParserExtractsCorrectTypeAndCoordinatesFromArbitraryJson() throws IOException {
String pointGeoJson = XContentFactory.jsonBuilder().startObject()
.startObject("crs")
@ -796,7 +789,6 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
assertGeometryEquals(new JtsPoint(expected, SPATIAL_CONTEXT), pointGeoJson);
}
@Test
public void testParse_orientationOption() throws IOException {
// test 1: valid ccw (right handed system) poly not crossing dateline (with 'right' field)
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")