Skip to content

Commit

Permalink
FlatGeobuf: fix reading of conformant single-part MultiLineString (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
bjornharrtell authored and rouault committed Sep 13, 2024
1 parent ca7537f commit 7eb028e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 36 deletions.
Binary file not shown.
58 changes: 35 additions & 23 deletions autotest/ogr/ogr_flatgeobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@

pytestmark = pytest.mark.require_driver("FlatGeobuf")

###############################################################################
@pytest.fixture(autouse=True, scope="module")
def module_disable_exceptions():
with gdaltest.disable_exceptions():
yield


###############################################################################
@pytest.fixture(autouse=True, scope="module")
Expand Down Expand Up @@ -648,11 +642,10 @@ def test_ogr_flatgeobuf_huge_number_of_columns():
lyr.CreateField(ogr.FieldDefn("col%d" % i, ogr.OFTInteger))
== ogr.OGRERR_NONE
), i
with gdal.quiet_errors():
assert (
lyr.CreateField(ogr.FieldDefn("col65536", ogr.OFTInteger))
== ogr.OGRERR_FAILURE
)
with pytest.raises(
Exception, match="Cannot create features with more than 65536 columns"
):
lyr.CreateField(ogr.FieldDefn("col65536", ogr.OFTInteger))
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (0 0)"))
for i in range(65536):
Expand Down Expand Up @@ -765,7 +758,8 @@ def test_ogr_flatgeobuf_editing():

assert lyr.TestCapability(ogr.OLCDeleteFeature) == 1
assert lyr.DeleteFeature(1) == 0
assert lyr.DeleteFeature(1) == ogr.OGRERR_NON_EXISTING_FEATURE
with pytest.raises(Exception, match="Non existing feature"):
lyr.DeleteFeature(1)
assert lyr.TestCapability(ogr.OLCReorderFields) == 1
# assert lyr.ReorderFields([0, 1]) == 0
assert lyr.DeleteField(1) == 0
Expand Down Expand Up @@ -797,8 +791,8 @@ def test_ogr_flatgeobuf_editing():

f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (1 1)"))
with gdal.quiet_errors():
assert lyr.CreateFeature(f) != ogr.OGRERR_NONE
with pytest.raises(Exception, match="not supported on read-only layer"):
lyr.CreateFeature(f)

ogr.GetDriverByName("FlatGeobuf").DeleteDataSource("/vsimem/test.fgb")
assert not gdal.VSIStatL("/vsimem/test.fgb")
Expand Down Expand Up @@ -871,8 +865,27 @@ def test_ogr_flatgeobuf_read_invalid_geometries(filename):
with gdal.quiet_errors():
ds = gdal.OpenEx(filename)
lyr = ds.GetLayer(0)
for f in lyr:
pass
with pytest.raises(Exception, match="Fatal error parsing feature"):
for f in lyr:
pass


###############################################################################
# Check that we can read multilinestrings with a single part, without the
# "ends" array (cf https://github.com/OSGeo/gdal/issues/10774)


@pytest.mark.parametrize(
"filename",
[
"data/flatgeobuf/test_ogr_flatgeobuf_singlepart_mls_new.fgb",
],
)
def test_ogr_flatgeobuf_read_singlepart_mls_new(filename):
with gdal.OpenEx(filename) as ds:
lyr = ds.GetLayer(0)
f = lyr.GetNextFeature()
ogrtest.check_feature_geometry(f, "MULTILINESTRING ((0 0,1 1))")


###############################################################################
Expand Down Expand Up @@ -959,8 +972,8 @@ def test_ogr_flatgeobuf_coordinate_epoch_custom_wkt():
def test_ogr_flatgeobuf_invalid_output_filename():

ds = ogr.GetDriverByName("FlatGeobuf").CreateDataSource("/i_do/not_exist/my.fgb")
with gdal.quiet_errors():
assert ds.CreateLayer("foo") is None
with pytest.raises(Exception, match="Failed to create"):
ds.CreateLayer("foo")


###############################################################################
Expand Down Expand Up @@ -1208,12 +1221,11 @@ def test_ogr_flatgeobuf_issue_7401():
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (0 0)"))
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
lyr.CreateFeature(f)
with pytest.raises(
Exception, match="NULL geometry not supported with spatial index"
):
lyr.CreateFeature(f)
ds = None
assert (
gdal.GetLastErrorMsg()
== "ICreateFeature: NULL geometry not supported with spatial index"
)

ogr.GetDriverByName("FlatGeobuf").DeleteDataSource("/vsimem/test.fgb")
assert not gdal.VSIStatL("/vsimem/test.fgb")
Expand Down
35 changes: 22 additions & 13 deletions ogr/ogrsf_frmts/flatgeobuf/geometryreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,31 @@ OGRMultiPoint *GeometryReader::readMultiPoint()

OGRMultiLineString *GeometryReader::readMultiLineString()
{
const auto pEnds = m_geometry->ends();
if (pEnds == nullptr)
return CPLErrorInvalidPointer("MultiLineString ends data");
const auto ends = m_geometry->ends();
auto mls = std::make_unique<OGRMultiLineString>();
m_offset = 0;
for (uint32_t i = 0; i < pEnds->size(); i++)
if (ends == nullptr || ends->size() < 2)
{
const auto e = pEnds->Get(i);
if (e < m_offset)
return CPLErrorInvalidLength("MultiLineString");
m_length = e - m_offset;
const auto ls = readSimpleCurve<OGRLineString>();
if (ls == nullptr)
m_length = m_length / 2;
const auto part = readSimpleCurve<OGRLineString>();
if (part == nullptr)
return nullptr;
mls->addGeometryDirectly(ls);
m_offset = e;
mls->addGeometryDirectly(part);
}
else
{
m_offset = 0;
for (uint32_t i = 0; i < ends->size(); i++)
{
const auto e = ends->Get(i);
if (e < m_offset)
return CPLErrorInvalidLength("MultiLineString");
m_length = e - m_offset;
const auto ls = readSimpleCurve<OGRLineString>();
if (ls == nullptr)
return nullptr;
mls->addGeometryDirectly(ls);
m_offset = e;
}
}
return mls.release();
}
Expand Down

0 comments on commit 7eb028e

Please sign in to comment.