Fix unexpected and unwrapped NullPointerException#994
Open
arthurscchan wants to merge 1 commit intolocationtech:masterfrom
Open
Fix unexpected and unwrapped NullPointerException#994arthurscchan wants to merge 1 commit intolocationtech:masterfrom
arthurscchan wants to merge 1 commit intolocationtech:masterfrom
Conversation
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
Contributor
|
This seems to fall under the task of improving how JTS handles EMPTY geometries. In fact, the It seems to me to be mostly a matter of taste as to whether the problem is detected by an NPE or by an ISE. In any case, if this change is to be made it should be done in all the |
Contributor
|
It would be good if you can complete the Eclipse ECA so that CI can run on this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes an unwrapped NullPointerException where a null value could be returned from getcoordinate() method in modules/core/src/main/java/org/locationtech/jts/geom/Point.java.
The
getCoordinate()method in the Point class will return a null value if there are no coordinates stored. This could cause potential NullPointerException as the return null has been used in many code without further null check. Indeed, further process on empty point with no coordinates are meaningless. For example, in https://github.com/locationtech/jts/blob/master/modules/core/src/main/java/org/locationtech/jts/operation/distance/DistanceOp.java#L423-L427, it calls thegetCoordinate()method and could get a null object. Then the coordinate object is passed to theDistance.pointToSegment()method. The coordinate is then used in https://github.com/locationtech/jts/blob/master/modules/core/src/main/java/org/locationtech/jts/algorithm/Distance.java#L169 and cause a NullPointerException because no null checking is in place. There are many other places that have the similar situation, thus the best way to solve the possible NullPointerException in different locations is to throw an exception directly when thegetCoordinate()method is called on an empty point instance. This is similar to the handling in thegetX()andgetY()method in the same Point class.This PR wraps the possible NullPointerException by throwing an IllegalStateException when the
getCoordinate()method is called on an empty point instance with no coordinates. This avoids further NullPointerException when code trying to use the null coordinate on some other logic.We found this bug using fuzzing by way of OSS-Fuzz, where we recently integrated jts (google/oss-fuzz#10745). OSS-Fuzz is a free service run by Google for fuzzing important open source software. If you'd like to know more about this then I'm happy to go into detail and also set up things so you can receive emails and detailed reports when bugs are found.