Merge pull request #8941 from rouault/fix_qgis_55558

GPX: fix crash with GPX_ELE_AS_25D=YES config option (fixes qgis/QGIS#55558, master only); add open options

autotest/ogr/ogr_gpx.py

@@ -526,3 +526,49 @@ def test_ogr_gpx_creator(tmp_vsimem, options, expected):
     data = gdal.VSIFReadL(1, gdal.VSIStatL(filename).size, f)
     gdal.VSIFCloseL(f)
     assert expected in data
+
+
+###############################################################################
+# Test ELE_AS_25D open option
+
+
+def test_ogr_gpx_ELE_AS_25D():
+
+    ds = gdal.OpenEx(
+        "data/gpx/test.gpx", gdal.OF_VECTOR, open_options=["ELE_AS_25D=YES"]
+    )
+    lyr = ds.GetLayerByName("waypoints")
+    f = lyr.GetNextFeature()
+    assert f.GetGeometryRef().ExportToIsoWkt() == "POINT Z (1 0 2)"
+
+    lyr = ds.GetLayerByName("routes")
+    f = lyr.GetNextFeature()
+    assert f.GetGeometryRef().ExportToIsoWkt() == "LINESTRING Z (6 5 7,9 8 10,12 11 13)"
+
+
+###############################################################################
+# Test SHORT_NAMES open option
+
+
+def test_ogr_gpx_SHORT_NAMES():
+
+    ds = gdal.OpenEx(
+        "data/gpx/test.gpx", gdal.OF_VECTOR, open_options=["SHORT_NAMES=YES"]
+    )
+    lyr = ds.GetLayerByName("track_points")
+    f = lyr.GetNextFeature()
+    assert f["trksegid"] == 0
+
+
+###############################################################################
+# Test N_MAX_LINKS open option
+
+
+def test_ogr_gpx_N_MAX_LINKS():
+
+    ds = gdal.OpenEx(
+        "data/gpx/test.gpx", gdal.OF_VECTOR, open_options=["N_MAX_LINKS=3"]
+    )
+    lyr = ds.GetLayerByName("track_points")
+    f = lyr.GetNextFeature()
+    assert f["link3_href"] is None

doc/source/drivers/vector/gpx.rst

@@ -85,6 +85,40 @@ available:
       Determines the number of *<link>* elements can be taken into account by
       feature.
 
+Open options
+------------
+
+.. versionadded:: 3.9
+
+The following open options are available:
+
+-  .. oo:: SHORT_NAMES
+      :choices: YES, NO
+      :default: FALSE
+
+      If ``YES``, the following fields will be renamed:
+
+      - "track_seg_id" to "trksegid"
+      - "track_seg_point_id" to  "trksegptid"
+      - "route_point_id" to "rteptid"
+
+      But note that
+      no particular processing will be done for any extension field names.
+
+-  .. oo:: ELE_AS_25D
+      :choices: YES, NO
+      :default: NO
+
+      If ``YES``, the elevation
+      element will be used to set the Z coordinates of waypoints, route points
+      and track points.
+
+-  .. oo:: N_MAX_LINKS
+      :default: 2
+
+      Determines the number of *<link>* elements can be taken into account by
+      feature.
+
 Encoding issues
 ---------------
 

ogr/ogrsf_frmts/gpx/ogr_gpx.h

@@ -137,7 +137,7 @@ class OGRGPXLayer final : public OGRLayer
   public:
     OGRGPXLayer(const char *pszFilename, const char *layerName,
                 GPXGeometryType gpxGeomType, OGRGPXDataSource *poDS,
-                bool bWriteMode);
+                bool bWriteMode, CSLConstList papszOpenOptions);
     ~OGRGPXLayer();
 
     void ResetReading() override;
@@ -223,7 +223,7 @@ class OGRGPXDataSource final : public GDALDataset
     int m_nLastTrkId = -1;
     int m_nLastTrkSegId = -1;
 
-    int Open(const char *pszFilename, int bUpdate);
+    int Open(GDALOpenInfo *poOpenInfo);
 
     int Create(const char *pszFilename, char **papszOptions);
 

