Skip to content

Commit

Permalink
[GEO] NPE parsing GeoJSON polygon with single LinearRing
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nknize
nknize committed Jan 16, 2015
1 parent 6ab21e5 commit e69b5c3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 33 deletions.
7 changes: 6 additions & 1 deletion src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,12 @@ protected static LineStringBuilder parseLinearRing(CoordinateNode coordinates) {
* 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(
Expand Down
56 changes: 24 additions & 32 deletions src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -62,7 +61,6 @@ public void testParse_simplePoint() throws IOException {
assertGeometryEquals(new JtsPoint(expected, SPATIAL_CONTEXT), pointGeoJson);
}

@Test
public void testParse_lineString() throws IOException {
String lineGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "LineString")
.startArray("coordinates")
Expand All @@ -80,7 +78,6 @@ public void testParse_lineString() throws IOException {
assertGeometryEquals(jtsGeom(expected), lineGeoJson);
}

@Test
public void testParse_multiLineString() throws IOException {
String multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiLineString")
.startArray("coordinates")
Expand Down Expand Up @@ -108,7 +105,6 @@ public void testParse_multiLineString() throws IOException {
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()
Expand All @@ -119,7 +115,6 @@ public void testParse_circle() throws IOException {
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")
Expand Down Expand Up @@ -165,7 +160,6 @@ public void testParse_envelope() throws IOException {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}

@Test
public void testParse_polygonNoHoles() throws IOException {
String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
.startArray("coordinates")
Expand All @@ -191,7 +185,6 @@ public void testParse_polygonNoHoles() throws IOException {
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")
Expand All @@ -213,7 +206,6 @@ public void testParse_invalidPoint() throws IOException {
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")
Expand Down Expand Up @@ -243,7 +235,6 @@ public void testParse_invalidMultipoint() throws IOException {
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")
Expand Down Expand Up @@ -279,7 +270,6 @@ public void testParse_invalidMultiPolygon() throws IOException {
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")
Expand Down Expand Up @@ -362,7 +352,6 @@ public void testParse_OGCPolygonWithoutHoles() throws IOException {
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")
Expand Down Expand Up @@ -468,87 +457,96 @@ public void testParse_OGCPolygonWithHoles() throws IOException {

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()
.startArray().value(-75.022).value(41.783).endArray()
.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()
.endArray()
.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()
.endArray()
.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()
.endArray()
.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")
Expand Down Expand Up @@ -592,7 +590,6 @@ public void testParse_polygonWithHole() throws IOException {
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")
Expand All @@ -614,7 +611,6 @@ public void testParse_selfCrossingPolygon() throws IOException {
ElasticsearchGeoAssertions.assertValidException(parser, InvalidShapeException.class);
}

@Test
public void testParse_multiPoint() throws IOException {
String multiPointGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPoint")
.startArray("coordinates")
Expand All @@ -629,7 +625,6 @@ public void testParse_multiPoint() throws IOException {
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")
Expand Down Expand Up @@ -742,7 +737,6 @@ public void testParse_multiPolygon() throws IOException {
assertGeometryEquals(jtsGeom(withHoles), multiPolygonGeoJson);
}

@Test
public void testParse_geometryCollection() throws IOException {
String geometryCollectionGeoJson = XContentFactory.jsonBuilder().startObject()
.field("type", "GeometryCollection")
Expand Down Expand Up @@ -775,7 +769,6 @@ public void testParse_geometryCollection() throws IOException {
assertGeometryEquals(shapeCollection(expected), geometryCollectionGeoJson);
}

@Test
public void testThatParserExtractsCorrectTypeAndCoordinatesFromArbitraryJson() throws IOException {
String pointGeoJson = XContentFactory.jsonBuilder().startObject()
.startObject("crs")
Expand All @@ -796,7 +789,6 @@ public void testThatParserExtractsCorrectTypeAndCoordinatesFromArbitraryJson() t
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")
Expand Down

0 comments on commit e69b5c3

Please sign in to comment.