Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LargestEmptyCircle crash on malformed input #1003

Closed
pramsey opened this issue Nov 29, 2023 · 8 comments · Fixed by #1004
Closed

LargestEmptyCircle crash on malformed input #1003

pramsey opened this issue Nov 29, 2023 · 8 comments · Fixed by #1004
Labels

Comments

@pramsey
Copy link
Member

pramsey commented Nov 29, 2023

From https://trac.osgeo.org/postgis/ticket/5626#comment:3

Test case for LargestEmptyCircleTest.cpp

// https://trac.osgeo.org/postgis/ticket/5626
template<>
template<>
void object::test<20>()
{
    checkCircle(
        "POINT(-11 40)", // input
        "POLYGON((-71.1 42.3,-71.1 42.4,-71.2 42.3,-71.1 42.3))", // boundary
        1, // build tolerance
        5.5, 4.5, 2.12 ); // expected x, y, radius
}

This doesn't appear to be structural, other combinations of point geometry outside of the polygon boundary don't yield a crash. On the other hand, the crash doesn't seem to be a tightly bounded numerical issue, as this rounded up version of the initial bug report still crashes.

@pramsey pramsey added the Bug label Nov 29, 2023
@pramsey
Copy link
Member Author

pramsey commented Nov 29, 2023

Moving the point input around also changes nothing, the problem seems to be in the facet distance search. It's not the 3-edge polygon, other 3-edge polygons work, and if you round the triangle coordinates even more, so it's a one-unit edge, the crash goes away too.

@dr-jts
Copy link
Contributor

dr-jts commented Nov 29, 2023

Works in JTS.

@dr-jts
Copy link
Contributor

dr-jts commented Nov 29, 2023

It looks like the issue is the horizontal botton edge of the triangle ( LINESTRING(-71.2 42.3,-71.1 42.3)) along with the right angle. Lengthening the edge while keeping it horizontal and keeping the right angle still crashes. Moving other vertices, or lengthening the bottom edge to the right doesn't cause a crash.

@robe2
Copy link
Member

robe2 commented Nov 29, 2023

@pramsey @dr-jts are you sure this isn't a recent issue. I saw the error locally when I was fixing the garden tests, but even with my fixes I didn't see berrie and berrie64 (the two raspberry pis we have testing PostGIS and GEOS). But then I realized I had them testing against 3.12.0 (instead of following the branches).

I'm going to check the GEOS old versions I have lieing around to check, but given JTS can't trigger it, I'm thinking its an issue introduced in a recent micro release.

@dr-jts
Copy link
Contributor

dr-jts commented Nov 29, 2023

I'm going to check the GEOS old versions I have lieing around to check, but given JTS can't trigger it, I'm thinking its an issue introduced in a recent micro release.

Sounds good, I'll check too.

@pramsey
Copy link
Member Author

pramsey commented Nov 29, 2023

You are correct, 3.12.0 does not crash.

@dr-jts
Copy link
Contributor

dr-jts commented Nov 29, 2023

It might be due to #939. But that may not help - it's probably just the change that reveals the problem.

@robe2
Copy link
Member

robe2 commented Nov 30, 2023

You are correct, 3.12.0 does not crash.

I just tested on 3.12.2dev and no crash (so looks like this is a new issue in 3.13 cycle)

pramsey added a commit to pramsey/geos that referenced this issue Nov 30, 2023
pramsey added a commit that referenced this issue Nov 30, 2023
* Fix and tests for GH-1003
* Call to init() was missing? Does not explain why the bug in the original issue seemed tied to particular coordinates, as the fix will apply across all inputs. So be warned, there may still be something hiding in there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants