Skip to content

Commit

Permalink
Fix for #1516 (#1530)
Browse files Browse the repository at this point in the history 
This problem is that ZipStream, in contrast to ZipArchive,
is saving 2 files with the same path. I have opened an issue with
ZipStream, who agree that this appears to be a bug.

For the case in question, PhpSpreadsheet is attempting to save
a file with the same path twice (and unexpectedly succeeding)
because of a clone operation. This fix attempts to rectify the problem
by keeping track of all the paths being saved in the zip file,
and not attempting to save any duplicate paths.

The problem case attempted to save printersettings1.bin twice,
but there are other possible exposures, e.g. by cloning a sheet
with a drawing.The new test cases clone an existing sample which
has both printer settings and drawings.
  • Loading branch information
@oleibman
oleibman authored Jun 19, 2020
1 parent 12dd92b commit 82ea1d5
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 35 deletions.
88 changes: 53 additions & 35 deletions src/PhpSpreadsheet/Writer/Xlsx.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ class Xlsx extends BaseWriter
*/
private $drawingHashTable;

/**
* Private handle for zip stream.
*
* @var ZipStream
*/
private $zip;

/**
* Create a new Xlsx Writer.
*/
Expand Down Expand Up @@ -173,6 +180,7 @@ public function save($pFilename): void
{
if ($this->spreadSheet !== null) {
// garbage collect
$this->pathNames = [];
$this->spreadSheet->garbageCollect();

$this->openFileHandle($pFilename);
Expand Down Expand Up @@ -203,73 +211,73 @@ public function save($pFilename): void
$options->setEnableZip64(false);
$options->setOutputStream($this->fileHandle);

$zip = new ZipStream(null, $options);
$this->zip = new ZipStream(null, $options);

// Add [Content_Types].xml to ZIP file
$zip->addFile('[Content_Types].xml', $this->getWriterPart('ContentTypes')->writeContentTypes($this->spreadSheet, $this->includeCharts));
$this->addZipFile('[Content_Types].xml', $this->getWriterPart('ContentTypes')->writeContentTypes($this->spreadSheet, $this->includeCharts));

//if hasMacros, add the vbaProject.bin file, Certificate file(if exists)
if ($this->spreadSheet->hasMacros()) {
$macrosCode = $this->spreadSheet->getMacrosCode();
if ($macrosCode !== null) {
// we have the code ?
$zip->addFile('xl/vbaProject.bin', $macrosCode); //allways in 'xl', allways named vbaProject.bin
$this->addZipFile('xl/vbaProject.bin', $macrosCode); //allways in 'xl', allways named vbaProject.bin
if ($this->spreadSheet->hasMacrosCertificate()) {
//signed macros ?
// Yes : add the certificate file and the related rels file
$zip->addFile('xl/vbaProjectSignature.bin', $this->spreadSheet->getMacrosCertificate());
$zip->addFile('xl/_rels/vbaProject.bin.rels', $this->getWriterPart('RelsVBA')->writeVBARelationships($this->spreadSheet));
$this->addZipFile('xl/vbaProjectSignature.bin', $this->spreadSheet->getMacrosCertificate());
$this->addZipFile('xl/_rels/vbaProject.bin.rels', $this->getWriterPart('RelsVBA')->writeVBARelationships($this->spreadSheet));
}
}
}
//a custom UI in this workbook ? add it ("base" xml and additional objects (pictures) and rels)
if ($this->spreadSheet->hasRibbon()) {
$tmpRibbonTarget = $this->spreadSheet->getRibbonXMLData('target');
$zip->addFile($tmpRibbonTarget, $this->spreadSheet->getRibbonXMLData('data'));
$this->addZipFile($tmpRibbonTarget, $this->spreadSheet->getRibbonXMLData('data'));
if ($this->spreadSheet->hasRibbonBinObjects()) {
$tmpRootPath = dirname($tmpRibbonTarget) . '/';
$ribbonBinObjects = $this->spreadSheet->getRibbonBinObjects('data'); //the files to write
foreach ($ribbonBinObjects as $aPath => $aContent) {
$zip->addFile($tmpRootPath . $aPath, $aContent);
$this->addZipFile($tmpRootPath . $aPath, $aContent);
}
//the rels for files
$zip->addFile($tmpRootPath . '_rels/' . basename($tmpRibbonTarget) . '.rels', $this->getWriterPart('RelsRibbonObjects')->writeRibbonRelationships($this->spreadSheet));
$this->addZipFile($tmpRootPath . '_rels/' . basename($tmpRibbonTarget) . '.rels', $this->getWriterPart('RelsRibbonObjects')->writeRibbonRelationships($this->spreadSheet));
}
}

// Add relationships to ZIP file
$zip->addFile('_rels/.rels', $this->getWriterPart('Rels')->writeRelationships($this->spreadSheet));
$zip->addFile('xl/_rels/workbook.xml.rels', $this->getWriterPart('Rels')->writeWorkbookRelationships($this->spreadSheet));
$this->addZipFile('_rels/.rels', $this->getWriterPart('Rels')->writeRelationships($this->spreadSheet));
$this->addZipFile('xl/_rels/workbook.xml.rels', $this->getWriterPart('Rels')->writeWorkbookRelationships($this->spreadSheet));

// Add document properties to ZIP file
$zip->addFile('docProps/app.xml', $this->getWriterPart('DocProps')->writeDocPropsApp($this->spreadSheet));
$zip->addFile('docProps/core.xml', $this->getWriterPart('DocProps')->writeDocPropsCore($this->spreadSheet));
$this->addZipFile('docProps/app.xml', $this->getWriterPart('DocProps')->writeDocPropsApp($this->spreadSheet));
$this->addZipFile('docProps/core.xml', $this->getWriterPart('DocProps')->writeDocPropsCore($this->spreadSheet));
$customPropertiesPart = $this->getWriterPart('DocProps')->writeDocPropsCustom($this->spreadSheet);
if ($customPropertiesPart !== null) {
$zip->addFile('docProps/custom.xml', $customPropertiesPart);
$this->addZipFile('docProps/custom.xml', $customPropertiesPart);
}

// Add theme to ZIP file
$zip->addFile('xl/theme/theme1.xml', $this->getWriterPart('Theme')->writeTheme($this->spreadSheet));
$this->addZipFile('xl/theme/theme1.xml', $this->getWriterPart('Theme')->writeTheme($this->spreadSheet));

// Add string table to ZIP file
$zip->addFile('xl/sharedStrings.xml', $this->getWriterPart('StringTable')->writeStringTable($this->stringTable));
$this->addZipFile('xl/sharedStrings.xml', $this->getWriterPart('StringTable')->writeStringTable($this->stringTable));

// Add styles to ZIP file
$zip->addFile('xl/styles.xml', $this->getWriterPart('Style')->writeStyles($this->spreadSheet));
$this->addZipFile('xl/styles.xml', $this->getWriterPart('Style')->writeStyles($this->spreadSheet));

// Add workbook to ZIP file
$zip->addFile('xl/workbook.xml', $this->getWriterPart('Workbook')->writeWorkbook($this->spreadSheet, $this->preCalculateFormulas));
$this->addZipFile('xl/workbook.xml', $this->getWriterPart('Workbook')->writeWorkbook($this->spreadSheet, $this->preCalculateFormulas));

$chartCount = 0;
// Add worksheets
for ($i = 0; $i < $this->spreadSheet->getSheetCount(); ++$i) {
$zip->addFile('xl/worksheets/sheet' . ($i + 1) . '.xml', $this->getWriterPart('Worksheet')->writeWorksheet($this->spreadSheet->getSheet($i), $this->stringTable, $this->includeCharts));
$this->addZipFile('xl/worksheets/sheet' . ($i + 1) . '.xml', $this->getWriterPart('Worksheet')->writeWorksheet($this->spreadSheet->getSheet($i), $this->stringTable, $this->includeCharts));
if ($this->includeCharts) {
$charts = $this->spreadSheet->getSheet($i)->getChartCollection();
if (count($charts) > 0) {
foreach ($charts as $chart) {
$zip->addFile('xl/charts/chart' . ($chartCount + 1) . '.xml', $this->getWriterPart('Chart')->writeChart($chart, $this->preCalculateFormulas));
$this->addZipFile('xl/charts/chart' . ($chartCount + 1) . '.xml', $this->getWriterPart('Chart')->writeChart($chart, $this->preCalculateFormulas));
++$chartCount;
}
}
Expand All @@ -280,19 +288,19 @@ public function save($pFilename): void
// Add worksheet relationships (drawings, ...)
for ($i = 0; $i < $this->spreadSheet->getSheetCount(); ++$i) {
// Add relationships
$zip->addFile('xl/worksheets/_rels/sheet' . ($i + 1) . '.xml.rels', $this->getWriterPart('Rels')->writeWorksheetRelationships($this->spreadSheet->getSheet($i), ($i + 1), $this->includeCharts));
$this->addZipFile('xl/worksheets/_rels/sheet' . ($i + 1) . '.xml.rels', $this->getWriterPart('Rels')->writeWorksheetRelationships($this->spreadSheet->getSheet($i), ($i + 1), $this->includeCharts));

// Add unparsedLoadedData
$sheetCodeName = $this->spreadSheet->getSheet($i)->getCodeName();
$unparsedLoadedData = $this->spreadSheet->getUnparsedLoadedData();
if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['ctrlProps'])) {
foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['ctrlProps'] as $ctrlProp) {
$zip->addFile($ctrlProp['filePath'], $ctrlProp['content']);
$this->addZipFile($ctrlProp['filePath'], $ctrlProp['content']);
}
}
if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['printerSettings'])) {
foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['printerSettings'] as $ctrlProp) {
$zip->addFile($ctrlProp['filePath'], $ctrlProp['content']);
$this->addZipFile($ctrlProp['filePath'], $ctrlProp['content']);
}
}

