From ebc0b5695912e67545e0442ee3df0c044eac066b Mon Sep 17 00:00:00 2001 From: AlexPravdin <44760753+AlexPravdin@users.noreply.github.com> Date: Thu, 30 May 2019 17:38:04 +0900 Subject: [PATCH] =?UTF-8?q?Fix=20#853=20when=20loading=20and=20saving=20XL?= =?UTF-8?q?SX=20file=20with=20empty=20drawing=20cause=20c=E2=80=A6=20(#882?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix #853 when loading and saving XLSX file with empty drawing cause corrupted output file. Store empty drawing as unparsed entity and save it as is when saving the file. * Fix code style --- src/PhpSpreadsheet/Reader/Xlsx.php | 25 +++++++++++++----- src/PhpSpreadsheet/Writer/Xlsx.php | 15 +++++++++-- tests/PhpSpreadsheetTests/Reader/XlsxTest.php | 18 +++++++++++++ tests/data/Reader/XLSX/empty_drawing.xlsx | Bin 0 -> 3651 bytes 4 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 tests/data/Reader/XLSX/empty_drawing.xlsx diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 2607ccbf..63a5eb8e 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1598,8 +1598,10 @@ class Xlsx extends BaseReader } } if ($xmlSheet->drawing && !$this->readDataOnly) { + $unparsedDrawings = []; foreach ($xmlSheet->drawing as $drawing) { - $fileDrawing = $drawings[(string) self::getArrayItem($drawing->attributes('http://schemas.openxmlformats.org/officeDocument/2006/relationships'), 'id')]; + $drawingRelId = (string) self::getArrayItem($drawing->attributes('http://schemas.openxmlformats.org/officeDocument/2006/relationships'), 'id'); + $fileDrawing = $drawings[$drawingRelId]; //~ http://schemas.openxmlformats.org/package/2006/relationships" $relsDrawing = simplexml_load_string( $this->securityScanner->scan( @@ -1631,10 +1633,11 @@ class Xlsx extends BaseReader $this->securityScanner->scan($this->getFromZipArchive($zip, $fileDrawing)), 'SimpleXMLElement', Settings::getLibXmlLoaderOptions() - )->children('http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing'); + ); + $xmlDrawingChildren = $xmlDrawing->children('http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing'); - if ($xmlDrawing->oneCellAnchor) { - foreach ($xmlDrawing->oneCellAnchor as $oneCellAnchor) { + if ($xmlDrawingChildren->oneCellAnchor) { + foreach ($xmlDrawingChildren->oneCellAnchor as $oneCellAnchor) { if ($oneCellAnchor->pic->blipFill) { /** @var SimpleXMLElement $blip */ $blip = $oneCellAnchor->pic->blipFill->children('http://schemas.openxmlformats.org/drawingml/2006/main')->blip; @@ -1690,8 +1693,8 @@ class Xlsx extends BaseReader } } } - if ($xmlDrawing->twoCellAnchor) { - foreach ($xmlDrawing->twoCellAnchor as $twoCellAnchor) { + if ($xmlDrawingChildren->twoCellAnchor) { + foreach ($xmlDrawingChildren->twoCellAnchor as $twoCellAnchor) { if ($twoCellAnchor->pic->blipFill) { $blip = $twoCellAnchor->pic->blipFill->children('http://schemas.openxmlformats.org/drawingml/2006/main')->blip; $xfrm = $twoCellAnchor->pic->spPr->children('http://schemas.openxmlformats.org/drawingml/2006/main')->xfrm; @@ -1757,13 +1760,21 @@ class Xlsx extends BaseReader } } } + if ($relsDrawing === false && $xmlDrawing->count() == 0) { + // Save Drawing without rels and children as unparsed + $unparsedDrawings[$drawingRelId] = $xmlDrawing->asXML(); + } } // store original rId of drawing files $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['drawingOriginalIds'] = []; foreach ($relsWorksheet->Relationship as $ele) { if ($ele['Type'] == 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing') { - $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['drawingOriginalIds'][(string) $ele['Target']] = (string) $ele['Id']; + $drawingRelId = (string) $ele['Id']; + $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['drawingOriginalIds'][(string) $ele['Target']] = $drawingRelId; + if (isset($unparsedDrawings[$drawingRelId])) { + $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['Drawings'][$drawingRelId] = $unparsedDrawings[$drawingRelId]; + } } } diff --git a/src/PhpSpreadsheet/Writer/Xlsx.php b/src/PhpSpreadsheet/Writer/Xlsx.php index dd19021e..58897639 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx.php +++ b/src/PhpSpreadsheet/Writer/Xlsx.php @@ -328,6 +328,17 @@ class Xlsx extends BaseWriter $zip->addFromString('xl/drawings/drawing' . ($i + 1) . '.xml', $this->getWriterPart('Drawing')->writeDrawings($this->spreadSheet->getSheet($i), $this->includeCharts)); } + // Add unparsed drawings + if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['Drawings'])) { + foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['Drawings'] as $relId => $drawingXml) { + $drawingFile = array_search($relId, $unparsedLoadedData['sheets'][$sheetCodeName]['drawingOriginalIds']); + if ($drawingFile !== false) { + $drawingFile = ltrim($drawingFile, '.'); + $zip->addFromString('xl' . $drawingFile, $drawingXml); + } + } + } + // Add comment relationship parts if (count($this->spreadSheet->getSheet($i)->getComments()) > 0) { // VML Comments @@ -338,8 +349,8 @@ class Xlsx extends BaseWriter } // Add unparsed relationship parts - if (isset($unparsedLoadedData['sheets'][$this->spreadSheet->getSheet($i)->getCodeName()]['vmlDrawings'])) { - foreach ($unparsedLoadedData['sheets'][$this->spreadSheet->getSheet($i)->getCodeName()]['vmlDrawings'] as $vmlDrawing) { + if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['vmlDrawings'])) { + foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['vmlDrawings'] as $vmlDrawing) { $zip->addFromString($vmlDrawing['filePath'], $vmlDrawing['content']); } } diff --git a/tests/PhpSpreadsheetTests/Reader/XlsxTest.php b/tests/PhpSpreadsheetTests/Reader/XlsxTest.php index 7c2851f9..46b30509 100644 --- a/tests/PhpSpreadsheetTests/Reader/XlsxTest.php +++ b/tests/PhpSpreadsheetTests/Reader/XlsxTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader; use PhpOffice\PhpSpreadsheet\Reader\Xlsx; +use PhpOffice\PhpSpreadsheet\Shared\File; use PHPUnit\Framework\TestCase; class XlsxTest extends TestCase @@ -32,4 +33,21 @@ class XlsxTest extends TestCase $this->assertEquals($ref, \array_slice($data[$i], 0, 10, true)); } } + + /** + * Test correct save and load xlsx files with empty drawings. + * Such files can be generated by Google Sheets. + */ + public function testLoadSaveWithEmptyDrawings() + { + $filename = __DIR__ . '/../../data/Reader/XLSX/empty_drawing.xlsx'; + $reader = new Xlsx(); + $excel = $reader->load($filename); + $resultFilename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); + $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($excel); + $writer->save($resultFilename); + $excel = $reader->load($resultFilename); + // Fake assert. The only thing we need is to ensure the file is loaded without exception + $this->assertNotNull($excel); + } } diff --git a/tests/data/Reader/XLSX/empty_drawing.xlsx b/tests/data/Reader/XLSX/empty_drawing.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..8ccc575706f5c02cdb29c23a0423ae7423ed8167 GIT binary patch literal 3651 zcmai02{@E%8=f(C#$ZsE5R)a_SP!x!C7LiKe;mS$v5pzWp3t!*Le@i;$(|5pPnIHs z$iAj5QCV~B%ZdJ(I_Hr8)c?%&UGsg{^}YA^y!Z1y_j4l=AZiW(Jv}{uo&#Kb-($=RIR!DN2J*o{j0hTa<`^K=t0 zQq3r|hZE+*?O!e#xU;*&0tJuIl~zwUU;YsKvOCo}eaUHkW2bba;G&QZllC813Yj8Q z7nZ!@-_J>c?Ptdji6rxN+33VU)HPU)TI5V$CHNw1Uu$|L^G8@;#?tL~dhMNsH)+3&FnEM4I)XO&|=d1l-Hv6x_>3SGMlsF3f(b~c-tLz%-6rKtVP9l;id#xN%ySBpTFaeFK@#+ zj}%wFGF^OF@cPqT6kj#~4Xli>TG}#{6n(O#BrmW!g}$*hnQaMv&S zgLCCukIcJP{S`a>t_6PJ*IU>DteQpMfvH2}qur{96h6h^m7+3NF7Pbdf&Sx|HoR;k z=D;3Q;v!btZaFzhrH@wK@O+-c5taHv>(7mP;D*aeO1h%{kcM|vh~LUf-)@<#an!k> zi_=BMdkp=7cekmW=Ux3z4UT7WAHg?jrgDw5RdtjpRCZHWzR!`x2RG=hFHIWB-(bs-!uuf?}D0z)PO;M&hb!;GC*7v%!fwELfP^)eXg8J%xES-F85f znor?cJ&S<=06q23YA*b(Qx4SJih#i$th_YkIjRf6*a&AOH;qpi+%JAG^p?V+P6#fS z-&!{hg5`%T_l%}SWlc1`@94VTWTgm^FXIhWR*}e@rh%8foNZ%a?|kc42jYU7N0pVq zY%=Nf>k14M&@X>+OS^8%{n)56Ic}VZZ9tb+*qewoxORj&Pwl3bkI4}}U2FtOqVYa( zJ*mxH>Uen#^`I1S@3G~$VFZhTC3s|QDzqE8^yG?&EOP?Pb?5fzMiK^RwL`6Klz#TzUfq!zxt@+0L;fOT>o>Fch^r zav8~+azTtL)Qb|;jkWGN3G=TM*JVU6ge49#1fLa>8o8SObR;PJxpW{U>yo-44-axq zH^|qIvj+c_wTCYj^EKTHlXl!Fob_|i9_YE>#SyU}kr8tpta{ z`%9LL6P$RZyE9IE##BQ(CB3Up3g~3Fr7|0-{MxgamvPp7QWXQ?+^xss>XGcfjYd1$ zW6p_k&uZ^Jh=eF2JW0ISZi&jY!!!L(+Bf3~4FrJQ1VKGZhYI(r_SaGvNJ`F%UM;~~?rf_ege zcJ9Ih1Z$UQMS$LgFEcX6EyU@x`^8y481|W(7x7XEKZPhW@TWQ=|6`c5?aj7g5=LT~ z*oRq>AA8%d9KB1&l#i&V)vK{o29C22)T*#(jSST}b95*fFpI@<;EvspPpGB)v`I*7 znw3O<$s|;L$$pb~PfAJ+x^jjzJ_dEEsp0fDKxc_N-nkH5s^XQ&it?J$OXE}D<(RXg zl)vbKb>s{3fi?SS<3hfbpVGJ}Jl>gdUZdjlalvp_9mDQZf=f_Qm&+%_V)QSIrxqBG zf|Tv(P1TfEloQ_6gMc|=yrz5TF||~D6~0FVOf5HtLDgA=Q>Sf&y4}C{dGETUl2B&8 zMP`wAOt?hM@JGX^ngX5Ym%12+PQ5(KiB7+uS3_k|zNVHzl9$4S{&Gj6-g*<3b4E2T7I5S7eBXS`cV<|8k(o74BDRHLB5Tf(Oekb%+B>M zmyxGYfo7Wh{$Q(+W%V>_qOgRKZf$$()wwBkJAz?cyW0@g{IZqYFG&f$x3^n~x&h|` zbnX@8DvZnROjnIUC=4+LI=a0k?_cq!3^9@4_(soP!$XTD2+m5=O&%V08^*3Sk(rNV zMX+H0j7sg}r|&x!HZCq2M=>SEZ?6XA6(#wiAZs>)5*e)oYX{ER=RClnjaqd8laaZU zXsMWWGr+S#Tt>Mq8Fxmr`WZVa_=bj#_q19*zdg4c!J#itShGD@2VWiUlt8;J-L?6q zffwnYT+~B#J)6mTaqbU+)EXSpN67PD4@rR2DjbHu!x^uL?7=k0ZjmH2pB2Uhs$Hog zt1caV4n9+`l)MqKpUw^_`Hf$-@i-3*&cn*Y*A?S#NomE3OYH_AS?14Fdp3y@PQ8TF z^q!-jJHmoS8qZ*3xc*v>rzH)dPhDd3BdCr2BH|z0lMLjDxY|O~PBcLP~Vj=Z4 zpu5R0O_L9MW#P2^t}Vw!rp{xnXREIbr+fET+(<|pc-EYB(l+bbYYtcYve81#{)a&- za`MBxvD}vTv>)nw_)8*@a~xOOZA%rmWWt9POtZtU%_72c64;n4ZY`u+wA9W_rGMbH z4FFo{4v-$;V@w(%r|bJx|IFZ$OY!b}pEaA1B*5@-%|@<>>7)27^|N6Ual5O7ld~kh z>9}~A!ZqtAyy1jAsaeZvUBG39hQjoiXMC18ndN*CW!?d)H~{;u{-ATExccwoM;`w$ z%0VymJ&F?fyJVCD{%7Cw9!5FnCMdpXKTeV}{~qOEUDRQ~gM*%8LiR(Nto>xb|FI;8 z5f07@iXqsK>Aw*Eu?L5j9uzyuFxihO+JAQQ_h>o1_@E9@^l(2c>Ax=ifjS<>IoM