From 497a934374a8a084414d08644dab828b527ac097 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 09:08:10 -0800 Subject: [PATCH] Fix for 3 Issues Involving ReadXlsx and NamedRange (#1742) * Fix for 3 Issues Involving ReadXlsx and NamedRange Issues #1686 and #1723, which provide sample spreadsheets, are probably solved by this ticket. Issue #1730 is also probably solved, but I have no way to verify. There are two problems with how PhpSpreadsheet is handling things now. Although the first problem is much less severe, and isn't really a factor in the issues named above, it is helpful to get it out of the way first. If you define a named range in Excel, and then delete the sheet where the range exists, Excel saves the range as #REF!. If there is a cell which references the range, it will similarly have the value #REF! when you open the Excel file. Currently, PhpSpreadsheet discards the #REF! definition, so a cell which references the range will appear as #NAME? rather than #REF!. This PR changes the behavior so that PhpSpreadsheet retains the #REF! definition, and cells which reference it will appear as #REF!. The second problem is the more severe, and is, I believe, responsible for the 3 issues identified above. If you define a named range and the sheet on which the range is defined does not exist at the time, Excel will save the range as something like: '[1]Unknown Sheet'!$A$1 If a cell references such a range, Excel will again display #REF!. PhpSpreadsheet currently throws an Exception when it encounters such a definition while reading the file. This PR changes the behavior so that PhpSpreadsheet saves the definition as #REF!, and cells which reference it will behave similarly. For the record, I will note that Excel does not magically recalculate when a missing sheet is subsequently added, despite the fact that the reference might now become resolvable. PhpSpreadsheet behaves likewise. * Remove Dead Code in Test Identified it after push but before merge. --- src/PhpSpreadsheet/Reader/Xlsx.php | 7 ++++-- .../Reader/Xlsx/NamedRangeTest.php | 22 ++++++++++++++++++ tests/data/Reader/XLSX/bug1686b.xlsx | Bin 0 -> 11740 bytes 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php create mode 100644 tests/data/Reader/XLSX/bug1686b.xlsx diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 3a75edf6..5dc48ff8 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1280,7 +1280,7 @@ class Xlsx extends BaseReader } // Valid range? - if (stripos((string) $definedName, '#REF!') !== false || $extractedRange == '') { + if ($extractedRange == '') { continue; } @@ -1350,7 +1350,7 @@ class Xlsx extends BaseReader $extractedRange = (string) $definedName; // Valid range? - if (stripos((string) $definedName, '#REF!') !== false || $extractedRange == '') { + if ($extractedRange == '') { continue; } @@ -1398,6 +1398,9 @@ class Xlsx extends BaseReader $locatedSheet = $excel->getSheetByName($extractedSheetName); } + if ($locatedSheet === null && !DefinedName::testIfFormula($definedRange)) { + $definedRange = '#REF!'; + } $excel->addDefinedName(DefinedName::createInstance((string) $definedName['name'], $locatedSheet, $definedRange, false)); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php new file mode 100644 index 00000000..e4090d98 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php @@ -0,0 +1,22 @@ +load($xlsxFile); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals(2.1, $sheet->getCell('A1')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A2')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A3')->getCalculatedValue()); + self::assertEquals('#NAME?', $sheet->getCell('A4')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A5')->getCalculatedValue()); + } +} diff --git a/tests/data/Reader/XLSX/bug1686b.xlsx b/tests/data/Reader/XLSX/bug1686b.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..d496c5c76e8235f805e988117eaa75e1a39782f0 GIT binary patch literal 11740 zcmeHtWmH_-(rpKKcM0we!QF$qyEN|Z1b3I<0fHq+aF^f&f|tqpNL1Q>NL01R~h|E~XH1bR~B6u?PPLJuh)!Vonk3%!{8Ty{I- zl8E@Hh?u)-Wd^~FD(fGYwY2d~ZW8*~^;0XE4{w=`LOX}^t9l8@a1Uu6^)JK&C-e*-otZN0Llp$RB~;6or_q&>dfx|Ca`ueygRGAY@$n0J zAb>rHW-!X{QJMA-t$9kOJQN4%zya)#1M?F8p^Y@cLJFgyG^SjGf*8UYgPbzOx!Aw) z>Wk^{cU8dWLr~GTtIKO|IFmkid-fCWUsoR`5MV#{1o$}SgWp;#Mz3FL z&7@yf_!4us?+FeuN2B#I+*ai$5026j+S6xUrmgMd_D z`l35Z$Y$emdF`BpD^3)?VB53yto_Cke+iyGm<4yyWy&_&a&4geW`@d+scy2Q!DHy| z3PBJQsgIA~0L8z=aGe@6*%^qKvY-%!2gR^~6VS$)iSftl|6=#QIADK!^s+=bg>Ghq z(8HH^VFQ=bOYtZoGVa1JTS!#>UP&z?*TuXg$6xB8B0y2a3j&w)Z}YnwTv+0Z*%=_Y zSbb3zg@(aPR_|UO`ufJv6`GdPDOJL;>{}12+w}SLMY^Q4C$(E!9BpZ1NuJE76*7s* zW3eifQAQ0sSoA`IP%QrR0PQ|Gt!3k@if7XzD*NT3l?|M^J4vIN{xhjX+lYeU+_HNU z>6in~#^!SsJ_9x+=ht{@Di++9mBu+vJfxlmrZyi>#WFk4?|m5Lvj&t&U!YyFj!O1Z z;a-t^rP%t=TM5T05f2j=%MWDq?)HP9mO`Ok8g~ z;6V)kzWZocusAA1aB=252nwLaHs(z7J;0K~QeI{vYsy(Wz*PG*5k~VQeeN?ig;tUZ z?O<0=>P9++QOYt2#2rqw7Y~poX1A8Xl7a_C9-j2$9i{oHS&Vv7|HqVI!CGgs@*!@a z#hSnfD#-WNK)kHs_ZdmIV2k>s8ahr_o=0)?I7c^>DEoS9)yXq*{Fqb#TQ`Zr>o|J! zJZuupgZ{5v-#Nokf7>h(mZYyb^ge%3XFuIP1RA)HeL2JWCH}Q^!XccPuuh>^`WK!TxE4pY2 z)(lKgUKl>vEXS^4VWw}+9s?rCQx4L<8kmb--j{wbghT#f(}JNJbXapmK7qUpCaJbSOiq=lMXt_NhmdX!IjIYOVShfrd!-dxpx`|}R?abh)HdEn zyRj>?rF`7nn6EPU4L8@}czOAvk^%Wq;(+={G*{8MEI++Wv(ebR?xeC4Vq0{B%J0_h z3@JNUqlp*YmIndgSYQsK@@y zw_ehfetm^0+~175q;3Flf{MiCh*LGINw89j>>MuMdBC6u`9QR8Vc#Jb(NUH~t|~pE z-tk$v`abE);tzXQ{3_W+Vzpz4Kr+=`q1aDW?ls)|9O~p=<2Qk&nti^mH|~)$Uwwt+ zRORhrRig#;WEo?p_=yw_?9kQtpxgGVNhEtooU^;XDS$%y@0C%oI`=IVC>`8F004LZ zm}j7%|D!nm9{T?(l+QpS7j*A`_tlf!Dc!>i5_y4#icSrdr4^)^2g0aWXQaE}2-~%Y za!HBN&A#5E3H9(YZ5VA3W}$bF{j!t`OI6e3R6HNbQ0ZgPvGNk?6^qdJ?vH$2M@bb7 z;l{q?8uEM94>2*-+m{4%Q5cktNDB9QbXw&f1UNHU+jREVi=xvOx-uxj==Ji(BdQM6 zkk2`A&KO=tnAI{tINy?95#2CyJyzHbDNFfOpS%?pHK%WcU?8$qqzn%%jfcQjaU=%b z@%5Z%pm>nGK%GUg4)?H(dI`i$P*k_DfAWcW`QfD^pfmW(gJ~*PH9L2EnKe{ugpsVL zfNC{G+F6_t7FDg5tH{)DWx(3*xoGQD)B^iH%^th_f#^V(a}^}#W0Sy|%r`0n^#{a% zq8S7f$*K`Zd~HDj05~B2{8?L^Er385XQrQTEI*1&R`Nn@<_~b`&Ixrdc~HQ)NxEpa zDR=a3)NGM2V{y^wUcK8V$5->cjLjoSskR!I7RF2ZcDs2b>U61#wGK1?Qf-*#G&EU= zLY1;~Kg;j7`ME3&OR5GglPaXM7nJDXM1y@|(u8*ncqIgEO8R7Wo$KDM#9r=tp9;I` zl|qHfCx++eUpVWaa@dhmLWmH;Ogh%9^BG9K_~Oux7QXVPK%!8s53!(b?7bP5|MFdg zPMk>h)A@rUaxks6n2jmPep7(5a^#hf4$Uwp#kI>`?Lc(6=t|t@(j7DLoQ|bd6nI8eM&apJQbDq0@)AN4GsN z%<43mxd!ta#Cku{8-C?Ot zHDu{md;-9NA*T$JxxDR?=UaN9TiM0(Ezt-P;OXLyo<)LnlDS;PslP=%;thGx)Q4T? zYO*z&VH$rbYV>Amn`~fzB;RITe73|{IxG9I6Q-70+S^FulD~mZu9gRNkt5a8>R_1= z%j9k~$dXidC(!8L(+X?^O;91p1kVG9uu3EV8;qBhv4RuRxk|`l&Zyt-u~xgoJ3TgY zPp*G4R9)@st z$Joa?b?dS(NbDb_jQbSas`ypc0sb|-_VlvCoPlYq85J8GUNd|uZ3agxG$!v z%`^#dNz{nO_Aruufi%^We8Yt1{*aKHT<0aSQD-2ojMsO>uV-;O(mnOpDYi)|3d(ANL z+_X(FPruH`Hjcx6k$pWSG(nz@-doUTE@E_OlvbajTZXtomv@hA-20Jl%e{?YDWvF% zpR0GzPEca81QkVx`{g5tFpF}l$98R%4X@BLRR&9}JXSl#mpwGI8-H{uDZ3c9^0)xm zkqrAugLp`zJL%@(M@EF znipP!IhM)% zcs20&OekcqlAng>hNq%FV6o=|Z#(@t`#*@dwT1PKt|Gh$CB+_I(B?u~g3R@Bi)l8? znJ#3Pgb2xJ#w3HcRe-O2^sZv`F=cI#71eChXw%p zh<|MO{LED@7C>7d)6dtRX=+bvI0~N=qXYX!5W(5~j(shTV)^T+P0|v%X?8L}UHzVt zIvZRD*WK6aK8|*);OWRDWwO>K8r=FO#EDfv|Sh@bq^w3vc|K zP;EcK^v{A{i`nF*5=n6{&|>w%geEk8G~)_bp^WZ-*+upStDk@{XgLjwu28ughb-|* zKhRjw+n1GuY!5L-+TZ(KtHL*y1QEn&>J^RmRTOzuYu(i%;UQbf2ZSW^Cq_9eyw6v? zCoZYc(EKc}DegD1813k|?aboR%a3gwXs| z9R7najeu@;aRxJ85LXdD< zBF(5$JYwmdvWl!GDd&cE^Z}~?;1!rMxzy^XUIc|PnCiwTYVNN+3lx(uX!fCKl!lF} zG>Fn_S04ld9*W2!>zP>#QuGWXH1U-$JdArj$=2V|dsMOvf2K-rr*{8+1Gz?HqG%Pf;Cr^>o}_554YqJmxvCTwTOuYWF=k z*-pY<@jc&Eh$Ea|ARtUTB8jLvs`zGdNEu}mfOLEOOfb8LS)?aX2zAS?1(sNwgb|JI~km8AC@$4mq+*NvBN_EfHf0 z)(4EzWq`{~wp@)><++$j;CIAGw1^_E&Iy%{D$!`d(iYM1jTwH{RoTjjvjdb5LE)1W zqE3bs%8a$5#^FO!R#0QZliC=f>CKtnOls7ov=O1 zbe+}7zf+R3f{9$rE0X5)#)Ljt<~vW=Ii7F_9KssX?Q^{zjzk%!t%;^69JU00x7ip) zKIeJa#DN|^N+c5T1ot2*Y@fI}UNL@A0>x^U#Rk~=u6r}mM#y!mjAW|t8{i0GRcmy5 z8xgikU}Y3=p09qkZa9LCm*@0d%6tosic54Kj(3BAv*wr)+t6myxCW_V3gMn~8Qs8$ z+Ig1YGhpI->n}7;uVLtbm&eJrmFD)2fgV`Y5vVH#*LY$}(t2DQS*l*yu}E@X*}*o5 z)}tnAW+1ni6I(i@c4J_)%wKz99r)-u(D~Wd^cxy1^Y6Dq;QCnM5??f245KJn2l5$( z!oVS5F9n8UKIn>NmulM_BxlZMav+0aNT|1=C#}cyjz&~d-MJ4wcYo##1sSJve4EUZ zNe$c-3#fs;B%-(WvdCHd=#W_E{KZqvS%3QK@%Fvhq`trh;D~klsdnG_#ksImdaF;s zAm!o7R*phz)dm|$^1%>fZ;Ob#FGTBjG&FO`f>JY8lhv~k(yI*iH2TG4GKMurN9KL~RlIE@On6>6)Qli`(*(1)HoY~v0&XN-!YhgGUHUUcV+DT>^x`d;YBTp3YO{K!&;eGE4 zVHx=xL0215Ok5i{&n72ofu<%C9dvBOtEeESone-M%u39JjcfN+!$FIg*2oF&{I0|zaoq4;Ad};5&FWMBAIEkhQouMqhn-}%c?o7EPZHMu$ zBbJ_sPF2$u+TF`DI8|6PHX)%+drG%{oAQkU?__3p^O;pnt5v=`vZh99p_Yvv20>wp zYEAtfn*ND zsKq3$7?_n-ZYqXHKN=h8CFxm)WIUq|6xAT*JkCw$KlINq9;|IJ42PgbMadd`#;*Hzv)ZmZ=i_*YN}PRLH<+ts>t+>VR>w~b z(I)?PZ0`$Ch<>@wA)?n(jY@2yq|bc!@@Ndx|U z?maP`iQxu~uW38r(yFEY5v08SUVc6EUfC3Y3bQh3-U8|+ezXeCE}k|(=O03>K+V<~ zB*Y#BX8loa4hG%U=wa)JCZ-g(`f>^PN{xpiO7x@2%}4qkZspzd33%$q{UYQyC!I%^ zoRuds<>6{g`kcyH3G_m6Sk!hROXWg41TZ@h$aYKKmyAUMJx1xZVCZrsE@~H@y}lPR!V> zhm~f)-?_?wKe*t>lBm{Qv`^-6VR&VQEoTwYY++?Qm2Q%_d`lX)UO28ka{%WMQDLoL z{n};IRNlf<0&IPTL>v}&Eoz?kRJs6`#iqZ_hfBe_JWIULE(M$=pv1m&Rcrh3{n49N z=kP(5Tk!o7fI_fTPO7#=5W!36>|OpyX%4YP<<6PV+sX`S#)}}{i&*`A(X90`??dvO zexoXfaCk>!OmDES11j;OEN|gvbA#?{?TOn0rSJ`J8vVuW5tbw!S$TwRf<2qR6a{_~ zxLn&BROz&W7DHUIx!-XC@37t{D0x$iS&Uy^a#V;wOa(qjla3PSchfPAJ*yX^N(aAv zP>RsaeWd^~|8t&o*pSw2b({yXd)2N2j;$`h#m#VQKuN_^?Y(4YnK0jP3 zr8#2xVkp?`rEi!?#d|j*mLJM{gX!~Xd%Lmgc6;m(&n8uUfB@hXuYM| zrk<}cK81Uq44RXYE4|?3%JHdo&}HqMRw{ifV!{*-vCx4b94j4v`bPKx|DWir6s5$+ z03tdah|;8g&>86A0(7!7vXQm4vvy|s{nJlz8J*Ax+Wq^{Y_y9G=+)>HOaa|PjNwJL zdoP~#?UmunvJR0)w)?&GttU*{jpZUlS=fDxIQpIB8+~nKXqLKG^*r-n zN|*3)t@A|ShrxhjCt?hFK`_UUnYMxx>t5kzz_N%ibQ!qfsNQ+>?m^0`TOTKq;uEC( zXfwE}K1UH6E1kE;`rlOLpkvObPkG1ZoIB%So{u?C=!6L%)#KWtDG0a}AuBr=Wk{do ze*)^|9GoH|b;~ih8PYZJ<=By`>Ai1x$1SF5%y&q@>zJWM?lhQE1w>eaW}}$C7F=sC zX3TqzSVUyUR`Cde^6$B$WY-Uc5d=2v7jBR4@64xb6 zK7{3bYf?sLPgR!T18@sh4V|ZFtIX$9b%e{H2M$cgWSmge>To@grBhqvA7H}=Pxcz` zM@;_!`|eyseuHoxcNn82W-T~zs$OEPt?OMD65ej1ClR+4ck0< zj?b1Xkq`)MIFIpl#n;8RK6GK0cpLN>~oD=;c+VKFy$|4ZjOCYrW92nRFU5rePT#SC?YEvZ{ z1$-8ic8yg*&35}%5)6^r7HY5!F$z`co$0Fn2(=DVZ%cKxM{o1CU?{G+HC+Ew{*25Z z_H{_(iRC&Mfl``hRIO8XGZRD#nO0I#gNssjtwE_s@mq547ll>vP3!6C5DxDOZ?n21 ztH>`5Ju|KWNW6oRLu{#z{mS7Vf%D*|*0Y{^2CPGMZ_&GUh_9(IO2yAUmGYr4vwld% zI)XL#>s|lKHFAP;E!4tkry%-DEcj4Q!j=dRq9Zr=&87Qp*kQ8f{_|TDpU=c^R_=sS z*|kZ1a3mnmJO(Q{zWX14mP0Nf%EVelLOo6m9QS-{LhCJc)?we~xoFKk-35rt{FK3J z@Z7Gg?(0c#ugXLVOM)XvNPmN4{*I3)*j`g%)Q=K5R-xp3Ko$*7k$wrLU^Dx$gU7Xg zn>-|SQ=)TYPS>zd7@x1~gEZZHPNC{^eg0lMJVbjcU!Vgks>6a5s?Lkuhac|&g=H^| zUAz3Ch#1a3Bm)r{)>LL!1lyf-d(Dhv!tjvWeOSp@=_tS%!<6-CBV3bMz9TDb;vTf+ z@%I2~5K=s=11&ZiXjxJI$W|ssHYTDLMwWIz*3>6X53Y+DC3pq$uyU_Ou3Rwu1p}!E z7{Aa<4fFv8slcuMnEc+rtLjR&&wCB7-ZfAa82J~mYpv9%15zuN?%zCko}1wZrv>i@ zAIEYn(-<$vQq^Hd9HfjPIR!-#s7c?aR#9VWp~9~u4UNn}(<$qBhx7`>Z{?Fpf2n*= zKlAxJn2i-|z?xl1E*p5Dr~F02Yckv(dp2|*Al4&m5ci1c;b#Y~q*jmWkD>0FpdS7o z9&uh(M@$m*c$Og6qJnn3P3=t-o$MW)nM~}RfIprQG!g#aMmGq$=tL!jZeom(6}dZP z#6yO{;YD)W*Clh4SaMn8SD_9K>{!x({fooGef84QPD!p!zbt1_j+G(WD+rSz?Tv6T z7C50}?PROWx5ekAS$Q9MiXcj@Ah~&7iJz{XkgjE%%zq4e$Bv^Bh{z|UHRu}aHewlB zwZR+zSzwPH#ttLtAc~%%X^l+Nf`*=y%oH0&Z&E%5^QP zuOej~1`(MxGFRK5VbbUMB*_9j$ZKWF(p>2}4&`81zaI^GS9sfLybTM2@nN2y;i4Jc z-e2+xh$NR^jVCwCs#t@~mZeE=s~>$#vk$t!KEYY_7fV!jlAGKP_W;Ad6}Tnn#r{Y3 zWp5P-z5}tZ1QdQKzq8NC!Qp@813~uZm8BwQzrc*phCJ)f>|(!OAkG3&9Z9-B)-4c7 zu|JX}+b?4K2}O%+?!y~`@`_1J@Ou1SNqzwVr5&pk4z0L=GWU?QQKy7Fw)R>(jquz3 z{u*RbGAIp^HA1Y_SL79Ex9(PiK4Ae2%2+f6nE7T0smh7(svtAOXT=Ln@*rdTE0fya zmv!RX=V-goV|)71IF4>f(~mYMD3bR9M;~Rhz1X(ihVAD?1=ckmH22|sS>gJFT%@ zC<=IixQO&v*n78+71Z~_de3*K>x)y+S)I1%S58^(HyKXJsq|8`oI#N9Htpdh*kTnk zuGq!kxtg5w4^nbc?0Fr+sJ){Fv@Ty>E`3=6tew)mp`0!XeFfBwc7*Q>ZjM4G$Otf1 zLw-mdW%mc`mKP|9E8cuvbH^9A%khck`lR=a3A9juuOC&Oeznt}UwAUyLlm# zv5Yt2OC_XiK1L*)m(TG;4(Kr^XRvMJDP@#JFG z`nweU>fC-|1G(oNAO!x7k2jyX}vwqR`Tmp%uR+p?U){f=)o%g7RLU$*Y7T)E?gk|tX(Fbvl&Z$HXJ z9PV1pvmOk`__C6ah$vJ6AwwrTi-{bMA_1&lSHF;ajoE6)sdFleW~QX6%{5g#vnEHW z;?|FB*8nP-Q?{xBZfChdD2uU>eWmIxC%Z~cBOCW1pWO1DWQxozLp1yL(ZS@kXpQy6 zA-6|;y~#LQ=u<+hH{x(*>^&j`3^6_Udth|DQw8z!(7k?#LzRroUYOZ~#+L z=C2O^dfw}I>mE?v{q6MEQ^TicV1AiygSI37oLc_tM9fp;zs}NsnF0XIa6gUz&1C(l zpQpppU!KH48zTR1bo$iG)7jQ9FN+|t{tsS$678v%r^Aq6UXD?JdU-k;dFtTlcFr#c z4!A!Z{My`kYWlQw{$(mi@YD47H1ZFP^izLNyTf1p$_W1tfB%Th)Bfj|KRi&w1VZ@t zZs_Tsd8)L38SfMSH2#b3ero+!5&O#)02m_y0RARvpPK(wh5T+VPyU