Skip to content

Commit

Permalink
geomfn: prevent smaller densifyFrac for ST_{Frechet,Hausdorff}Distance
Browse files Browse the repository at this point in the history
Release justification: low-risk change to existing functionality

Release note (sql change): Prevent densifyFracs < 1e-6 for
ST_FrechetDistance and ST_HausdorffDistance to protect panics and out of
memory errors.
  • Loading branch information
otan committed Mar 3, 2021
1 parent 320dab6 commit 099e843
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
27 changes: 19 additions & 8 deletions pkg/geo/geomfn/distance.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,14 +739,8 @@ func FrechetDistanceDensify(a, b geo.Geometry, densifyFrac float64) (*float64, e
if a.SRID() != b.SRID() {
return nil, geo.NewMismatchingSRIDsError(a.SpatialObject(), b.SpatialObject())
}
// GEOS throws SIGFPE due to division by zero if densifyFraq is too small,
// so we explicitly error instead. The threshold was empirically found to be
// 1e-20, while 1e-19 results in "geos error: vector".
//
// Note that small values of densifyFrac are prohibitively expensive and
// likely to cause out-of-memory conditions.
if densifyFrac < 1e-19 {
return nil, errors.New("densifyFrac is too small")
if err := verifyDensifyFrac(densifyFrac); err != nil {
return nil, err
}
distance, err := geos.FrechetDistanceDensify(a.EWKB(), b.EWKB(), densifyFrac)
if err != nil {
Expand Down Expand Up @@ -778,6 +772,10 @@ func HausdorffDistanceDensify(a, b geo.Geometry, densifyFrac float64) (*float64,
if a.SRID() != b.SRID() {
return nil, geo.NewMismatchingSRIDsError(a.SpatialObject(), b.SpatialObject())
}
if err := verifyDensifyFrac(densifyFrac); err != nil {
return nil, err
}

distance, err := geos.HausdorffDistanceDensify(a.EWKB(), b.EWKB(), densifyFrac)
if err != nil {
return nil, err
Expand All @@ -803,3 +801,16 @@ func ClosestPoint(a, b geo.Geometry) (geo.Geometry, error) {
}
return closestPoint, nil
}

func verifyDensifyFrac(f float64) error {
if f < 0 || f > 1 {
return errors.Newf("fraction must be in range [0, 1], got %f", f)
}
// Very small densifyFrac potentially causes a SIGFPE or generate a large
// amount of memory. Guard against this.
const fracTooSmall = 1e-6
if f > 0 && f < fracTooSmall {
return errors.Newf("fraction %f is too small, must be at least %f", f, fracTooSmall)
}
return nil
}
10 changes: 3 additions & 7 deletions pkg/geo/geomfn/distance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -790,11 +790,8 @@ func TestFrechetDistanceDensify(t *testing.T) {
densifyFrac float64
}{
// Very small densifyFrac causes a SIGFPE in GEOS due to division-by-zero.
// The threshold was empirically found to be at 1e-20, while at 1e-19
// GEOS instead returns the error "geos error: vector". We explicitly
// disallow <1e-19 in the code, and test that both of these error. We do
// not test larger values, since very small densify values consume a
// large amount of memory causing out-of-memory errors.
// We explicitly disallow <1e-6 in the code, and test that both of these error.
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-7},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-19},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-20},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-100},
Expand Down Expand Up @@ -867,8 +864,6 @@ func TestHausdorffDistanceDensify(t *testing.T) {
{"LINESTRING EMPTY", "LINESTRING EMPTY", 0.5, nil},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING EMPTY", 0.5, nil},
{"LINESTRING EMPTY", "LINESTRING (10 10, 10 150, 130 10)", 0.5, nil},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-100, pf(14.142135623730951)},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1e-20, pf(14.142135623730951)},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 0.2, pf(66)},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 0.4, pf(56.66666666666667)},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 0.6, pf(70)},
Expand Down Expand Up @@ -902,6 +897,7 @@ func TestHausdorffDistanceDensify(t *testing.T) {
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", -1},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", -0.1},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 0.0},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 0.0000001},
{"LINESTRING (130 0, 0 0, 0 150)", "LINESTRING (10 10, 10 150, 130 10)", 1.1},
}

Expand Down

0 comments on commit 099e843

Please sign in to comment.