From ee6278f8dd1be7064db87be87fe31fa8943730ec Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Fri, 1 Feb 2019 23:12:48 +0100
Subject: [PATCH] ODS: allow opening tables wih empty cells with huge values of
 columns-repeated attribute at end of line (fixes #1243)

---
 .../ogr/data/testrepeatedcolatendofrow.ods    | Bin 0 -> 3230 bytes
 autotest/ogr/ogr_ods.py                       |  13 +++
 gdal/ogr/ogrsf_frmts/ods/ogr_ods.h            |   2 +
 gdal/ogr/ogrsf_frmts/ods/ogrodsdatasource.cpp | 104 ++++++++++--------
 4 files changed, 76 insertions(+), 43 deletions(-)
 create mode 100644 autotest/ogr/data/testrepeatedcolatendofrow.ods

diff --git a/autotest/ogr/data/testrepeatedcolatendofrow.ods b/autotest/ogr/data/testrepeatedcolatendofrow.ods
new file mode 100644
index 0000000000000000000000000000000000000000..ff2a6decdabea714aaa64278aed284e14316f9b8
GIT binary patch
literal 3230
zcma)82{csgAHHKrmXO9CQ7B738D$1x64^&gW8WG3l94sLvWp?RY-O?}J~9<!X)GaG
zzsQ~>vW(9%D%*dm|2fhBTmJW+_dV}@&ppq5@9#O!^LzBPsA<3e034<aLBM_W#+o(=
z0II0I?HmW(?7gie{oGuyA?X3?mGswaY0XHp8-9!ch-Qu&0KQKNSRDjWfUBJ(5*&3q
zP-`A-t})6NA0V9tz36vprJPMvLV0PMT;ZXG_g{Ir_hD0kb#<cw)hugtIzf^(#s(ZZ
ztEO(wKIgW^TA??kFc2<URG_Rh)%(1@p|jgZ9HwXOXAnWQK5~ziF2l#Wl3~zD=Ln|%
zYPYlJAja7zAIFf3b5aV>lN$^fHTRiduUF2SAf0KeYb$*@%)pokhZ}ljT&6+1C1AfU
zaCwYgaOMmOvopRO$VvM?p!&3?9&EfJ+hTTynccDS6|bqHhI<0UbZ;z5N}~u0jol1n
zW|%zJ%G<gP`l6Vmf=w3c{*&7^RO>9kPHcuFV55)6Q#WBABg>g%A>=U9w1^nDYF0?q
z&_)RgT4P;bVkgG>RaehVBO>^=Ym(ikQzZ>mY`vnm1(>tv#lQ(mM?Z9l1oKsKS|lPY
z*!MPwotAnB*tq2ii<O}8BKe&k3L8!oKKwoH|A`IS<}i9d;4n>7f~!bhRHVL!%kDB1
zlj&*t*{=d@h62+MCDpytxl31mK5X~k*R~hOjYTP>nlk_Z1BH8xx4)}B=HJ+FMRmB3
zO0g0+=addh1f&YXm|YpuFphn;h$XsSurQ=RJ?wt{`hq-I)y;>hD1Elgzq-Nyl*6W1
ziJxboCPY*^Rl)|W2e*5DIkRB0F*uSJ>=>b%_zWzgVp!VOwY!GPm0(bF%+l&dA~W=|
z#c^_>-j{HPPC%iun)lgUiVecV8PM3yEQS*)i8%wcHIS!+cW(P8#7`d~WJJY9kf61E
z@B60S_#e*|zZonu{FSQf{mLapo|}CN5_al#*KCRCZFA@`4{kXg1Kd?_#XQcBxLZj3
zR9F1yL)ER@8YDy6SiA)+<_xYjBN400=5N`Dq$}m;XW_j`yrX!R1rsPX$&KK+qeVTP
zc4kgU8vBK9i4e|6|LLq$fweCE&{pokb6L7&w*|X7-r2=`uCbPqbxHR3L!cOP-5lXn
z?s#rz@W;bTemCf3M9B~}2rfVHns%f%m&qlw*P3V<ra#geQU(y-rc?4~J&8v?#RsR7
z1ynA<HlxjpwKx(@FvC-PZy3_@nIcG`L|w9~6iZkRTXcIuGP`SPetyzCC<U+Ip*B<L
zBndZBvo3Hcx4Pw%RRQG8LC;_IS+<-eKPFACUYr~N)59S~H$zQ3t_N`YD?j6Sv&bT(
ziyhG}EPv}jU?Gc|4of=KuqfFrqt0Coo4iuu8AS8C-eZ#=W1}SE)C}!OO3V4+Sn229
zSdLl{Hog&Ee$M(3_Vo4bC)G|xGrUU>qZoMuKlC#g`iw<O#{jykg=YOrdh42(fp-38
zS>y_P;bSbwg&l&_c_E?ZnOmS@+ZL~*&a?b#(qMB@l_Q`@b?9m2;Ab&j$g}Y0n!S8^
znKNSLnos3UxpWazK3G-q-w(_31#}s0yBA7~t5%`IIA0ovM<bS&Af>1L<;^sp)fLT{
zJ`l{te(X*2H5R<reQ4ffkc0UB$X9D6`I)CS#>+1(T+2~-oVRBkEpHd<KRix&{XA{g
z1@B&vg2K6}%64+4`iehcemI5i!OhVQbGsGC0A+IXn+pAtkL37d*X&2bc6_tuc?uy&
ztBQF6M%li_FW0TS2iBy>uFvOo_rkXtzln{aEXuW(3fKL<`0qqd5u1)G5`I=gSM9g&
z+BS|vn*D{-9@HzaNUE5nK=@I-nteZno3*<W+8*QmZ_<5)ZFVL~G2JFb5L8w!l6`oK
zbv=T@mq9f~I!v{$Cm+aoC^_`n+k#Dob<$e)f~ZL3bZ|CA_qa4{qtp@^G3ao@(eu-e
zpW`HwNHrsfzJ%kjdqTRG=R<!cC#lsUu2|$~h*ECW>EF4bmBQ*TQ32VE*q)u7q?X+Y
zym;NH4MNbybg1~j+W<_w8;Cns+S%pwI>^LWb0b3EW)A)dHmhwBLO+*h)@^@jMd4(u
z^yV?+wCM7(jUzQvs;}gN1!{VP$_Dm|`P*SZtdTF(DjrD6j44ObP-orP1x^(-<&Mnu
zSk5bgeikkEDO^{fMAj^jVsf0^0KiBonyrVsx4rw%QrW&hntdC;^th$WZm%LJYbeQ|
zA+|35y7ZTV=~vcx@>5(U9cy88=}~oIdRx)}f{_)(G9hJ}BHB}q@eKb;T0r^M#VUj-
zq@Em(xL}4Fm9lt51jIxj7VM)dYq-Xjwp?6VW$_Hg%@-8H>6IGB6uJ~XGGVoUe;(P%
zCL$i!NbPNy+0u{9_j*)zS)Zt*-^aF+A*>Jokbq70>c73rEH|mu9f(aP+=?(ie*8vz
zZ5w}j3ck~JL3;pQQZ>*jcBdQjZ1A%cO@}awfE-*+`N~K0xWSc}`$R0Ju-FOJy`=f+
zWNhBOdxa=Dpc;<&!=%Q9J5D*25nbrVe#HV`+67*pUVr#ta<F|duJy?{r$c|rm(@4Q
zbvj(@XQiFjbUu3>qiKl4xLRNh4hd0<#2+iDEIMHa-&~w_BPu<dDw&1yZoMc=H9Kx8
ze3e%~?nE;k?#W$YbcZ^x6%S!2=Q5f#YA1~4Ei<dNKxF!v(s8|7miyIP$Gl7!7tW7u
zXwQT^mY)e>Jlk_Ctx&S85^~~ry!~aT`V(%j41siw(y(Ab(HZ?BOG6OpsiB7-+IQ5m
zI`;k6P#ztMIjJvt^@w{=qG=bJpNH0VT6c!ow<(TKGW~M|kESuNfWEPXKD<p1HN<+a
zgREp{J%5v9x3c9rwGQ~~_T4nGdkBwI6Z}nxduslbp0W80S*B)s_fxs$j%5w6OKkSe
zVa6BbFW!Ayg(o56U-a9d>8$8_6Qi#-JBKCOsWKVSO<QxfhVwpTnqEqlz`YRgJ}sks
ze)Q<=#eA5sy_-aVX3VK7tc?6yL@eS-wU^<rXY#pH$l4=yl`x5{CXj!^sNk&7qRR;E
zvGhqt2Akd(T`w1<K;m+-A}r{gNL`POOvdsAA)+KMxEAd?qOn9>V_Ii;J|mXOS&+!l
z&=^#|W@3!z8|l}66K*Qom$@>$2&o#cA@Jm|742E`8C<2Vx^hdc&AI;Fvvt0v;atpI
zL-ikBiaQ@+pZ56M<8-Na!91mshYv|c3pWa+O@|x3qODq_JnS5m@E|orZFN{=e{OFj
z?~4D%i5>Ag39sA;AZtzCe3sKHKRzMx?(5dd8^c=h(Ys4~&16Q8@orzrAO&at*js%&
z5wbvT_|t0IFKF}gyLUd4@9~CfqrG>0<MK~a?fE$GZuEOj5Tbeb5qmLzdabeRX@RKt
zssApba((wuN@4|S!oDqk23Fc1f%O+5|B7}nV(mvuq=5bmT>l^1!3eb<ZJUC&KT!P>
z-rseJA9w)33hcn0Xo3Abg97)hXZ*|09_$$Vk5|4Q?qL7;r<49*<$ry3KlHmpz(JR#
Z0RDx;>S@tZUIc(cl=~y4SOwp!@E;r&LB0S0

literal 0
HcmV?d00001

diff --git a/autotest/ogr/ogr_ods.py b/autotest/ogr/ogr_ods.py
index c57583252a39..c6e391a2b2df 100755
--- 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
index 44f0437a4fe0..bceddca28f37 100644
--- 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
index 5af35698d1a5..87b47379aeb9 100644
--- a/gdal/ogr/ogrsf_frmts/ods/ogrodsdatasource.cpp
+++ b/gdal/ogr/ogrsf_frmts/ods/ogrodsdatasource.cpp
@@ -786,6 +786,63 @@ 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()                         */
 /************************************************************************/
@@ -793,6 +850,8 @@ void OGRODSDataSource::endElementTable( CPL_UNUSED /* in non-DEBUG*/ const char
 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;
     }
 }