From af485bb975fb00a9572e1cf2a2d72cdfe834a60d Mon Sep 17 00:00:00 2001 From: Aron Budea Date: Wed, 18 Feb 2026 03:37:36 +1030 Subject: [PATCH] Revert "tdf#170515 sc: formula with invalid parameters saved into XLSX" Invalid XML gets exported with problematic matrix formula, eg. fdo73235-1.ods as found by crashtesting. Further problem this error exposed is that in certain cases normal formula or error results (eg. #VALUE!) turn into parameter errors (Err:504) in Calc, which are then discarded by the previous change. Let's revert the change until these issues are resolved. Affected files: https://wiki.documentfoundation.org/index.php?title=Development/Crashtesting&oldid=869226 (under sax_fastparser::FastSaxSerializer::endFastElement) If the change is later reapplied: - handle matrix formula case as well, - keep #VALUE! in the formula, don't remove formula completely. This reverts commit bebe7a0b270506b4d1a32bd99740865fbfbea3a3. Change-Id: I19aca20cc447871d89f462af874e46993ed6f4ce Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199635 Tested-by: Jenkins Reviewed-by: Aron Budea --- .../data/ods/tdf170515_invalid_parameters.ods | Bin 9356 -> 0 bytes sc/qa/unit/subsequent_export_test2.cxx | 26 ------------------ sc/source/filter/excel/xestream.cxx | 5 +--- sc/source/filter/excel/xetable.cxx | 4 +-- sc/source/filter/excel/xltools.cxx | 10 ------- sc/source/filter/inc/xestream.hxx | 3 +- sc/source/filter/inc/xltools.hxx | 2 -- 7 files changed, 4 insertions(+), 46 deletions(-) delete mode 100644 sc/qa/unit/data/ods/tdf170515_invalid_parameters.ods diff --git a/sc/qa/unit/data/ods/tdf170515_invalid_parameters.ods b/sc/qa/unit/data/ods/tdf170515_invalid_parameters.ods deleted file mode 100644 index 35ac902111e605c6e4eaa752f3f22456744b5885..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 9356 zcmb_?bzB_F()QvG!6jIL1b1g~cZWqoaF?*S!{Qn|K!8A4+#$FG4-g>1T?2&R5Zu1y z+&A}}d*6HRAK&lo-Ja>0nyT)8s(ZStrqq=X5D5SPQ~=;-fT&TJgJ3ip004N*uv-8} zTSrSbZx>5b7Z(Rxb5l24XD4#(-OUYx=&U$95|S&!P_td` zC*E*9B=(a`yycGgWL=Gd(x17L8clnWw^PZ8Tj|0f=s3 z)e!8;+g~4e)jk2q7k+ib96vU@9En-D`Es?r`oIXX;+JaUb71b{KGAo)LYwmH9oK={ z&sL!K!}dMf*aPW$RlSF_gZStN>+3h$^(Rz1a#iX|D5&)pHseTe000C50Qd)dzg6;2 z>|li_uq@^5WMymZ?)rx^z+CKq4>mgNX|S8OgC&^F%h6%UXxI5EKi)$?V4q|cA?w#E zQ95868BqSPG9CW4!S~=Ef2Lw%MVi`HBNxWuQIo))?GBLrpvOCKNW=OUmzVFTA_$g1 zRn8N=&0XcxJgx5#wF1{yS@Jmt`O=feas$BZFRH5U&!a16=(>mB%C-_%FS1@5hhU(D z)|r(=rYBPjrx6eq=vguJ^(DzEWo~>yWi4A&Dp5>O_Ys_P&0!Z&LuH6(w0z3`>#n&- z^yvXFMcGb+{35SAZRS>FvW}*fPu82g7@gLY!=jrEZBiB@3^h$Gdvg5_zX7~KLODv+ zYIR+&E}kPy=V(DP``p!vOge~=t{xW%D%i1U9){$sleXzcjbKgLBWStU56QdJQwbrS z*3{AeVs-3wtZ#(zZN$ng$cay+B%vWP4m;JNvX8W)-&z4Cpw76Z+oAp@@`G-Ep3ynv ztkyczT-P)Kd5$(knvX|@bHiBYcn3mnQ>iEDQ8RINnU8|gZGak47O;dr@yb~K~=ME7%2mZzEQ%;P!;n!Sx8%O+kk z;wcYJ0U&r@rGZMEF2l0Z$V5N4IHm&wJKK5u(oYE3#4D)Z`4$Cx!XgpVvsMMNbx5K5 z1C6yAqx2xFZ6mE&-Wi5&r`Qh#O+L_}d2Ha%y02b>-aV*_@5V@j<4AxrRP@+E8EYyE zuAYKdaGUj)(7|F^dR0f)XRA&w$XCeJ3Ce$YQi?jRt-*RQ6G^KN8WKZR>E@eLclTN@dn2+bYekWo6}9 zBrV3@t-!^(CunBVqu${TF4p`a5R9#y81&RCulZ?m$OmN}b3miDh5@oNz|&BjY(9CK zSLb_(AK~ehV7fWl5FfBUIB#DSTN+^ZE_lRQOf7SwMO}_zS5bM8ev98vF@}arCj8vp zp26V5v)c`S!>ZXUe7^;KC9J8mk6O)8Ck3tc=XdunUv$!znQO9w6=|}p@0!zC^!kyk zf5lYO=Y17xdpj~V_SJ7a5(~XrNP%V3vPn!AYdYHp<~jV;Up8Tdtw` zDhq*IM`P>baNXMaJmGBE7D!r?X$EcGAtG*2yLg-CQy@D=C6z2(%rxz%=x+p;5WOju zgCokm>{rYo}0$GIAG*R z`h*;5tDSSzBErh&ld=!K9@1brFfjT=W@=1bUWQr4u$`aksD; zmqS5Gwh$`!T;A42^Mdv|vv)s;EO7jl>#w2|ixb3FxaPMGsO;hq+Fv=hQDwSr^R=H( z-=sbbt!vI!#ikIu${)iK;Cw^y+$l#q9Z?j}tu0Jm$ag0u*7s1itS(j%l?R0vKR8j8 zGfhc^w`##3U}SUQKk|Q|YYY+duVZnxfnTMqJS`ZaceyAaL@PvDsn|I)s|6}VyDs!v zaB9eN)WrNUhAAS$`eLmDdl@2l)h1@h^^+H30c$n{weUpE<+^Fu;{J8LmHBQ z>CQ^^do`Xr4RoJ-7bFoXrXRkPAxtx`5w_nqx*!H8kw2jfBx4FI_4hc;j_0!-mOn?r z#y`k(0e|`7IKS)fDlp>-uv3S6?rN>w7{r1p#&B%<=a$_~c!oT7eo6Fz7_MpNKd@A;Oi6DhNj=T% z5^YvA8Vv=LI(-v(!&<>lE=gfo(>auI%gW1_T|ZY+`sL;-mqd{(^X0pyHY!q2!TxOID6?Jz`p^=e zd=8SyfN~BNq6>_ksvm=(5Y$iR;ndN752DX5agtW9MeIm&!ah8h#nu*8`PzQ}fMm)V zC`-PL{qD0WP`y2>x6yITS?mYa3V*`Xr#UT7a_?W083I-%*Z0XRQ?&3?;M&Pxp@Uc6 zlJA+a_qj199XQnMimwb8#_ypF)yFUlef>Y^Z|dGlz0)WU8?>7a+|~rq$lqJ`tkUm zKo-6}FtL0)9BUBhyEaYFyu>-nV&+9AoXPLMh%YB`;!tWJ17VILZEflyM6?IgNK02e z2X*(qX*rU2XKn8J`9?ZEExQXT^Qx{V#8Jt58-qvEWtu>`j+FAl>t%vAKn*9^SZ2sK zGh8eb9kiTAq#o)|QXvZ`LLai_g&A536(lbNJbbqWfEZu<6w}OngJ)2N8^6^W4by*v zkJao|_)gZ{b(kX?I6;OTAV@@cmNN9`1pZxSHu>&lA|Kpp78B1<@9#$qnk)siwJs+%T`VXxY*=QxpzB8rHVt?a~% z!mX78IgQ(l#O27~se_`SxAxfw28-?w9WjK?NxM2o-`PoxSc!Dtc+dBldv|4ps%{<( zUCzWFce6x$S?e~i?dOH~@7?U*83SNTH#b`+>pym=`-b-5QX%5|`}^p4lQFll_WhrG z=gJ}V>)&+Q#+^E4Vm~v}DWgKqXL1qiiJ6y(pw_wEm83|LC*?}I(~8u$m#3UZIFuYQ z!isE6XU-po;9g);`1ZCJW0sfooDSutKio*rIpM`cofi1W~EaJTwv zM3LHR5_oGQzX!;v9Prpi4%X_Qy6;;JIcP}k$XE_}&z9?2LdyI3hZcWoRCs8PRHlEV z@=y4ZvrDgV!YZh`JN-0HmbbmN{HHqC@uCXSu$^@XYp9=PX7bSHIJVLmdmu}uMTAts zhWPx}D-D(mW9?9n5fmKyG2OCq`-_ISeVOIY13{)1->Pfd2DkH?c5q)dm){k4pj;8S z5UY6!e)e2`t2QG+MUkE39Jkz`12ks#zH9SnV7xUA>Xu4~J*K6ss_h=U7FAq84KzJf zLF|v>dzr|!t}GzFso5fRrhh~)Gd`M#)^CBIyaHUZKhA%cbceXyoffQ*dMq$WaWc-13U^2>J;~$)lu4I|gm+U2;6q%aYRCxD1$#Si?nS zZPy{q6Q}~r3SZ`VwrSDEa!ODqlf`wY(DdX)B|>9`>RB4FruqCVnH_XjWBkkL^7YOdCetrm;xE5Og#&nag38W8-B7Nj_hQD z^1Ce72GgYY`G&Xk*RJbK$pndAow3TrGF(-Llj4c;ZO=JA2ZgE5;nneABgcn=H!F*z z`4~uQKx;E>r43>H8{cMqkMfO68D^pBJlyvf+)3EG0m|f*MaRtqvL>~~m!*RSJIv^bN7fJ*mJ^wT zcrcB3SCO`HbbmOBqc&nZ9-SJP*NT6gW?O>NB1Mf8Xyb{~DmPH{ae66OzMV22o5-=z zY*Fr+6(|D&Hr@l#}~X=a|k1OKukSs2ug zAdXaNX!x=WpseCe9@F`9sp(@~OCX#MLOceQxFHIvI8{CGd$?jaUN~clb8#hvcEm{u z3;@C>-}iu3I(?CXVRN7GA-N}2tUk7{D=I&ibk!?Uv7m2$ z+|&7H%YKhw!Ji#s8SI)LsfEJEX&C?G^_s=JZ^rcssG=@DJ`5O43ks4HFZf0Ax|C^j z-QYFhGy8RPL_U@|gsSIecj8)>fISsmUp zNOdcCXjzZ{QF1Z4P+o`{s4x63WcT~bl$g|vum5oViug+Wm*k$A=l-QUM(t-@qmzy1 zs!15DWv-V8_lIiSS2J%TkKZ|uvQ$QThrF!{T{S@ZK!NeBXE4j}%5YvIEV@1{HRljs z{W7Lk;nXgT%98v+AeX2p9TPLQ%XUH2e|QnrjM9mkB@bTfoO{h0_l|kDmL?6~rJ^b6 z9~C3gQR9h3N0>)zXTH(nfjSvq3c&MDL^uII-vsZ@g&XYKsR(t@ z;O=i9GCB%5k1XyCiw==I>kHWHzWb75psZE$Nq+aAO={06@Foe_N~na0XhsnLc`w;*y8svp5LC zFZ|KJ?+~p7A+mHklvWyi)a17F#Ck?vBIV7*lPRCh}D&rmkC zuW0mPf{>T2>uE$_{p7me%kG`aMV+9?e z4rNGS5XKss>gy23!oyDH&f`4btvr8(3i)DCL>+IZxKJ&e?n(GW@p2kVvQ7#Qm%>C+ ztsO#5N(wJ#lE6!YE0Fua`gJUD8)fHRQddE1t=+Sf!kN)rkxEkhPboyWZMcOo~FX+esz+)zXc?`8}+#SuFOl=*&>~6n{ zY%WgLQR=F4SmZ=~rMUD|KaU8Q2*e9i4!HfP#X8 zj*gC%m6eBwM^sc)T3T8`K|xDP%gD&c!otGA!2t{g`}z4nAdu+j=%l2ickkXoq0sX3 z^7{Jvj*gDr-rnKi;qmeD#l^*qjg7s%z0=cE*ya!>|KZ``oS2pvRzh=;*9QXt82FDF z4v_f{)&(R**s7=PHNQW4WveU$2R849hls-m+-wowSi zI-2Ph6CNnVeIPn4xjO-?#|HY2hI`(cwOw_0&NNKOUii%YOkWV!G{HV?;C2!^I`r(h zL+n)h$@G*RQ^DBpUb>X8q8yx*05$sUfaWNRV{hM;x_j1%Pu}K zIa~Z z@BNrS9IeLgIXrjrB!OtitA)s^Rpb%8o38nt%(Rgb<*J^V04mxJ%Lz~F9HDokqaFug zD+LMeCeQ?xm4r(jzJs%kxL1N>R8zxJ96iRq3v(1y@dFX|HpxQqxanE;8GC^_{UQN5 z78Ii!g%d(kM*f?*l{E)rr?*XA1KguC^?_8hIip=eoeNkOrwY@zJ7LD&Bc-K6!Z%0r z*-lLl23N*F5obR4`GJ}9Gl8_t1V2z;u2{|VUCqfxL?K%%TWFEr{gyEMA^hHzy)*FC zLU~|T0WAcErLh^ce0|^Ff z0&y%ZztL5HRc3Qk`Ia>Rl+Wr&uQRBZK`$KpC!u#K-{}?U*kvHOBg#1gZ9*4*OAN3zY>p$}T zZO36f`Y*8jck2H=EHJG8??L-7u)whTd+JJqiJWRbW!QMp5bRY8P>@lTu9SQg^7mhy zFknxaJ3F~Oeh-9g6gKq?ofr5B0#>S?Xur3ivYA(fr;FatU(j?AuZ2@Ga7*BEx4?%= z#}%;LtQ@NIYgKNdwE_^X%?5;51h;`8JpHzXBCgqBK1s`A7ru;eI18gbngy1g1a_*= zj+ahnOOnr{R-g1m6Ej+*xuojfVygqWHCT@WDOnA)8aO_t3{uiS&Rr}ZM zko1WAc-{AvX7b0g%RUp1`O<`>FVGr6Qbgy1Hfz$#x^^04lASpE-idtV<*nnRgj z2r4o+>@oF~8vgiM4#z0&AT0j;d_z(WDS!Pt&o#bejUtMHyKFYnY;*ha)KV|x$Cz5e zm>&isFr?1}l9*G&0$jFzN-x~=pfF4&uyEaW8UbU9LE$Ca~+y-u8pkl+&ob zDd7QGmx5)HJ>+K{F&e0DR9~vS1>p!UT_!WEqA*b2mE85KZV(zCVFhnBy9JiOq0QH3 zCsw{A&0eSd5&F|C!yMgP5uu*NUr+cehVae2*6T`T4qPzI>iQD1RV$she%x}cbpte- z2^o`nXBt|9j@wv)W33{}$L?^M-yIMNrY~{vbme{VQi#>>@}t5Ap96XP$m|c6f}KT@ z)@|6NN}`MdRZ_9}WNG!vE?pLxn!@xP8UP+2i;Y{e+TKj1;374yJ(9}N$*1SrX>)?wj22%q;zu}lKUQBm1i4S=P;}{+86io>6aLuY z&Gtn$z!FbhZb1))r1TEdZ|;C!Q@HlO-<`)_v8jS<0pccCV;(aT^qSL z%%xiXa$r5qJM~9!kMX z{~3g1Ajdzki{u!<{)2HTnKt%UCt7|7UCh z?Cn*Ntm4!|{Uq!{Fs!eEt3ZdCFgjTVnnNuD_=tjnhTJQjve+>jjStc(&>O;jyXJp* zTsN2ft|I=)pqIlevq>F#^;vdThv~{q)iq?lp)5|zhDSYV;yu&No63ps&B$?PE;I{P&gjd@Odw}HI1$Mb{cRuc;QMU2v9^3J__IIK6W3wgNE$q|+NRd_yjLhxRg-C*0 zBnk^U?lJG7;LQa!!KCXG@-qs2nm2t3MRuT+3!cyfD}UdVow*;@Suge6N950>|*d+c6;DG15W7>3QX@gou{a3}6V!0kEl zBkZ85)Bqlsslft+UR?HuF zoAd92C;sG2hPCj2hf@5bwZH5As~h{#?)fuy!MOhnt@vH*Z@#~p)c@LEQ`qHSUEcrB z`ByXT(d7O!VZlIz#p3*7fdA(D-OGPB;r`@0g^ljNGw6Ox`Q7TjPM61p{LjQq{x8e= mzcc<-;g7TF&$LbbH*-r}2?=Iy!pt|=j|3h7cuDhk_5T1_g(JKG diff --git a/sc/qa/unit/subsequent_export_test2.cxx b/sc/qa/unit/subsequent_export_test2.cxx index 56bf56009e97..2cd025858d9a 100644 --- a/sc/qa/unit/subsequent_export_test2.cxx +++ b/sc/qa/unit/subsequent_export_test2.cxx @@ -1608,32 +1608,6 @@ CPPUNIT_TEST_FIXTURE(ScExportTest2, testTdf137543XLSX) u"_xlfn.LET(_xlpm.first,B5:E15,_xlfn.CHOOSEROWS(_xlpm.first, 1, 3, 5, 7, 9, 11))"); } -CPPUNIT_TEST_FIXTURE(ScExportTest2, testTdf170515_invalid_parameters) -{ - createScDoc("ods/tdf170515_invalid_parameters.ods"); - - save(TestFilter::XLSX); - xmlDocUniquePtr pSheet = parseExport(u"xl/worksheets/sheet1.xml"_ustr); - CPPUNIT_ASSERT(pSheet); - - // No function must be saved into the XLSX, the parameters are invalid (Err:504 in Calc), - // and Excel fails to open the result - // Function with invalid parameter: =COUNTIF(BASE(1;14);"VV") - CPPUNIT_ASSERT_EQUAL(0, - countXPathNodes(pSheet, "/x:worksheet/x:sheetData/x:row[1]/x:c[1]/x:f")); - // Function with invalid parameter: =COUNTIF("hello";"VV") - CPPUNIT_ASSERT_EQUAL(0, - countXPathNodes(pSheet, "/x:worksheet/x:sheetData/x:row[2]/x:c[1]/x:f")); - // Function with invalid parameter: =COUNTIF(1;"VV") - CPPUNIT_ASSERT_EQUAL(0, - countXPathNodes(pSheet, "/x:worksheet/x:sheetData/x:row[3]/x:c[1]/x:f")); - // Function with invalid parameter: =COUNTIF(TEXT(1;"#");"VV") - CPPUNIT_ASSERT_EQUAL(0, - countXPathNodes(pSheet, "/x:worksheet/x:sheetData/x:row[4]/x:c[1]/x:f")); - // Just check one of them that it has the right text - assertXPathContent(pSheet, "/x:worksheet/x:sheetData/x:row[1]/x:c[1]/x:v", u"#VALUE!"); -} - CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/filter/excel/xestream.cxx b/sc/source/filter/excel/xestream.cxx index a0aea0032b00..67e3caad51ef 100644 --- a/sc/source/filter/excel/xestream.cxx +++ b/sc/source/filter/excel/xestream.cxx @@ -665,8 +665,7 @@ static const char* lcl_GetErrorString( FormulaError nScErrCode ) } } -void XclXmlUtils::GetFormulaTypeAndValue( ScFormulaCell& rCell, const char*& rsType, - OUString& rsValue, bool* pbWriteFormula ) +void XclXmlUtils::GetFormulaTypeAndValue( ScFormulaCell& rCell, const char*& rsType, OUString& rsValue ) { sc::FormulaResultValue aResValue = rCell.GetResult(); @@ -675,8 +674,6 @@ void XclXmlUtils::GetFormulaTypeAndValue( ScFormulaCell& rCell, const char*& rsT case sc::FormulaResultValue::Error: rsType = "e"; rsValue = ToOUString(lcl_GetErrorString(aResValue.mnError)); - if ( pbWriteFormula ) - *pbWriteFormula = XclTools::IsFormulaWithErrorValid(aResValue.mnError); break; case sc::FormulaResultValue::Value: rsType = rCell.GetFormatType() == SvNumFormatType::LOGICAL diff --git a/sc/source/filter/excel/xetable.cxx b/sc/source/filter/excel/xetable.cxx index 652b19174604..4e329d8a9b65 100644 --- a/sc/source/filter/excel/xetable.cxx +++ b/sc/source/filter/excel/xetable.cxx @@ -943,8 +943,7 @@ void XclExpFormulaCell::SaveXml( XclExpXmlStream& rStrm ) { const char* sType = nullptr; OUString sValue; - bool bWriteFormula = true; - XclXmlUtils::GetFormulaTypeAndValue( mrScFmlaCell, sType, sValue, &bWriteFormula ); + XclXmlUtils::GetFormulaTypeAndValue( mrScFmlaCell, sType, sValue ); sax_fastparser::FSHelperPtr& rWorksheet = rStrm.GetCurrentStream(); rWorksheet->startElement( XML_c, XML_r, XclXmlUtils::ToOString(rStrm.GetRoot().GetStringBuf(), GetXclPos()).getStr(), @@ -953,6 +952,7 @@ void XclExpFormulaCell::SaveXml( XclExpXmlStream& rStrm ) // OOXTODO: XML_cm, XML_vm, XML_ph ); + bool bWriteFormula = true; bool bTagStarted = false; ScAddress aScPos( static_cast< SCCOL >( GetXclPos().mnCol ), static_cast< SCROW >( GetXclPos().mnRow ), rStrm.GetRoot().GetCurrScTab() ); diff --git a/sc/source/filter/excel/xltools.cxx b/sc/source/filter/excel/xltools.cxx index 84e637e034c8..c6ecb31ed6d5 100644 --- a/sc/source/filter/excel/xltools.cxx +++ b/sc/source/filter/excel/xltools.cxx @@ -218,16 +218,6 @@ sal_uInt8 XclTools::GetXclErrorCode( FormulaError nScError ) return EXC_ERR_NA; } -bool XclTools::IsFormulaWithErrorValid( FormulaError nScError ) -{ - switch ( nScError ) - { - // illegal parameter, eg. COUNTIF(1, "x") - case FormulaError::IllegalParameter: return false; - default: return true; - } -} - FormulaError XclTools::GetScErrorCode( sal_uInt8 nXclError ) { switch( nXclError ) diff --git a/sc/source/filter/inc/xestream.hxx b/sc/source/filter/inc/xestream.hxx index daab0f76f32b..cb2671b385a8 100644 --- a/sc/source/filter/inc/xestream.hxx +++ b/sc/source/filter/inc/xestream.hxx @@ -251,8 +251,7 @@ public: XclXmlUtils(const XclXmlUtils&) = delete; XclXmlUtils& operator=(const XclXmlUtils&) = delete; - static void GetFormulaTypeAndValue( ScFormulaCell& rCell, const char*& sType, OUString& rValue, - bool* pbWriteFormula = nullptr ); + static void GetFormulaTypeAndValue( ScFormulaCell& rCell, const char*& sType, OUString& rValue); static OUString GetStreamName( const char* sStreamDir, const char* sStream, sal_Int32 nId ); static OString ToOString( const Color& rColor ); diff --git a/sc/source/filter/inc/xltools.hxx b/sc/source/filter/inc/xltools.hxx index 057b1a684864..f67d896af949 100644 --- a/sc/source/filter/inc/xltools.hxx +++ b/sc/source/filter/inc/xltools.hxx @@ -119,8 +119,6 @@ public: /** Converts a Calc error code to an Excel error code. */ static sal_uInt8 GetXclErrorCode( FormulaError nScError ); - /** Checks if erroneous formula is valid (file can be opened) in Excel */ - static bool IsFormulaWithErrorValid( FormulaError nScError ); /** Converts an Excel error code to a Calc error code. */ static FormulaError GetScErrorCode( sal_uInt8 nXclError );