Fix for NPE enclosed in SearchParseException for a "geo_shape" filter or query #8785
Conversation
nknize
added
:Analytics/Geo
| } | ||
| throw new ElasticsearchParseException("No data provided for multipoint object when expecting " + | ||
| ">0 points (e.g., [[lat, lng]] or [[lat, lng], ...])"); | ||
| } else if (!(coordinates.children.get(0) instanceof CoordinateNode)) { |
There was a problem hiding this comment.
can we use == false instead of ! it's so much easier to read and burned my fingers too often
|
left some minor comments - LGTM feel free to push if you fixes the nitpicks |
|
should this go into 1.5 and 2.0 as well? can you label it? |
| throw new ElasticsearchParseException("No data provided for multipoint object when expecting " + | ||
| ">0 points (e.g., [[lat, lng]] or [[lat, lng], ...])"); | ||
| } else if (coordinates.children.get(0) instanceof CoordinateNode == false) { | ||
| throw new ElasticsearchParseException("multipoint expects an array of >=0 coordinates (e.g., [[lat, lng]]"); |
There was a problem hiding this comment.
This message suggests that it is valid to have a multipoint with no coordinates, is that correct?
There was a problem hiding this comment.
Unlike LinearRing, LineString, and Polygon, the spec doesn't explicitly state a minimum number of points. Since you raised a very valid question - and as I think about more use cases - I think you're correct to point this out as not being user friendly. In fact, accepting 0 coordinates could lead to a lot of silent failures and extremely unhappy users.
I made the change to throw a Parse Exception if an empty array is provided. Good catch!
|
LGTM |
…filter or query This fix adds better error handling for parsing multipoint, linestring, and polygon GeoJSONs. Current logic throws a NPE when parsing a multipoint, linestring, or polygon that does not comply with the GeoJSON specification. That is, if a user provides a single coordinate instead of an array of coordinates, or array of linestrings, the ShapeParser throws a NPE wrapped in a SearchParseException instead of a more useful error message. Closes elastic#8432
clintongormley
changed the title
clintongormley
changed the title
...ter or query
This fix adds better error handling for parsing multipoint, linestring, and polygon GeoJSONs. Current logic throws a NPE when parsing a multipoint, linestring, or polygon that does not comply with the GeoJSON specification. That is, if a user provides a single coordinate instead of an array of coordinates, or array of linestrings, the ShapeParser throws a NPE wrapped in a SearchParseException instead of a more useful error message.
Closes #8432