alex
/
PhpSpreadsheet
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix for #1516 (#1530) 

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.
This commit is contained in:
oleibman 2020-06-19 11:26:02 -07:00 committed by GitHub
parent 12dd92bafe
commit 82ea1d5596
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 154 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
src/PhpSpreadsheet/Writer/Xlsx.php
View File

@ -107,6 +107,13 @@ class Xlsx extends BaseWriter
*/
private $drawingHashTable;
/**
* Private handle for zip stream.
*
* @var ZipStream
*/
private $zip;
/**
* Create a new Xlsx Writer.
*/
@ -173,6 +180,7 @@ class Xlsx extends BaseWriter
{
if ($this->spreadSheet !== null) {
// garbage collect
$this->pathNames = [];
$this->spreadSheet->garbageCollect();
$this->openFileHandle($pFilename);
@ -203,73 +211,73 @@ class Xlsx extends BaseWriter
$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;
}
}
@ -280,19 +288,19 @@ class Xlsx extends BaseWriter
// 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']);
}
}
@ -305,13 +313,13 @@ class Xlsx extends BaseWriter
// 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
@ -320,7 +328,7 @@ class Xlsx extends BaseWriter
$drawingFile = array_search($relId, $unparsedLoadedData['sheets'][$sheetCodeName]['drawingOriginalIds']);
if ($drawingFile !== false) {
$drawingFile = ltrim($drawingFile, '.');
$zip->addFile('xl' . $drawingFile, $drawingXml);
$this->addZipFile('xl' . $drawingFile, $drawingXml);
}
}
}
@ -328,30 +336,30 @@ class Xlsx extends BaseWriter
// 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()));
}
}
}
@ -374,7 +382,7 @@ class Xlsx extends BaseWriter
$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(
@ -384,7 +392,7 @@ class Xlsx extends BaseWriter
$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);
}
}
@ -393,7 +401,7 @@ class Xlsx extends BaseWriter
// Close file
try {
$zip->finish();
$this->zip->finish();
} catch (OverflowException $e) {
throw new WriterException('Could not close resource.');
}
@ -535,4 +543,14 @@ class Xlsx extends BaseWriter
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);
}
}
}

101
tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php Normal file
View File

@ -0,0 +1,101 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
use PhpOffice\PhpSpreadsheet\Settings;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PHPUnit\Framework\TestCase;
class UnparsedDataCloneTest extends TestCase
{
/**
* Test load and save Xlsx file with unparsed data (form elements, protected sheets, alternate contents, printer settings,..).
*/
public function testLoadSaveXlsxWithUnparsedDataClone(): void
{
$sampleFilename = 'tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx';
$resultFilename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test');
Settings::setLibXmlLoaderOptions(null); // reset to default options
$reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
$spreadsheet = $reader->load($sampleFilename);
$spreadsheet->setActiveSheetIndex(1);
$sheet = $spreadsheet->getActiveSheet();
$drawings = $sheet->getDrawingCollection();
self::assertCount(1, $drawings);
$sheetCodeName = $sheet->getCodeName();
$unparsedLoadedData = $spreadsheet->getUnparsedLoadedData();
self::assertArrayHasKey('printerSettings', $unparsedLoadedData['sheets'][$sheetCodeName]);
self::assertCount(1, $unparsedLoadedData['sheets'][$sheetCodeName]['printerSettings']);
$clonedSheet = clone $spreadsheet->getActiveSheet();
$clonedSheet->setTitle('Clone');
$spreadsheet->addSheet($clonedSheet);
$writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($spreadsheet);
$writer->save($resultFilename);
$dupname = 'Unable to open saved file';
$zip = zip_open($resultFilename);
if (is_resource($zip)) {
$names = [];
$dupname = '';
while ($zip_entry = zip_read($zip)) {
$zipname = zip_entry_name($zip_entry);
if (in_array($zipname, $names)) {
$dupname .= "$zipname,";
} else {
$names[] = $zipname;
}
}
zip_close($zip);
}
unlink($resultFilename);
self::assertEquals('', $dupname);
}
/**
* Test that saving twice with same writer works.
*/
public function testSaveTwice(): void
{
$sampleFilename = 'tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx';
$resultFilename1 = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test1');
$resultFilename2 = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test2');
self::assertNotEquals($resultFilename1, $resultFilename2);
Settings::setLibXmlLoaderOptions(null); // reset to default options
$reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
$spreadsheet = $reader->load($sampleFilename);
$sheet = $spreadsheet->setActiveSheetIndex(1);
$sheet->setTitle('Original');
$clonedSheet = clone $spreadsheet->getActiveSheet();
$clonedSheet->setTitle('Clone');
$spreadsheet->addSheet($clonedSheet);
$clonedSheet->getCell('A8')->setValue('cloned');
$sheet->getCell('A8')->setValue('original');
$writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($spreadsheet);
$writer->save($resultFilename1);
$reader1 = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
$spreadsheet1 = $reader1->load($resultFilename1);
unlink($resultFilename1);
$sheet1c = $spreadsheet1->getSheetByName('Clone');
$sheet1o = $spreadsheet1->getSheetByName('Original');
$writer->save($resultFilename2);
$reader2 = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx();
$spreadsheet2 = $reader2->load($resultFilename2);
unlink($resultFilename2);
$sheet2c = $spreadsheet2->getSheetByName('Clone');
$sheet2o = $spreadsheet2->getSheetByName('Original');
self::assertEquals($spreadsheet1->getSheetCount(), $spreadsheet2->getSheetCount());
self::assertCount(1, $sheet1c->getDrawingCollection());
self::assertCount(1, $sheet1o->getDrawingCollection());
self::assertCount(1, $sheet2c->getDrawingCollection());
self::assertCount(1, $sheet2o->getDrawingCollection());
self::assertEquals('original', $sheet1o->getCell('A8')->getValue());
self::assertEquals('original', $sheet2o->getCell('A8')->getValue());
self::assertEquals('cloned', $sheet1c->getCell('A8')->getValue());
self::assertEquals('cloned', $sheet2c->getCell('A8')->getValue());
}
}