Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#5842 closed enhancement (fixed)

Change how OpenFileGDB handles the total number of records (from .gdbtablx) being lower than the number of valid records (from .gdbtable)

Reported by: hchen Owned by: warmerdam
Priority: normal Milestone: 2.0.0
Component: OGR_SF Version: unspecified
Severity: normal Keywords: openfilegdb
Cc:

Description

Potentially change OpenFileGDB driver, which throws an error when the total record count (from the .gdbtablx file) is exceeded by the valid record count (from the .gdbtable file) - see https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp#L634. This is reasonable, but results in the feature count being returned as 0 for that layer by OGR_L_GetFeatureCount(). The FileGDB driver will return the (non-zero) feature count if the layer has features in this case.

Attached is a modified version of filegdbtable.cpp with a (naive?) alternative - see line 630.

Attachments (1)

filegdbtable.cpp (84.3 KB) - added by hchen 7 years ago.
gdal/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp, changed at line 630 (-632)

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by hchen

Attachment: filegdbtable.cpp added

gdal/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp, changed at line 630 (-632)

comment:1 Changed 7 years ago by Even Rouault

Hum, I'd be curious to have access to such a dataset, and compare with the behaviour of the FileGDB driver. Either the dataset is buggy, or there's something that hasn't been understood in the reverse engineering. I'm afraid that this change will prevent the last records (beyond nTotalRecordCount) from being accessed

comment:2 Changed 7 years ago by Even Rouault

Note: sample data privately provided.

It looks like there's some corruption in the .gdbtablx that advertizes only 14 features in the header, at offset 8, (well if I guessed correctly the meaning of that field), but the hex dump shows that the first 22 entries have non zero offsets, and they actually point to correct offsets in the .gdbtable. And the .gdbtable effectively contain 22 records.

Harriet's suggested change indeed workarounds the issue, but I'm afraid it could be abused with crafted files to trigger crashes in the driver, particularly if the .gdbtablx contains bit arrays in the trailer section to indicate if entire blocks of 1024 features are present or omitted in the .gdbtablx (see https://github.com/rouault/dump_gdbtable/wiki/FGDB-Spec). Or it could cause the trailer bytes of the .gdbtablx to be considered as valid offsets to the .gdbtable. So a proper fix would require more validations at a few extra places. Or perhaps do a "dry run" step when nValidRecordCount > nTotalRecordCount to check if we can safely assign nTotalRecordCount = nValidRecordCount.

comment:3 Changed 7 years ago by Even Rouault

Component: defaultOGR_SF
Keywords: openfilegdb added
Milestone: 2.0
Resolution: fixed
Status: newclosed

trunk r28549 "OpenFileGDB: try to deal more gracefully with inconsistent nValidRecordCount vs nTotalRecordCount values (#5842)"

comment:4 Changed 6 years ago by Even Rouault

Milestone: 2.02.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.