Expand All @@ -305,13 +313,13 @@ public function save($pFilename): void
// Add drawing and image relationship parts
if (($drawingCount > 0) || ($chartCount > 0)) {
// Drawing relationships
$zip->addFile('xl/drawings/_rels/drawing' . ($i + 1) . '.xml.rels', $this->getWriterPart('Rels')->writeDrawingRelationships($this->spreadSheet->getSheet($i), $chartRef1, $this->includeCharts));
$this->addZipFile('xl/drawings/_rels/drawing' . ($i + 1) . '.xml.rels', $this->getWriterPart('Rels')->writeDrawingRelationships($this->spreadSheet->getSheet($i), $chartRef1, $this->includeCharts));

// Drawings
$zip->addFile('xl/drawings/drawing' . ($i + 1) . '.xml', $this->getWriterPart('Drawing')->writeDrawings($this->spreadSheet->getSheet($i), $this->includeCharts));
$this->addZipFile('xl/drawings/drawing' . ($i + 1) . '.xml', $this->getWriterPart('Drawing')->writeDrawings($this->spreadSheet->getSheet($i), $this->includeCharts));
} elseif (isset($unparsedLoadedData['sheets'][$sheetCodeName]['drawingAlternateContents'])) {
// Drawings
$zip->addFile('xl/drawings/drawing' . ($i + 1) . '.xml', $this->getWriterPart('Drawing')->writeDrawings($this->spreadSheet->getSheet($i), $this->includeCharts));
$this->addZipFile('xl/drawings/drawing' . ($i + 1) . '.xml', $this->getWriterPart('Drawing')->writeDrawings($this->spreadSheet->getSheet($i), $this->includeCharts));
}

