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

WKT1 parser: in non-strict mode, accept missing UNIT[] in GEOGCS, GEOCCS, PROJCS and VERT_CS elements (fixes #3925) #3926

Merged
merged 1 commit into from
Oct 27, 2023
Merged
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
48 changes: 39 additions & 9 deletions src/iso19111/io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -2774,15 +2797,15 @@ 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;
if (axisCount == 0) {
auto unit =
buildUnitInSubNode(parentNode, UnitOfMeasure::Type::LINEAR);
if (unit == UnitOfMeasure::NONE) {
ThrowParsingExceptionMissingUNIT();
unit = UnitOfMeasure::METRE;
emitRecoverableMissingUNIT(parentNodeName, unit);
}
return CartesianCS::createGeocentric(unit);
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<CoordinateSystemAxisNNPtr> axisList;
for (int i = 0; i < axisCount; i++) {
axisList.emplace_back(
Expand Down
113 changes: 113 additions & 0 deletions test/unit/test_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<GeographicCRS>(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"
Expand Down Expand Up @@ -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<GeodeticCRS>(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,
Expand Down Expand Up @@ -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<ProjectedCRS>(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"
Expand Down Expand Up @@ -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<VerticalCRS>(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<VerticalCRS>(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);
}

// ---------------------------------------------------------------------------
Expand Down
Loading