ogr/ogrsf_frmts/gpx/ogrgpxdatasource.cpp

@@ -170,7 +170,7 @@ OGRGPXDataSource::ICreateLayer(const char *pszLayerName,
         return nullptr;
     }
     m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-        GetDescription(), pszLayerName, gpxGeomType, this, true));
+        GetDescription(), pszLayerName, gpxGeomType, this, true, nullptr));
 
     return m_apoLayers.back().get();
 }
@@ -447,10 +447,11 @@ static void XMLCALL dataHandlerValidateCbk(void *pUserData, const char *data,
 /*                                Open()                                */
 /************************************************************************/
 
-int OGRGPXDataSource::Open(const char *pszFilename, int bUpdateIn)
+int OGRGPXDataSource::Open(GDALOpenInfo *poOpenInfo)
 
 {
-    if (bUpdateIn)
+    const char *pszFilename = poOpenInfo->pszFilename;
+    if (poOpenInfo->eAccess == GA_Update)
     {
         CPLError(CE_Failure, CPLE_NotSupported,
                  "OGR/GPX driver does not support opening a file in "
@@ -567,15 +568,20 @@ int OGRGPXDataSource::Open(const char *pszFilename, int bUpdateIn)
         }
 
         m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-            GetDescription(), "waypoints", GPX_WPT, this, false));
+            GetDescription(), "waypoints", GPX_WPT, this, false,
+            poOpenInfo->papszOpenOptions));
         m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-            GetDescription(), "routes", GPX_ROUTE, this, false));
+            GetDescription(), "routes", GPX_ROUTE, this, false,
+            poOpenInfo->papszOpenOptions));
         m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-            GetDescription(), "tracks", GPX_TRACK, this, false));
+            GetDescription(), "tracks", GPX_TRACK, this, false,
+            poOpenInfo->papszOpenOptions));
         m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-            GetDescription(), "route_points", GPX_ROUTE_POINT, this, false));
+            GetDescription(), "route_points", GPX_ROUTE_POINT, this, false,
+            poOpenInfo->papszOpenOptions));
         m_apoLayers.emplace_back(std::make_unique<OGRGPXLayer>(
-            GetDescription(), "track_points", GPX_TRACK_POINT, this, false));
+            GetDescription(), "track_points", GPX_TRACK_POINT, this, false,
+            poOpenInfo->papszOpenOptions));
     }
 
     return m_validity == GPX_VALIDITY_VALID;

ogr/ogrsf_frmts/gpx/ogrgpxdriver.cpp

@@ -39,22 +39,32 @@
 #include "ogr_core.h"
 
 /************************************************************************/
-/*                                Open()                                */
+/*                               Identify()                             */
 /************************************************************************/
 
-static GDALDataset *OGRGPXDriverOpen(GDALOpenInfo *poOpenInfo)
+static int OGRGPXDriverIdentify(GDALOpenInfo *poOpenInfo)
 
 {
     if (poOpenInfo->eAccess == GA_Update || poOpenInfo->fpL == nullptr)
-        return nullptr;
+        return false;
+
+    return strstr(reinterpret_cast<const char *>(poOpenInfo->pabyHeader),
+                  "<gpx") != nullptr;
+}
+
+/************************************************************************/
+/*                                Open()                                */
+/************************************************************************/
+
+static GDALDataset *OGRGPXDriverOpen(GDALOpenInfo *poOpenInfo)
 