// Add unparsed drawings
Expand All @@ -320,38 +328,38 @@ public function save($pFilename): void
$drawingFile = array_search($relId, $unparsedLoadedData['sheets'][$sheetCodeName]['drawingOriginalIds']);
if ($drawingFile !== false) {
$drawingFile = ltrim($drawingFile, '.');
$zip->addFile('xl' . $drawingFile, $drawingXml);
$this->addZipFile('xl' . $drawingFile, $drawingXml);
}
}
}

// Add comment relationship parts
if (count($this->spreadSheet->getSheet($i)->getComments()) > 0) {
// VML Comments
$zip->addFile('xl/drawings/vmlDrawing' . ($i + 1) . '.vml', $this->getWriterPart('Comments')->writeVMLComments($this->spreadSheet->getSheet($i)));
$this->addZipFile('xl/drawings/vmlDrawing' . ($i + 1) . '.vml', $this->getWriterPart('Comments')->writeVMLComments($this->spreadSheet->getSheet($i)));

// Comments
$zip->addFile('xl/comments' . ($i + 1) . '.xml', $this->getWriterPart('Comments')->writeComments($this->spreadSheet->getSheet($i)));
$this->addZipFile('xl/comments' . ($i + 1) . '.xml', $this->getWriterPart('Comments')->writeComments($this->spreadSheet->getSheet($i)));
}

