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

Prevent integer overflow during segment touch ratio calculation #354

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 48 additions & 15 deletions include/boost/geometry/strategies/cartesian/intersection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,24 +339,57 @@ struct cartesian_segments
// (only calculated for non-collinear segments)
if (! collinear)
{
math::detail::equals_factor_policy<robust_coordinate_type>
policy(robust_dx_a, robust_dy_a, robust_dx_b, robust_dy_b);

robust_coordinate_type const zero = 0;

robust_coordinate_type robust_da0, robust_da;
robust_coordinate_type robust_db0, robust_db;

cramers_rule(robust_dx_a, robust_dy_a, robust_dx_b, robust_dy_b,
get<0>(robust_a1) - get<0>(robust_b1),
get<1>(robust_a1) - get<1>(robust_b1),
robust_da0, robust_da);
bool robust_da0_is_zero = false;
if (sides.get<0, 0>() == 0)
{
robust_da0 = 1;
robust_da = 0;
}
else if (sides.get<0, 1>() == 0)
{
robust_da0 = 1;
robust_da = 1;
}
else
{
cramers_rule(robust_dx_a, robust_dy_a, robust_dx_b, robust_dy_b,
get<0>(robust_a1) - get<0>(robust_b1),
get<1>(robust_a1) - get<1>(robust_b1),
robust_da0, robust_da);

cramers_rule(robust_dx_b, robust_dy_b, robust_dx_a, robust_dy_a,
get<0>(robust_b1) - get<0>(robust_a1),
get<1>(robust_b1) - get<1>(robust_a1),
robust_db0, robust_db);
robust_da0_is_zero = math::detail::equals_by_policy(robust_da0, zero, policy);
}

math::detail::equals_factor_policy<robust_coordinate_type>
policy(robust_dx_a, robust_dy_a, robust_dx_b, robust_dy_b);
robust_coordinate_type const zero = 0;
if (math::detail::equals_by_policy(robust_da0, zero, policy)
|| math::detail::equals_by_policy(robust_db0, zero, policy))
bool robust_db0_is_zero = false;
if (sides.get<1, 0>() == 0)
{
robust_db0 = 1;
robust_db = 0;
}
else if (sides.get<1, 1>() == 0)
{
robust_db0 = 1;
robust_db = 1;
}
else
{
cramers_rule(robust_dx_b, robust_dy_b, robust_dx_a, robust_dy_a,
get<0>(robust_b1) - get<0>(robust_a1),
get<1>(robust_b1) - get<1>(robust_a1),
robust_db0, robust_db);

robust_db0_is_zero = math::detail::equals_by_policy(robust_db0, zero, policy);
}

if (robust_da0_is_zero || robust_db0_is_zero)
{
// If this is the case, no rescaling is done for FP precision.
// We set it to collinear, but it indicates a robustness issue.
Expand Down Expand Up @@ -536,7 +569,7 @@ struct cartesian_segments
int const a2_wrt_b = position_value(oa_2, ob_1, ob_2);
int const b1_wrt_a = position_value(ob_1, oa_1, oa_2);
int const b2_wrt_a = position_value(ob_2, oa_1, oa_2);

// fix the ratios if necessary
// CONSIDER: fixing ratios also in other cases, if they're inconsistent
// e.g. if ratio == 1 or 0 (so IP at the endpoint)
Expand All @@ -553,7 +586,7 @@ struct cartesian_segments
{
ra_from.assign(1, 1);
rb_to.assign(0, 1);
}
}

if (a2_wrt_b == 1)
{
Expand Down
5 changes: 3 additions & 2 deletions test/algorithms/is_simple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,12 @@ BOOST_AUTO_TEST_CASE( test_geometry_with_NaN_coordinates )
bg::read_wkt("LINESTRING(-1 1,1.115235e+308 1.738137e+308)", ls2);

// the intersection of the two linestrings is a new linestring
// (multilinestring with a single element) that has NaN coordinates
// (multilinestring with a single element) that is an isolated point
multi_linestring_type mls;
bg::intersection(ls1, ls2, mls);

test_simple(mls, true, false);
// "isolated point"-linestring is not simple
test_simple(mls, false, false);
}

BOOST_AUTO_TEST_CASE( test_is_simple_variant )
Expand Down
2 changes: 1 addition & 1 deletion test/algorithms/is_valid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ inline void test_open_rings()
typedef bg::model::ring<Point, false, true> CG; // ccw, closed ring
typedef bg::model::ring<Point, true, false> CW_OG; // cw, open ring
typedef bg::model::ring<Point, true, true> CW_CG; // cw, closed ring

typedef validity_tester_areal<AllowDuplicates> tester;
typedef test_valid<tester, OG, CG, CW_OG, CW_CG> test;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,7 @@ BOOST_AUTO_TEST_CASE( test_intersection_ml_ml_degenerate )
(8.5655 2.85228),(5.26567 4.81254),(4 3.8),\
(1.4995 3.27036),(0.591231 3.43401),\
(-0.706503 3.66784),\
(-0.7654 8.88178e-16,-0.7654 0,5 3))"),
(-0.7654 0,5 3))"),
from_wkt<ML>("MULTILINESTRING((1.87562 6.68515),(1.60494 6),\
(1.18124 4.9275),(1.00439 4.47984),(0.91526 4.25422),\
(0.729883 3.78498),(0.614728 3.49349),\
Expand Down Expand Up @@ -1340,7 +1340,7 @@ BOOST_AUTO_TEST_CASE( test_intersection_ml_ml_degenerate )
(9.98265 0.00543606),(9.09826 -100.515944),\
(7.08745 -329.0674155),(5.06428 -559.024344),\
(3.23365 -767.0972558),(3.16036 -775.427199),\
(-0.7654 8.88178e-16,-0.7654 0,5 3))"),
(-0.7654 0,5 3))"),
#endif
"mlmli21",
1e-4
Expand All @@ -1355,7 +1355,7 @@ BOOST_AUTO_TEST_CASE( test_intersection_ml_ml_spikes )
{
#ifdef BOOST_GEOMETRY_TEST_DEBUG
std::cout << std::endl << std::endl << std::endl;
std::cout << "*** MULTILINESTRING / MULTILINESTRING INTERSECTION"
std::cout << "*** MULTILINESTRING / MULTILINESTRING INTERSECTION"
<< " (WITH SPIKES) ***"
<< std::endl;
std::cout << std::endl;
Expand Down