-    if (strstr(reinterpret_cast<const char *>(poOpenInfo->pabyHeader),
-               "<gpx") == nullptr)
+{
+    if (poOpenInfo->eAccess == GA_Update || !OGRGPXDriverIdentify(poOpenInfo))
         return nullptr;
 
     OGRGPXDataSource *poDS = new OGRGPXDataSource();
 
-    if (!poDS->Open(poOpenInfo->pszFilename, FALSE))
+    if (!poDS->Open(poOpenInfo))
     {
         delete poDS;
         poDS = nullptr;
@@ -122,6 +132,19 @@ void RegisterOGRGPX()
     poDriver->SetMetadataItem(GDAL_DMD_HELPTOPIC, "drivers/vector/gpx.html");
     poDriver->SetMetadataItem(GDAL_DMD_SUPPORTED_SQL_DIALECTS, "OGRSQL SQLITE");
 
+    poDriver->SetMetadataItem(
+        GDAL_DMD_OPENOPTIONLIST,
+        "<OpenOptionList>"
+        "  <Option name='N_MAX_LINKS' type='integer' default='2' "
+        "description='Maximum number of links attributes'/>"
+        "  <Option name='ELE_AS_25D' type='boolean' default='NO' "
+        "description='Whether to use the value of the ele element as the Z "
+        "ordinate of geometries'/>"
+        "  <Option name='SHORT_NAMES' type='boolean' default='NO' "
+        "description='Whether to use short field names (typically for "
+        "shapefile compatibility'/>"
+        "</OpenOptionList>");
+
     poDriver->SetMetadataItem(
         GDAL_DMD_CREATIONOPTIONLIST,
         "<CreationOptionList>"
@@ -174,6 +197,7 @@ void RegisterOGRGPX()
     poDriver->SetMetadataItem(GDAL_DCAP_VIRTUALIO, "YES");
     poDriver->SetMetadataItem(GDAL_DCAP_MULTIPLE_VECTOR_LAYERS, "YES");
 
+    poDriver->pfnIdentify = OGRGPXDriverIdentify;
     poDriver->pfnOpen = OGRGPXDriverOpen;
     poDriver->pfnCreate = OGRGPXDriverCreate;
     poDriver->pfnDelete = OGRGPXDriverDelete;

ogr/ogrsf_frmts/gpx/ogrgpxlayer.cpp

@@ -70,23 +70,29 @@ constexpr int FLD_ROUTE_NAME = 2;
 
 OGRGPXLayer::OGRGPXLayer(const char *pszFilename, const char *pszLayerName,
                          GPXGeometryType gpxGeomTypeIn,
-                         OGRGPXDataSource *poDSIn, bool bWriteModeIn)
+                         OGRGPXDataSource *poDSIn, bool bWriteModeIn,
+                         CSLConstList papszOpenOptions)
     : m_poDS(poDSIn), m_gpxGeomType(gpxGeomTypeIn), m_bWriteMode(bWriteModeIn)
 {
 #ifdef HAVE_EXPAT
     const char *gpxVersion = m_poDS->GetVersion();
 #endif
 
-    m_nMaxLinks = atoi(CPLGetConfigOption("GPX_N_MAX_LINKS", "2"));
+    m_nMaxLinks =
+        atoi(CSLFetchNameValueDef(papszOpenOptions, "N_MAX_LINKS",
+                                  CPLGetConfigOption("GPX_N_MAX_LINKS", "2")));
     if (m_nMaxLinks < 0)
         m_nMaxLinks = 2;
     if (m_nMaxLinks > 100)
         m_nMaxLinks = 100;
 
-    m_bEleAs25D = CPLTestBool(CPLGetConfigOption("GPX_ELE_AS_25D", "NO"));
+    m_bEleAs25D = CPLTestBool(
+        CSLFetchNameValueDef(papszOpenOptions, "ELE_AS_25D",
+                             CPLGetConfigOption("GPX_ELE_AS_25D", "NO")));
 
-    const bool bShortNames =
-        CPLTestBool(CPLGetConfigOption("GPX_SHORT_NAMES", "NO"));
+    const bool bShortNames = CPLTestBool(
+        CSLFetchNameValueDef(papszOpenOptions, "SHORT_NAMES",
+                             CPLGetConfigOption("GPX_SHORT_NAMES", "NO")));
 
     m_poFeatureDefn = new OGRFeatureDefn(pszLayerName);
     SetDescription(m_poFeatureDefn->GetName());
@@ -859,7 +865,7 @@ void OGRGPXLayer::endElementCbk(const char *pszName)
                   (m_gpxGeomType == GPX_TRACK &&
                    m_depthLevel == m_interestingDepthLevel + 3)))
         {
-            m_poFeature->GetGeometryRef()->setCoordinateDimension(3);
+            m_lineString->setCoordinateDimension(3);
 
             if (!m_osSubElementValue.empty())
             {