// Add unparsed relationship parts
if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['vmlDrawings'])) {
foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['vmlDrawings'] as $vmlDrawing) {
$zip->addFile($vmlDrawing['filePath'], $vmlDrawing['content']);
$this->addZipFile($vmlDrawing['filePath'], $vmlDrawing['content']);
}
}

// Add header/footer relationship parts
if (count($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages()) > 0) {
// VML Drawings
$zip->addFile('xl/drawings/vmlDrawingHF' . ($i + 1) . '.vml', $this->getWriterPart('Drawing')->writeVMLHeaderFooterImages($this->spreadSheet->getSheet($i)));
$this->addZipFile('xl/drawings/vmlDrawingHF' . ($i + 1) . '.vml', $this->getWriterPart('Drawing')->writeVMLHeaderFooterImages($this->spreadSheet->getSheet($i)));

// VML Drawing relationships
$zip->addFile('xl/drawings/_rels/vmlDrawingHF' . ($i + 1) . '.vml.rels', $this->getWriterPart('Rels')->writeHeaderFooterDrawingRelationships($this->spreadSheet->getSheet($i)));
$this->addZipFile('xl/drawings/_rels/vmlDrawingHF' . ($i + 1) . '.vml.rels', $this->getWriterPart('Rels')->writeHeaderFooterDrawingRelationships($this->spreadSheet->getSheet($i)));

// Media
foreach ($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
$zip->addFile('xl/media/' . $image->getIndexedFilename(), file_get_contents($image->getPath()));
$this->addZipFile('xl/media/' . $image->getIndexedFilename(), file_get_contents($image->getPath()));
}
}
}
Expand All @@ -374,7 +382,7 @@ public function save($pFilename): void
$imageContents = file_get_contents($imagePath);
}

$zip->addFile('xl/media/' . str_replace(' ', '_', $this->getDrawingHashTable()->getByIndex($i)->getIndexedFilename()), $imageContents);
$this->addZipFile('xl/media/' . str_replace(' ', '_', $this->getDrawingHashTable()->getByIndex($i)->getIndexedFilename()), $imageContents);
} elseif ($this->getDrawingHashTable()->getByIndex($i) instanceof MemoryDrawing) {
ob_start();
call_user_func(
Expand All @@ -384,7 +392,7 @@ public function save($pFilename): void
$imageContents = ob_get_contents();
ob_end_clean();

$zip->addFile('xl/media/' . str_replace(' ', '_', $this->getDrawingHashTable()->getByIndex($i)->getIndexedFilename()), $imageContents);
$this->addZipFile('xl/media/' . str_replace(' ', '_', $this->getDrawingHashTable()->getByIndex($i)->getIndexedFilename()), $imageContents);
}
}

Expand All @@ -393,7 +401,7 @@ public function save($pFilename): void

// Close file
try {
$zip->finish();
$this->zip->finish();
} catch (OverflowException $e) {
throw new WriterException('Could not close resource.');
}
Expand Down Expand Up @@ -535,4 +543,14 @@ public function setOffice2003Compatibility($pValue)

return $this;
}

private $pathNames = [];

private function addZipFile(string $path, string $content): void
{
if (!in_array($path, $this->pathNames)) {
$this->pathNames[] = $path;
$this->zip->addFile($path, $content);
}
}
}
Loading

0 comments on commit 82ea1d5

Please sign in to comment.