Merge pull request #13521 from dbaston/check-coverage-multiple-input

GDALGeosNonStreamingAlgorithmDataset: Avoid crash with multiple input layers

Author: rouault

apps/gdalalg_vector_geom.cpp

@@ -138,22 +138,42 @@ GDALGeosNonStreamingAlgorithmDataset::GDALGeosNonStreamingAlgorithmDataset()
 
 GDALGeosNonStreamingAlgorithmDataset::~GDALGeosNonStreamingAlgorithmDataset()
 {
+    Cleanup();
+    if (m_poGeosContext != nullptr)
+    {
+        finishGEOS_r(m_poGeosContext);
+    }
+}
+
+void GDALGeosNonStreamingAlgorithmDataset::Cleanup()
+{
+    m_apoFeatures.clear();
+
     if (m_poGeosContext != nullptr)
     {
         for (auto &poGeom : m_apoGeosInputs)
         {
             GEOSGeom_destroy_r(m_poGeosContext, poGeom);
         }
+        m_apoGeosInputs.clear();
 
-        GEOSGeom_destroy_r(m_poGeosContext, m_poGeosResultAsCollection);
+        if (m_poGeosContext != nullptr)
+        {
+            GEOSGeom_destroy_r(m_poGeosContext, m_poGeosResultAsCollection);
+            m_poGeosResultAsCollection = nullptr;
+        }
 
         for (size_t i = 0; i < m_nGeosResultSize; i++)
         {
             GEOSGeom_destroy_r(m_poGeosContext, m_papoGeosResults[i]);
         }
+        m_nGeosResultSize = 0;
 
-        GEOSFree_r(m_poGeosContext, m_papoGeosResults);
-        finishGEOS_r(m_poGeosContext);
+        if (m_papoGeosResults != nullptr)
+        {
+            GEOSFree_r(m_poGeosContext, m_papoGeosResults);
+            m_papoGeosResults = nullptr;
+        }
     }
 }
 
@@ -314,6 +334,8 @@ bool GDALGeosNonStreamingAlgorithmDataset::ConvertOutputsFromGeos(
 bool GDALGeosNonStreamingAlgorithmDataset::Process(OGRLayer &srcLayer,
                                                    OGRLayer &dstLayer)
 {
+    Cleanup();
+
     bool sameDefn = dstLayer.GetLayerDefn()->IsSame(srcLayer.GetLayerDefn());
 
     if (!ConvertInputsToGeos(srcLayer, dstLayer, sameDefn))

apps/gdalalg_vector_geom.h

@@ -207,11 +207,6 @@ class GDALGeosNonStreamingAlgorithmDataset
 
     CPL_DISALLOW_COPY_ASSIGN(GDALGeosNonStreamingAlgorithmDataset)
 
-    bool ConvertInputsToGeos(OGRLayer &srcLayer, OGRLayer &dstLayer,
-                             bool sameDefn);
-
-    bool ConvertOutputsFromGeos(OGRLayer &dstLayer);
-
     bool Process(OGRLayer &srcLayer, OGRLayer &dstLayer) override;
 
     virtual bool ProcessGeos() = 0;
@@ -234,6 +229,13 @@ class GDALGeosNonStreamingAlgorithmDataset
     GEOSGeometry **m_papoGeosResults{nullptr};
 
   private:
+    bool ConvertInputsToGeos(OGRLayer &srcLayer, OGRLayer &dstLayer,
+                             bool sameDefn);
+
+    bool ConvertOutputsFromGeos(OGRLayer &dstLayer);
+
+    void Cleanup();
+
     std::vector<std::unique_ptr<OGRFeature>> m_apoFeatures{};
     unsigned int m_nGeosResultSize{0};
     int m_sourceGeometryField{0};

autotest/utilities/test_gdalalg_vector_check_coverage.py

@@ -80,7 +80,8 @@ def test_gdalalg_vector_check_coverage_invalid_layer(alg, three_rectangles):
         assert alg.Run()
 
 
-def test_gdalalg_vector_check_geometry_two_layers(alg, three_rectangles):
+@pytest.mark.parametrize("input_layers", (1, 2))
+def test_gdalalg_vector_check_coverage_two_layers(alg, three_rectangles, input_layers):
 
     poly_ds = gdal.OpenEx("../ogr/data/poly.shp", gdal.OF_VECTOR)
 
@@ -89,18 +90,32 @@ def test_gdalalg_vector_check_geometry_two_layers(alg, three_rectangles):
     ds.CopyLayer(three_rectangles.GetLayer(0), "poly2")
 
     alg["input"] = ds
-    alg["input-layer"] = "poly2"
     alg["output"] = ""
     alg["output-format"] = "stream"
+    if input_layers == 1:
+        alg["input-layer"] = "poly2"
+    else:
+        alg["input-layer"] = ["poly2", "poly1"]
 
     assert alg.Run()
 
     dst_ds = alg["output"].GetDataset()
-    dst_lyr = dst_ds.GetLayer(0)
+    assert dst_ds.GetLayerCount() == input_layers
+
+    for i in range(input_layers):
+        dst_lyr = dst_ds.GetLayer(i)
+
+        errors = 2 if i == 0 else 0
+
+        if input_layers == 1:
+            assert dst_lyr.GetName() == "invalid_edge"
+        else:
+            assert dst_lyr.GetName() == f"invalid_edge_{alg['input-layer'][i]}"
+
+        assert dst_lyr.GetFeatureCount() == errors
 
-    assert dst_lyr.GetFeatureCount() == 2
-    for f in dst_lyr:
-        assert f.GetGeometryRef().GetGeometryType() == ogr.wkbMultiLineString
+        for f in dst_lyr:
+            assert f.GetGeometryRef().GetGeometryType() == ogr.wkbMultiLineString
 
     assert alg.Finalize()