From e1f240408258b7505c80582721509110226848dc Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 3 Mar 2024 00:14:39 +0100 Subject: [PATCH 1/2] FlatGeobuf writing: in SPATIAL_INDEX=NO mode, deal with empty geometries as if there were null - this fixes the crash when writing an empty polygon. Fixes #11419 - this avoids a reading error when writing an empty line --- autotest/ogr/ogr_flatgeobuf.py | 21 +++++++++++++++++++ .../flatgeobuf/ogrflatgeobuflayer.cpp | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/autotest/ogr/ogr_flatgeobuf.py b/autotest/ogr/ogr_flatgeobuf.py index 5d770f64501a..25682f230ba7 100644 --- a/autotest/ogr/ogr_flatgeobuf.py +++ b/autotest/ogr/ogr_flatgeobuf.py @@ -1612,3 +1612,24 @@ def test_ogr_flatgeobuf_arrow_stream_numpy_datetime_as_string(tmp_vsimem): assert batch["datetime"][1] == b"2022-05-31T12:34:56.789Z" assert batch["datetime"][2] == b"2022-05-31T12:34:56" assert batch["datetime"][3] == b"2022-05-31T12:34:56+12:30" + + +def test_ogr_flatgeobuf_write_empty_geometries_no_spatial_index(tmp_vsimem): + # Verify empty geom handling without spatial index + out_filename = str(tmp_vsimem / "out.fgb") + with ogr.GetDriverByName("FlatGeobuf").CreateDataSource(out_filename) as ds: + lyr = ds.CreateLayer( + "test", geom_type=ogr.wkbPolygon, options=["SPATIAL_INDEX=NO"] + ) + + f = ogr.Feature(lyr.GetLayerDefn()) + f.SetGeometry(ogr.CreateGeometryFromWkt("POLYGON EMPTY")) + lyr.CreateFeature(f) + f = ogr.Feature(lyr.GetLayerDefn()) + lyr.CreateFeature(f) + + with gdal.OpenEx(out_filename) as ds: + lyr = ds.GetLayer(0) + f = lyr.GetNextFeature() + g = f.GetGeometryRef() + assert g is None diff --git a/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp b/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp index 22767210a8df..3fb69666526f 100644 --- a/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp +++ b/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp @@ -2295,7 +2295,7 @@ OGRErr OGRFlatGeobufLayer::ICreateFeature(OGRFeature *poNewFeature) // the size of the FlatBuffer, but WKB might be a good approximation. // Takes an extra security margin of 10% flatbuffers::Offset geometryOffset = 0; - if (ogrGeometry != nullptr) + if (ogrGeometry && !ogrGeometry->IsEmpty()) { const auto nWKBSize = ogrGeometry->WkbSize(); if (nWKBSize > feature_max_buffer_size - nWKBSize / 10) From c59ee2217869fb3ee5af5be28a48c653ac07f576 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 3 Dec 2024 00:17:40 +0100 Subject: [PATCH 2/2] FlatGeobuf writing: in SPATIAL_INDEX=NO mode, accept creating a file without features --- autotest/ogr/ogr_flatgeobuf.py | 17 +++++++++++ .../flatgeobuf/ogrflatgeobuflayer.cpp | 28 +++++++++++++------ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/autotest/ogr/ogr_flatgeobuf.py b/autotest/ogr/ogr_flatgeobuf.py index 25682f230ba7..18c9cd225a40 100644 --- a/autotest/ogr/ogr_flatgeobuf.py +++ b/autotest/ogr/ogr_flatgeobuf.py @@ -1633,3 +1633,20 @@ def test_ogr_flatgeobuf_write_empty_geometries_no_spatial_index(tmp_vsimem): f = lyr.GetNextFeature() g = f.GetGeometryRef() assert g is None + + +def test_ogr_flatgeobuf_write_empty_file_no_spatial_index(tmp_vsimem): + # Verify empty geom handling without spatial index + out_filename = str(tmp_vsimem / "out.fgb") + with ogr.GetDriverByName("FlatGeobuf").CreateDataSource(out_filename) as ds: + srs = osr.SpatialReference() + srs.ImportFromEPSG(32631) + ds.CreateLayer( + "test", geom_type=ogr.wkbPolygon, srs=srs, options=["SPATIAL_INDEX=NO"] + ) + + with gdal.OpenEx(out_filename) as ds: + lyr = ds.GetLayer(0) + assert lyr.GetFeatureCount() == 0 + assert lyr.GetExtent(can_return_null=True) is None + assert lyr.GetSpatialRef().GetAuthorityCode(None) == "32631" diff --git a/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp b/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp index 3fb69666526f..0889526680dc 100644 --- a/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp +++ b/ogr/ogrsf_frmts/flatgeobuf/ogrflatgeobuflayer.cpp @@ -27,6 +27,7 @@ #include "geometrywriter.h" #include +#include #include #include #include @@ -71,7 +72,9 @@ OGRFlatGeobufLayer::OGRFlatGeobufLayer(const Header *poHeader, GByte *headerBuf, m_hasM = m_poHeader->has_m(); m_hasT = m_poHeader->has_t(); const auto envelope = m_poHeader->envelope(); - if (envelope && envelope->size() == 4) + if (envelope && envelope->size() == 4 && std::isfinite((*envelope)[0]) && + std::isfinite((*envelope)[1]) && std::isfinite((*envelope)[2]) && + std::isfinite((*envelope)[3])) { m_sExtent.MinX = (*envelope)[0]; m_sExtent.MinY = (*envelope)[1]; @@ -536,8 +539,7 @@ bool OGRFlatGeobufLayer::CreateFinalFile() // no spatial index requested, we are (almost) done if (!m_bCreateSpatialIndexAtClose) { - if (m_poFpWrite == nullptr || m_featuresCount == 0 || - !SupportsSeekWhileWriting(m_osFilename)) + if (m_poFpWrite == nullptr || !SupportsSeekWhileWriting(m_osFilename)) { return true; } @@ -546,14 +548,24 @@ bool OGRFlatGeobufLayer::CreateFinalFile() VSIFSeekL(m_poFpWrite, 0, SEEK_SET); m_writeOffset = 0; std::vector extentVector; - extentVector.push_back(m_sExtent.MinX); - extentVector.push_back(m_sExtent.MinY); - extentVector.push_back(m_sExtent.MaxX); - extentVector.push_back(m_sExtent.MaxY); + if (!m_sExtent.IsInit()) + { + extentVector.resize(4, std::numeric_limits::quiet_NaN()); + } + else + { + extentVector.push_back(m_sExtent.MinX); + extentVector.push_back(m_sExtent.MinY); + extentVector.push_back(m_sExtent.MaxX); + extentVector.push_back(m_sExtent.MaxY); + } writeHeader(m_poFpWrite, m_featuresCount, &extentVector); // Sanity check to verify that the dummy header and the real header // have the same size. - CPLAssert(m_writeOffset == m_offsetAfterHeader); + if (m_featuresCount) + { + CPLAssert(m_writeOffset == m_offsetAfterHeader); + } CPL_IGNORE_RET_VAL(m_writeOffset); // otherwise checkers might tell the // member is not used return true;