From 4626ce6f5bdb13570cc613b178f06e150d9b6b4e Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 19 Oct 2023 15:46:13 +0200 Subject: [PATCH] WKT1 parser: in non-strict mode, accept missing UNIT[] in GEOGCS, GEOCCS, PROJCS and VERT_CS elements (fixes #3925) --- src/iso19111/io.cpp | 48 ++++++++++++++---- test/unit/test_io.cpp | 113 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 9 deletions(-) diff --git a/src/iso19111/io.cpp b/src/iso19111/io.cpp index e189ad54d2..1513b7c225 100644 --- a/src/iso19111/io.cpp +++ b/src/iso19111/io.cpp @@ -1308,6 +1308,8 @@ struct WKTParser::Private { Private &operator=(const Private &) = delete; void emitRecoverableWarning(const std::string &errorMsg); + void emitRecoverableMissingUNIT(const std::string &parentNodeName, + const UnitOfMeasure &fallbackUnit); BaseObjectNNPtr build(const WKTNodeNNPtr &node); @@ -2740,16 +2742,36 @@ WKTParser::Private::buildAxis(const WKTNodeNNPtr &node, static const PropertyMap emptyPropertyMap{}; +// --------------------------------------------------------------------------- + PROJ_NO_RETURN static void ThrowParsingException(const std::string &msg) { throw ParsingException(msg); } +// --------------------------------------------------------------------------- + static ParsingException buildParsingExceptionInvalidAxisCount(const std::string &csType) { return ParsingException( concat("buildCS: invalid CS axis count for ", csType)); } +// --------------------------------------------------------------------------- + +void WKTParser::Private::emitRecoverableMissingUNIT( + const std::string &parentNodeName, const UnitOfMeasure &fallbackUnit) { + std::string msg("buildCS: missing UNIT in "); + msg += parentNodeName; + if (!strict_ && fallbackUnit == UnitOfMeasure::METRE) { + msg += ". Assuming metre"; + } else if (!strict_ && fallbackUnit == UnitOfMeasure::DEGREE) { + msg += ". Assuming degree"; + } + emitRecoverableWarning(msg); +} + +// --------------------------------------------------------------------------- + CoordinateSystemNNPtr WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ const WKTNodeNNPtr &parentNode, @@ -2759,6 +2781,7 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ const int numberOfAxis = parentNode->countChildrenOfName(WKTConstants::AXIS); int axisCount = numberOfAxis; + const auto &parentNodeName = parentNode->GP()->value(); if (!isNull(node)) { const auto *nodeP = node->GP(); const auto &children = nodeP->children(); @@ -2774,7 +2797,6 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ } } else { const char *csTypeCStr = ""; - const auto &parentNodeName = parentNode->GP()->value(); if (ci_equal(parentNodeName, WKTConstants::GEOCCS)) { csTypeCStr = CartesianCS::WKT2_TYPE; isGeocentric = true; @@ -2782,7 +2804,8 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ auto unit = buildUnitInSubNode(parentNode, UnitOfMeasure::Type::LINEAR); if (unit == UnitOfMeasure::NONE) { - ThrowParsingExceptionMissingUNIT(); + unit = UnitOfMeasure::METRE; + emitRecoverableMissingUNIT(parentNodeName, unit); } return CartesianCS::createGeocentric(unit); } @@ -2794,7 +2817,8 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ auto unit = buildUnitInSubNode(parentNode, UnitOfMeasure::Type::ANGULAR); if (unit == UnitOfMeasure::NONE) { - ThrowParsingExceptionMissingUNIT(); + unit = defaultAngularUnit; + emitRecoverableMissingUNIT(parentNodeName, unit); } // ESRI WKT for geographic 3D CRS @@ -2830,10 +2854,9 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ auto unit = buildUnitInSubNode(parentNode, UnitOfMeasure::Type::LINEAR); if (unit == UnitOfMeasure::NONE) { + unit = UnitOfMeasure::METRE; if (ci_equal(parentNodeName, WKTConstants::PROJCS)) { - ThrowParsingExceptionMissingUNIT(); - } else { - unit = UnitOfMeasure::METRE; + emitRecoverableMissingUNIT(parentNodeName, unit); } } return CartesianCS::createEastingNorthing(unit); @@ -2875,11 +2898,10 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ auto unit = buildUnitInSubNode(parentNode, UnitOfMeasure::Type::LINEAR); if (unit == UnitOfMeasure::NONE) { + unit = UnitOfMeasure::METRE; if (ci_equal(parentNodeName, WKTConstants::VERT_CS) || ci_equal(parentNodeName, WKTConstants::VERTCS)) { - ThrowParsingExceptionMissingUNIT(); - } else { - unit = UnitOfMeasure::METRE; + emitRecoverableMissingUNIT(parentNodeName, unit); } } if (downDirection) { @@ -2974,6 +2996,14 @@ WKTParser::Private::buildCS(const WKTNodeNNPtr &node, /* maybe null */ : UnitOfMeasure::Type::UNKNOWN; UnitOfMeasure unit = buildUnitInSubNode(parentNode, unitType); + if (unit == UnitOfMeasure::NONE) { + if (ci_equal(parentNodeName, WKTConstants::VERT_CS) || + ci_equal(parentNodeName, WKTConstants::VERTCS)) { + unit = UnitOfMeasure::METRE; + emitRecoverableMissingUNIT(parentNodeName, unit); + } + } + std::vector axisList; for (int i = 0; i < axisCount; i++) { axisList.emplace_back( diff --git a/test/unit/test_io.cpp b/test/unit/test_io.cpp index 61c0ea2462..210181e4b4 100644 --- a/test/unit/test_io.cpp +++ b/test/unit/test_io.cpp @@ -785,6 +785,24 @@ TEST(wkt_parse, wkt1_geographic_epsg_org_api_4258) { // --------------------------------------------------------------------------- +TEST(wkt_parse, wkt1_geographic_missing_unit_and_axis) { + auto wkt = "GEOGCS[\"WGS 84\",DATUM[\"WGS_1984\"," + "SPHEROID[\"WGS 84\",6378137,298.257223563]]]]"; + + // Missing UNIT[] is illegal in strict mode + EXPECT_THROW(WKTParser().createFromWKT(wkt), ParsingException); + + auto obj = WKTParser().setStrict(false).createFromWKT(wkt); + auto crs = nn_dynamic_pointer_cast(obj); + ASSERT_TRUE(crs != nullptr); + + auto cs = crs->coordinateSystem(); + ASSERT_EQ(cs->axisList().size(), 2U); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::DEGREE); +} + +// --------------------------------------------------------------------------- + TEST(wkt_parse, wkt1_geocentric_with_PROJ4_extension) { auto wkt = "GEOCCS[\"WGS 84\",\n" " DATUM[\"unknown\",\n" @@ -817,6 +835,24 @@ TEST(wkt_parse, wkt1_geocentric_with_PROJ4_extension) { // --------------------------------------------------------------------------- +TEST(wkt_parse, wkt1_geocentric_missing_unit_and_axis) { + auto wkt = "GEOCCS[\"WGS 84\",DATUM[\"WGS_1984\"," + "SPHEROID[\"WGS 84\",6378137,298.257223563]]]]"; + + // Missing UNIT[] is illegal in strict mode + EXPECT_THROW(WKTParser().createFromWKT(wkt), ParsingException); + + auto obj = WKTParser().setStrict(false).createFromWKT(wkt); + auto crs = nn_dynamic_pointer_cast(obj); + ASSERT_TRUE(crs != nullptr); + + auto cs = crs->coordinateSystem(); + ASSERT_EQ(cs->axisList().size(), 3U); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::METRE); +} + +// --------------------------------------------------------------------------- + TEST(wkt_parse, wkt1_non_conformant_inf_inverse_flattening) { // Some WKT in the wild use "inf". Cf SPHEROID["unnamed",6370997,"inf"] // in https://zenodo.org/record/3878979#.Y_P4g4CZNH4, @@ -1465,6 +1501,35 @@ TEST(wkt_parse, wkt1_projected_with_PROJ4_extension) { // --------------------------------------------------------------------------- +TEST(wkt_parse, wkt1_projected_missing_unit_and_axis) { + auto wkt = "PROJCS[\"WGS 84 / UTM zone 31N\",GEOGCS[\"WGS 84\"," + "DATUM[\"WGS_1984\",SPHEROID[\"WGS 84\",6378137,298.257223563," + "AUTHORITY[\"EPSG\",\"7030\"]],AUTHORITY[\"EPSG\",\"6326\"]]," + "PRIMEM[\"Greenwich\",0,AUTHORITY[\"EPSG\",\"8901\"]]," + "UNIT[\"degree\",0.0174532925199433," + "AUTHORITY[\"EPSG\",\"9122\"]]," + "AUTHORITY[\"EPSG\",\"4326\"]]," + "PROJECTION[\"Transverse_Mercator\"]," + "PARAMETER[\"latitude_of_origin\",0]," + "PARAMETER[\"central_meridian\",3]," + "PARAMETER[\"scale_factor\",0.9996]," + "PARAMETER[\"false_easting\",500000]," + "PARAMETER[\"false_northing\",0]]"; + + // Missing UNIT[] is illegal in strict mode + EXPECT_THROW(WKTParser().createFromWKT(wkt), ParsingException); + + auto obj = WKTParser().setStrict(false).createFromWKT(wkt); + auto crs = nn_dynamic_pointer_cast(obj); + ASSERT_TRUE(crs != nullptr); + + auto cs = crs->coordinateSystem(); + ASSERT_EQ(cs->axisList().size(), 2U); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::METRE); +} + +// --------------------------------------------------------------------------- + TEST(wkt_parse, wkt1_Mercator_1SP_with_latitude_origin_0) { auto wkt = "PROJCS[\"unnamed\",\n" " GEOGCS[\"WGS 84\",\n" @@ -2857,6 +2922,54 @@ TEST(wkt_parse, vertcrs_WKT1_GDAL_minimum) { ASSERT_EQ(cs->axisList().size(), 1U); EXPECT_EQ(cs->axisList()[0]->nameStr(), "Gravity-related height"); EXPECT_EQ(cs->axisList()[0]->direction(), AxisDirection::UP); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::METRE); +} + +// --------------------------------------------------------------------------- + +TEST(wkt_parse, vertcrs_WKT1_GDAL_missing_unit_and_axis) { + auto wkt = "VERT_CS[\"ODN height\",\n" + " VERT_DATUM[\"Ordnance Datum Newlyn\",2005]]"; + + // Missing UNIT[] is illegal in strict mode + EXPECT_THROW(WKTParser().createFromWKT(wkt), ParsingException); + + auto obj = WKTParser().setStrict(false).createFromWKT(wkt); + auto crs = nn_dynamic_pointer_cast(obj); + EXPECT_EQ(crs->nameStr(), "ODN height"); + + auto datum = crs->datum(); + EXPECT_EQ(datum->nameStr(), "Ordnance Datum Newlyn"); + + auto cs = crs->coordinateSystem(); + ASSERT_EQ(cs->axisList().size(), 1U); + EXPECT_EQ(cs->axisList()[0]->nameStr(), "Gravity-related height"); + EXPECT_EQ(cs->axisList()[0]->direction(), AxisDirection::UP); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::METRE); +} + +// --------------------------------------------------------------------------- + +TEST(wkt_parse, vertcrs_WKT1_GDAl_missing_unit_with_axis) { + auto wkt = "VERT_CS[\"ODN height\",\n" + " VERT_DATUM[\"Ordnance Datum Newlyn\",2005],\n" + " AXIS[\"gravity-related height\",UP]]"; + + // Missing UNIT[] is illegal in strict mode + EXPECT_THROW(WKTParser().createFromWKT(wkt), ParsingException); + + auto obj = WKTParser().setStrict(false).createFromWKT(wkt); + auto crs = nn_dynamic_pointer_cast(obj); + EXPECT_EQ(crs->nameStr(), "ODN height"); + + auto datum = crs->datum(); + EXPECT_EQ(datum->nameStr(), "Ordnance Datum Newlyn"); + + auto cs = crs->coordinateSystem(); + ASSERT_EQ(cs->axisList().size(), 1U); + EXPECT_EQ(cs->axisList()[0]->nameStr(), "Gravity-related height"); + EXPECT_EQ(cs->axisList()[0]->direction(), AxisDirection::UP); + EXPECT_EQ(cs->axisList()[0]->unit(), UnitOfMeasure::METRE); } // ---------------------------------------------------------------------------