Merge pull request #10497 from rouault/fix_10496

CSV: emit warning when reading invalid WKT
OSGeo · Aug 10, 2024 · 2c5e36e · 2c5e36e
2 parents 7ba36ed + b96bdfb
commit 2c5e36e
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 19 deletions.
diff --git a/autotest/ogr/ogr_csv.py b/autotest/ogr/ogr_csv.py
@@ -3154,6 +3154,32 @@ def test_ogr_csv_inf_nan():
     assert math.isnan(f["v"])
 
 
+###############################################################################
+# Test reading invalid WKT
+
+
+@gdaltest.enable_exceptions()
+def test_ogr_csv_invalid_wkt(tmp_vsimem):
+
+    filename = str(tmp_vsimem / "test.csv")
+
+    with gdaltest.vsi_open(filename, "wb") as fdest:
+        fdest.write(b"id,WKT\n")
+        fdest.write(b'1,"POINT (1"\n')
+        fdest.write(b'1,"POINT (1 2)"\n')
+
+    ds = ogr.Open(filename)
+    lyr = ds.GetLayer(0)
+    gdal.ErrorReset()
+    with gdal.quiet_errors():
+        f = lyr.GetNextFeature()
+    assert gdal.GetLastErrorMsg() == "Ignoring invalid WKT: POINT (1"
+    assert f.GetGeometryRef() is None
+    f = lyr.GetNextFeature()
+    assert gdal.GetLastErrorMsg() == ""
+    assert f.GetGeometryRef().ExportToWkt() == "POINT (1 2)"
+
+
 ###############################################################################
 
 

diff --git a/ogr/ogrsf_frmts/csv/ogrcsvlayer.cpp b/ogr/ogrsf_frmts/csv/ogrcsvlayer.cpp
@@ -1393,40 +1393,62 @@ OGRFeature *OGRCSVLayer::GetNextUnfilteredFeature()
         }
         if (iGeom >= 0)
         {
+            const OGRGeomFieldDefn *poGeomFieldDefn =
+                poFeatureDefn->GetGeomFieldDefn(iGeom);
             if (papszTokens[iAttr][0] != '\0' &&
-                !(poFeatureDefn->GetGeomFieldDefn(iGeom)->IsIgnored()))
+                !(poGeomFieldDefn->IsIgnored()))
             {
                 const char *pszStr = papszTokens[iAttr];
                 while (*pszStr == ' ')
                     pszStr++;
                 OGRGeometry *poGeom = nullptr;
 
-                CPLPushErrorHandler(CPLQuietErrorHandler);
-                if (OGRGeometryFactory::createFromWkt(pszStr, nullptr,
-                                                      &poGeom) == OGRERR_NONE)
+                if (EQUAL(poGeomFieldDefn->GetNameRef(), ""))
                 {
-                    poGeom->assignSpatialReference(
-                        poFeatureDefn->GetGeomFieldDefn(iGeom)
-                            ->GetSpatialRef());
-                    poFeature->SetGeomFieldDirectly(iGeom, poGeom);
+                    if (OGRGeometryFactory::createFromWkt(
+                            pszStr, nullptr, &poGeom) != OGRERR_NONE)
+                    {
+                        CPLError(CE_Warning, CPLE_AppDefined,
+                                 "Ignoring invalid WKT: %s", pszStr);
+                        delete poGeom;
+                        poGeom = nullptr;
+                    }
                 }
-                else if (*pszStr == '{' &&
-                         (poGeom = OGRGeometry::FromHandle(
-                              OGR_G_CreateGeometryFromJson(pszStr))) != nullptr)
+                else
                 {
-                    poFeature->SetGeomFieldDirectly(iGeom, poGeom);
+                    CPLErrorHandlerPusher oErrorHandler(CPLQuietErrorHandler);
+
+                    if (OGRGeometryFactory::createFromWkt(
+                            pszStr, nullptr, &poGeom) != OGRERR_NONE)
+                    {
+                        delete poGeom;
+                        poGeom = nullptr;
+                    }
+
+                    if (!poGeom && *pszStr == '{')
+                    {
+                        poGeom = OGRGeometry::FromHandle(
+                            OGR_G_CreateGeometryFromJson(pszStr));
+                    }
+                    else if (!poGeom && ((*pszStr >= '0' && *pszStr <= '9') ||
+                                         (*pszStr >= 'a' && *pszStr <= 'z') ||
+                                         (*pszStr >= 'A' && *pszStr <= 'Z')))
+                    {
+                        poGeom = OGRGeometryFromHexEWKB(pszStr, nullptr, FALSE);
+                    }
                 }
-                else if (((*pszStr >= '0' && *pszStr <= '9') ||
-                          (*pszStr >= 'a' && *pszStr <= 'z') ||
-                          (*pszStr >= 'A' && *pszStr <= 'Z')) &&
-                         (poGeom = OGRGeometryFromHexEWKB(pszStr, nullptr,
-                                                          FALSE)) != nullptr)
+
+                if (poGeom)
                 {
+                    poGeom->assignSpatialReference(
+                        poGeomFieldDefn->GetSpatialRef());
                     poFeature->SetGeomFieldDirectly(iGeom, poGeom);
                 }
-                CPLPopErrorHandler();
             }
-            if (!bKeepGeomColumns || (iAttr == 0 && bHiddenWKTColumn))
+
+            const bool bHasAttributeField =
+                bKeepGeomColumns && !(iAttr == 0 && bHiddenWKTColumn);
+            if (!bHasAttributeField)
                 continue;
         }