ODS: allow opening tables wih empty cells with huge values of columns…

…-repeated attribute at end of line (fixes #1243)
OSGeo · Feb 1, 2019 · 4e84cd4 · 4e84cd4
1 parent dad2b83
commit 4e84cd4
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 43 deletions.
diff --git a/autotest/ogr/data/testrepeatedcolatendofrow.ods b/autotest/ogr/data/testrepeatedcolatendofrow.ods
diff --git a/autotest/ogr/ogr_ods.py b/autotest/ogr/ogr_ods.py
@@ -454,5 +454,18 @@ def test_ogr_ods_boolean():
 
     gdal.Unlink(out_filename)
 
+###############################################################################
+# Test number-columns-repeated at end of row.
+
+def test_ogr_ods_number_columns_repeated_at_end_of_row():
 
+    drv = ogr.GetDriverByName('ODS')
+    if drv is None:
+        pytest.skip()
 
+    ds = ogr.Open('data/testrepeatedcolatendofrow.ods')
+    lyr = ds.GetLayer(0)
+    f = lyr.GetNextFeature()
+    f = lyr.GetNextFeature()
+    assert f['vbz'] == 1002
+    assert f['b'] == 0
diff --git a/gdal/ogr/ogrsf_frmts/ods/ogr_ods.h b/gdal/ogr/ogrsf_frmts/ods/ogr_ods.h
@@ -191,6 +191,8 @@ class OGRODSDataSource final: public GDALDataset
 
     void                DeleteLayer( const char *pszLayerName );
 
+    void                FillRepeatedCells(bool wasLastCell);
+
   public:
                         OGRODSDataSource();
                         virtual ~OGRODSDataSource();

diff --git a/gdal/ogr/ogrsf_frmts/ods/ogrodsdatasource.cpp b/gdal/ogr/ogrsf_frmts/ods/ogrodsdatasource.cpp
@@ -786,13 +786,72 @@ void OGRODSDataSource::endElementTable( CPL_UNUSED /* in non-DEBUG*/ const char
     }
 }
 
+/************************************************************************/
+/*                           FillRepeatedCells()                        */
+/************************************************************************/
+
+void OGRODSDataSource::FillRepeatedCells(bool wasLastCell)
+{
+    if( wasLastCell && osValue.empty() && osFormula.empty() )
+    {
+        nCellsRepeated = 0;
+        return;
+    }
+
+    if (nCellsRepeated < 0 || nCellsRepeated > 10000)
+    {
+        CPLError(CE_Failure, CPLE_NotSupported,
+                    "Invalid value for number-columns-repeated = %d",
+                    nCellsRepeated);
+        bEndTableParsing = true;
+        nCellsRepeated = 0;
+        return;
+    }
+    const int nFields = nCellsRepeated +
+        (poCurLayer != nullptr ?
+            poCurLayer->GetLayerDefn()->GetFieldCount() : 0);
+    if( nFields > 0 && nRowsRepeated > 100000 / nFields )
+    {
+        CPLError(CE_Failure, CPLE_AppDefined,
+                    "Too big gap with previous valid row");
+        bEndTableParsing = true;
+        nCellsRepeated = 0;
+        return;
+    }
+
+    const size_t nCellMemSize =
+        (!osValue.empty()) ? osValue.size() : osFormula.size();
+    if( nCellMemSize > static_cast<size_t>(10 * 1024 * 1024) /
+            (std::max(nCellsRepeated, 1) * nRowsRepeated) )
+    {
+        CPLError(CE_Failure, CPLE_NotSupported,
+                    "Too much memory for row/cell repetition");
+        bEndTableParsing = true;
+        nCellsRepeated = 0;
+        return;
+    }
+    for(int i = 0; i < nCellsRepeated; i++)
+    {
+        if( !osValue.empty() )
+            apoCurLineValues.push_back(osValue);
+        else
+            apoCurLineValues.push_back(osFormula);
+        apoCurLineTypes.push_back(osValueType);
+    }
+
+    nCurCol += nCellsRepeated;
+    nCellsRepeated = 0;
+}
+
 /************************************************************************/
 /*                            startElementRow()                         */
 /************************************************************************/
 
 void OGRODSDataSource::startElementRow(const char *pszNameIn,
                                        const char **ppszAttr)
 {
+    FillRepeatedCells(false);
+
     if (strcmp(pszNameIn, "table:table-cell") == 0)
     {
         PushState(STATE_CELL);
@@ -838,38 +897,6 @@ void OGRODSDataSource::startElementRow(const char *pszNameIn,
 
         nCellsRepeated = atoi(
             GetAttributeValue(ppszAttr, "table:number-columns-repeated", "1"));
-        if (nCellsRepeated < 0 || nCellsRepeated > 10000)
-        {
-            CPLError(CE_Failure, CPLE_NotSupported,
-                     "Invalid value for number-columns-repeated = %d",
-                     nCellsRepeated);
-            bEndTableParsing = true;
-            nCellsRepeated = 0;
-            return;
-        }
-        const int nFields = nCellsRepeated +
-            (poCurLayer != nullptr ?
-                poCurLayer->GetLayerDefn()->GetFieldCount() : 0);
-        if( nFields > 0 && nRowsRepeated > 100000 / nFields )
-        {
-            CPLError(CE_Failure, CPLE_AppDefined,
-                     "Too big gap with previous valid row");
-            bEndTableParsing = true;
-            nCellsRepeated = 0;
-            return;
-        }
-
-        const size_t nCellMemSize =
-            (!osValue.empty()) ? osValue.size() : osFormula.size();
-        if( nCellMemSize > static_cast<size_t>(10 * 1024 * 1024) /
-                (std::max(nCellsRepeated, 1) * nRowsRepeated) )
-        {
-            CPLError(CE_Failure, CPLE_NotSupported,
-                     "Too much memory for row/cell repetition");
-            bEndTableParsing = true;
-            nCellsRepeated = 0;
-            return;
-        }
     }
     else if (strcmp(pszNameIn, "table:covered-table-cell") == 0)
     {
@@ -891,6 +918,8 @@ void OGRODSDataSource::endElementRow( CPL_UNUSED /*in non-DEBUG*/ const char * p
     {
         CPLAssert(strcmp(pszNameIn, "table:table-row") == 0);
 
+        FillRepeatedCells(true);
+
         /* Remove blank columns at the right to defer type evaluation */
         /* until necessary */
         size_t i = apoCurLineTypes.size();
@@ -1141,17 +1170,6 @@ void OGRODSDataSource::endElementCell( CPL_UNUSED /*in non-DEBUG*/ const char *
     if (stateStack[nStackDepth].nBeginDepth == nDepth)
     {
         CPLAssert(strcmp(pszNameIn, "table:table-cell") == 0);
-
-        for(int i = 0; i < nCellsRepeated; i++)
-        {
-            if( !osValue.empty() )
-                apoCurLineValues.push_back(osValue);
-            else
-                apoCurLineValues.push_back(osFormula);
-            apoCurLineTypes.push_back(osValueType);
-        }
-
-        nCurCol += nCellsRepeated;
     }
 }