From 85c6a0ec5431874f93c96fcbc651a55bbabeabf4 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 27 Aug 2024 13:33:50 +0200 Subject: [PATCH 1/2] GeographicBoundingBox::create(): accept degenerate bounding box reduced to a point or a line by extending it by the minimal amount to form a non-degenerate one. Also throws when south > north Fixes #4236 --- src/iso19111/metadata.cpp | 21 +++++++++++ test/unit/test_metadata.cpp | 74 ++++++++++++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/iso19111/metadata.cpp b/src/iso19111/metadata.cpp index 148cad191e..6c0a420690 100644 --- a/src/iso19111/metadata.cpp +++ b/src/iso19111/metadata.cpp @@ -238,6 +238,27 @@ GeographicBoundingBoxNNPtr GeographicBoundingBox::create(double west, throw InvalidValueTypeException( "GeographicBoundingBox::create() does not accept NaN values"); } + if (south > north) { + throw InvalidValueTypeException( + "GeographicBoundingBox::create() does not accept south > north"); + } + // Avoid creating a degenerate bounding box if reduced to a point or a line + if (west == east) { + if (west > -180) + west = + std::nextafter(west, -std::numeric_limits::infinity()); + if (east < 180) + east = + std::nextafter(east, std::numeric_limits::infinity()); + } + if (south == north) { + if (south > -90) + south = + std::nextafter(south, -std::numeric_limits::infinity()); + if (north < 90) + north = + std::nextafter(north, std::numeric_limits::infinity()); + } return GeographicBoundingBox::nn_make_shared( west, south, east, north); } diff --git a/test/unit/test_metadata.cpp b/test/unit/test_metadata.cpp index 6b21728293..da65c9740b 100644 --- a/test/unit/test_metadata.cpp +++ b/test/unit/test_metadata.cpp @@ -284,16 +284,6 @@ TEST(metadata, extent_edge_cases) { optional(), std::vector(), std::vector(), std::vector()); - { - auto A = Extent::createFromBBOX(-180, -90, 180, 90); - auto B = Extent::createFromBBOX(180, -90, 180, 90); - EXPECT_FALSE(A->intersects(B)); - EXPECT_FALSE(B->intersects(A)); - EXPECT_FALSE(A->contains(B)); - EXPECT_TRUE(A->intersection(B) == nullptr); - EXPECT_TRUE(B->intersection(A) == nullptr); - } - EXPECT_THROW(Extent::createFromBBOX( std::numeric_limits::quiet_NaN(), -90, 180, 90), InvalidValueTypeException); @@ -307,6 +297,10 @@ TEST(metadata, extent_edge_cases) { -180, -90, 180, std::numeric_limits::quiet_NaN()), InvalidValueTypeException); + // South > north + EXPECT_THROW(Extent::createFromBBOX(-100, 10, 100, 0), + InvalidValueTypeException); + // Scenario of https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57328 // and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=60084 { @@ -325,6 +319,66 @@ TEST(metadata, extent_edge_cases) { EXPECT_TRUE(A->intersection(B) == nullptr); EXPECT_TRUE(B->intersection(A) == nullptr); } + + // Test degenerate bounding box on a point + { + auto A = Extent::createFromBBOX(1, 2, 1, 2); + auto B = Extent::createFromBBOX(-10, -10, 10, 10); + EXPECT_TRUE(A->intersects(B)); + EXPECT_TRUE(B->intersects(A)); + EXPECT_FALSE(A->contains(B)); + EXPECT_TRUE(B->contains(A)); + EXPECT_TRUE(A->intersection(B) != nullptr); + EXPECT_TRUE(B->intersection(A) != nullptr); + } + + // Test degenerate bounding box on a line at long=-180 + { + auto A = Extent::createFromBBOX(-180, 2, -180, 3); + auto B = Extent::createFromBBOX(-180, -90, 180, 90); + EXPECT_TRUE(A->intersects(B)); + EXPECT_TRUE(B->intersects(A)); + EXPECT_FALSE(A->contains(B)); + EXPECT_TRUE(B->contains(A)); + EXPECT_TRUE(A->intersection(B) != nullptr); + EXPECT_TRUE(B->intersection(A) != nullptr); + } + + // Test degenerate bounding box on a line at long=180 + { + auto A = Extent::createFromBBOX(180, 2, 180, 3); + auto B = Extent::createFromBBOX(-180, -90, 180, 90); + EXPECT_TRUE(A->intersects(B)); + EXPECT_TRUE(B->intersects(A)); + EXPECT_FALSE(A->contains(B)); + EXPECT_TRUE(B->contains(A)); + EXPECT_TRUE(A->intersection(B) != nullptr); + EXPECT_TRUE(B->intersection(A) != nullptr); + } + + // Test degenerate bounding box on a line at lat=90 + { + auto A = Extent::createFromBBOX(2, 90, 3, 90); + auto B = Extent::createFromBBOX(-180, -90, 180, 90); + EXPECT_TRUE(A->intersects(B)); + EXPECT_TRUE(B->intersects(A)); + EXPECT_FALSE(A->contains(B)); + EXPECT_TRUE(B->contains(A)); + EXPECT_TRUE(A->intersection(B) != nullptr); + EXPECT_TRUE(B->intersection(A) != nullptr); + } + + // Test degenerate bounding box on a line at lat=-90 + { + auto A = Extent::createFromBBOX(2, -90, 3, -90); + auto B = Extent::createFromBBOX(-180, -90, 180, 90); + EXPECT_TRUE(A->intersects(B)); + EXPECT_TRUE(B->intersects(A)); + EXPECT_FALSE(A->contains(B)); + EXPECT_TRUE(B->contains(A)); + EXPECT_TRUE(A->intersection(B) != nullptr); + EXPECT_TRUE(B->intersection(A) != nullptr); + } } // --------------------------------------------------------------------------- From 903cfffd61a0eefc4e387f74208f44a770b58aae Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 27 Aug 2024 13:35:03 +0200 Subject: [PATCH 2/2] cs2cs and projinfo: emit warning when it looks like longitude and latitudes have been switched in --bbox --- src/apps/cs2cs.cpp | 21 ++++++++++++++++++--- src/apps/projinfo.cpp | 19 ++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/apps/cs2cs.cpp b/src/apps/cs2cs.cpp index 88353bf8f0..551249235e 100644 --- a/src/apps/cs2cs.cpp +++ b/src/apps/cs2cs.cpp @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -448,9 +449,23 @@ int main(int argc, char **argv) { std::exit(1); } try { - bboxFilter = Extent::createFromBBOX( - c_locale_stod(bbox[0]), c_locale_stod(bbox[1]), - c_locale_stod(bbox[2]), c_locale_stod(bbox[3])) + std::vector bboxValues = { + c_locale_stod(bbox[0]), c_locale_stod(bbox[1]), + c_locale_stod(bbox[2]), c_locale_stod(bbox[3])}; + const double west = bboxValues[0]; + const double south = bboxValues[1]; + const double east = bboxValues[2]; + const double north = bboxValues[3]; + constexpr double SOME_MARGIN = 10; + if (south < -90 - SOME_MARGIN && std::fabs(west) <= 90 && + std::fabs(east) <= 90) + std::cerr << "Warning: suspicious south latitude: " << south + << std::endl; + if (north > 90 + SOME_MARGIN && std::fabs(west) <= 90 && + std::fabs(east) <= 90) + std::cerr << "Warning: suspicious north latitude: " << north + << std::endl; + bboxFilter = Extent::createFromBBOX(west, south, east, north) .as_nullable(); } catch (const std::exception &e) { std::cerr << "Invalid value for option --bbox: " << bboxStr diff --git a/src/apps/projinfo.cpp b/src/apps/projinfo.cpp index b432e0867d..a100327b22 100644 --- a/src/apps/projinfo.cpp +++ b/src/apps/projinfo.cpp @@ -30,6 +30,7 @@ #define FROM_PROJ_CPP +#include #include #include // std::ifstream #include @@ -187,9 +188,21 @@ static ExtentPtr makeBboxFilter(DatabaseContextPtr dbContext, std::vector bboxValues = { c_locale_stod(bbox[0]), c_locale_stod(bbox[1]), c_locale_stod(bbox[2]), c_locale_stod(bbox[3])}; - bboxFilter = Extent::createFromBBOX(bboxValues[0], bboxValues[1], - bboxValues[2], bboxValues[3]) - .as_nullable(); + const double west = bboxValues[0]; + const double south = bboxValues[1]; + const double east = bboxValues[2]; + const double north = bboxValues[3]; + constexpr double SOME_MARGIN = 10; + if (south < -90 - SOME_MARGIN && std::fabs(west) <= 90 && + std::fabs(east) <= 90) + std::cerr << "Warning: suspicious south latitude: " << south + << std::endl; + if (north > 90 + SOME_MARGIN && std::fabs(west) <= 90 && + std::fabs(east) <= 90) + std::cerr << "Warning: suspicious north latitude: " << north + << std::endl; + bboxFilter = + Extent::createFromBBOX(west, south, east, north).as_nullable(); } catch (const std::exception &e) { std::cerr << "Invalid value for option --bbox: " << bboxStr << ", " << e.what() << std::endl;