geckon01/PhpSpreadsheet
1
0
Fork 0
mirror of https://github.com/PHPOffice/PhpSpreadsheet.git synced 2026-08-30 12:07:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix Xlsx Read Ignoring Comments (#3655)

Browse Source 
* Fix Xlsx Read Ignoring Comments

Fix #3654. Several places in Reader Xlsx make a truthy test for `$zip->locateName()`. However, zero is a legitimate result which should not be treated as false. Change all existing tests in this form to test for (in)equality to false. For the record, there is no existing exposure of this kind in Reader Ods.

* Absolute Path In Printer Settings File

Another unexpected use of an absolute path in a rels file. Easily fixed, but the test file is 7+ MB. I have asked the problem reporter to try to find a smaller file.

* Reduced Size of Test File

It was 7.8MB. I manipulated the file to replace all the comment backgrounds in column AN with a single image, much smaller than the existing ones. This reduced the file size by over 6MB. It's still larger than I'd like, but might be acceptable. (AN121 uses a different background than the others just so that I could check that they would all be copied faithfully.)
This commit is contained in:
oleibman
2023-08-21 23:36:29 -07:00
committed by GitHub
parent dcf7415762
commit b879cdd973
4 changed files with 59 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PhpSpreadsheet/Reader/Xlsx.php
+12 -11
View File
@@ -1049,7 +1049,7 @@ class Xlsx extends BaseReader
$hyperlinkReader = new Hyperlinks($docSheet);
// Locate hyperlink relations
$relationsFileName = dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels';
if ($zip->locateName($relationsFileName)) {
if ($zip->locateName($relationsFileName) !== false) {
$relsWorksheet = $this->loadZip($relationsFileName, Namespaces::RELATIONSHIPS);
$hyperlinkReader->readHyperlinks($relsWorksheet);
}
@@ -1066,7 +1066,7 @@ class Xlsx extends BaseReader
if (!$this->readDataOnly) {
// Locate comment relations
$commentRelations = dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels';
if ($zip->locateName($commentRelations)) {
if ($zip->locateName($commentRelations) !== false) {
$relsWorksheet = $this->loadZip($commentRelations, Namespaces::RELATIONSHIPS);
foreach ($relsWorksheet->Relationship as $elex) {
$ele = self::getAttributes($elex);
@@ -1128,7 +1128,7 @@ class Xlsx extends BaseReader
$drowingImages = [];
$VMLDrawingsRelations = dirname($relPath) . '/_rels/' . basename($relPath) . '.rels';
$vmlDrawingContents[$relName] = $this->getSecurityScannerOrThrow()->scan($this->getFromZipArchive($zip, $relPath));
if ($zip->locateName($VMLDrawingsRelations)) {
if ($zip->locateName($VMLDrawingsRelations) !== false) {
$relsVMLDrawing = $this->loadZip($VMLDrawingsRelations, Namespaces::RELATIONSHIPS);
foreach ($relsVMLDrawing->Relationship as $elex) {
$ele = self::getAttributes($elex);
@@ -1192,7 +1192,7 @@ class Xlsx extends BaseReader
if (isset($drowingImages[$fillImageRelId])) {
$objDrawing = new \PhpOffice\PhpSpreadsheet\Worksheet\Drawing();
$objDrawing->setName($fillImageTitle);
$imagePath = str_replace('../', 'xl/', $drowingImages[$fillImageRelId]);
$imagePath = str_replace(['../', '/xl/'], 'xl/', $drowingImages[$fillImageRelId]);
$objDrawing->setPath(
'zip://' . File::realpath($filename) . '#' . $imagePath,
true,
@@ -1249,7 +1249,7 @@ class Xlsx extends BaseReader
if ($vmlHfRidAttr !== null && isset($vmlHfRidAttr['id'])) {
$vmlHfRid = (string) $vmlHfRidAttr['id'][0];
}
if ($zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels')) {
if ($zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels') !== false) {
$relsWorksheet = $this->loadZipNoNamespace(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels', Namespaces::RELATIONSHIPS);
$vmlRelationship = '';
@@ -1328,7 +1328,7 @@ class Xlsx extends BaseReader
if (substr($drawingFilename, 0, 8) === '/xl//xl/') {
$drawingFilename = substr($drawingFilename, 5);
}
if ($zip->locateName($drawingFilename)) {
if ($zip->locateName($drawingFilename) !== false) {
$relsWorksheet = $this->loadZipNoNamespace($drawingFilename, Namespaces::RELATIONSHIPS);
$drawings = [];
foreach ($relsWorksheet->Relationship as $ele) {
@@ -2095,7 +2095,7 @@ class Xlsx extends BaseReader
private function readFormControlProperties(Spreadsheet $excel, string $dir, string $fileWorksheet, Worksheet $docSheet, array &$unparsedLoadedData): void
{
$zip = $this->zip;
if (!$zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels')) {
if ($zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels') === false) {
return;
}
@@ -2122,7 +2122,7 @@ class Xlsx extends BaseReader
private function readPrinterSettings(Spreadsheet $excel, string $dir, string $fileWorksheet, Worksheet $docSheet, array &$unparsedLoadedData): void
{
$zip = $this->zip;
if (!$zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels')) {
if ($zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels') === false) {
return;
}
@@ -2142,8 +2142,9 @@ class Xlsx extends BaseReader
$rId = $rId . 'ps'; // rIdXXX, add 'ps' suffix to avoid identical resource identifier collision with unparsed vmlDrawing
}
$unparsedPrinterSettings[$rId] = [];
$unparsedPrinterSettings[$rId]['filePath'] = self::dirAdd("$dir/$fileWorksheet", $printerSettings['Target']);
$unparsedPrinterSettings[$rId]['relFilePath'] = (string) $printerSettings['Target'];
$target = (string) str_replace('/xl/', '../', (string) $printerSettings['Target']);
$unparsedPrinterSettings[$rId]['filePath'] = self::dirAdd("$dir/$fileWorksheet", $target);
$unparsedPrinterSettings[$rId]['relFilePath'] = $target;
$unparsedPrinterSettings[$rId]['content'] = $this->getSecurityScannerOrThrow()->scan($this->getFromZipArchive($zip, $unparsedPrinterSettings[$rId]['filePath']));
}
unset($unparsedPrinterSettings);
@@ -2238,7 +2239,7 @@ class Xlsx extends BaseReader
$tablePartRel = (string) $relation['id'];
$relationsFileName = dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels';
if ($zip->locateName($relationsFileName)) {
if ($zip->locateName($relationsFileName) !== false) {
$relsTableReferences = $this->loadZip($relationsFileName, Namespaces::RELATIONSHIPS);
foreach ($relsTableReferences->Relationship as $relationship) {
$relationshipAttributes = self::getAttributes($relationship, '');
tests/PhpSpreadsheetTests/Reader/Xlsx/CommentTest.php
+47
View File
@@ -23,4 +23,51 @@ class CommentTest extends TestCase
self::assertStringContainsString('若為宅配物流僅能選「純配送」', $commentString);
self::assertSame('Anderson Chen 陳宗棠', $comment->getAuthor());
}
public function testIssue3654(): void
{
// Reader was ignoring comments.
$filename = 'tests/data/Reader/XLSX/issue.3654.xlsx';
$reader = new Xlsx();
$spreadsheet = $reader->load($filename);
$sheet = $spreadsheet->getActiveSheet();
$expectedComments = [
'X4', 'AD4', 'AN4',
'X5', 'AD5', 'AN5',
'AN6', // X6 and AD6 are uncommented on original
'X7', 'AD7', 'AN7',
'X8', 'AD8', 'AN8',
'X9', 'AD9', 'AN9',
'X10', 'AD10', 'AN10',
'X11', 'AD11', 'AN11',
'X12', 'AD12', 'AN12',
];
self::assertEquals($expectedComments, array_keys($sheet->getComments()));
self::assertStringContainsString('.png', $sheet->getComment('X4')->getBackgroundImage()->getPath());
self::assertStringContainsString('.jpeg', $sheet->getComment('AN12')->getBackgroundImage()->getPath());
$spreadsheet->disconnectWorksheets();
}
public function testIssue3654c(): void
{
// Reader was ignoring comments.
$filename = 'tests/data/Reader/XLSX/issue.3654c.xlsx';
$reader = new Xlsx();
$spreadsheet = $reader->load($filename);
$sheet = $spreadsheet->getActiveSheet();
$comments = $sheet->getComments();
self::assertCount(326, $comments);
self::assertStringContainsString('.png', $sheet->getComment('X4')->getBackgroundImage()->getPath());
self::assertStringContainsString('.jpeg', $sheet->getComment('AN12')->getBackgroundImage()->getPath());
// Spreadsheet generated by Flexcel. Make sure we managed
// to locate printerSettings since it used an unexpected link.
$pageSetupRel = $spreadsheet->getUnparsedLoadedData()['sheets']['Worksheet']['pageSetupRelId'] ?? '';
self::assertSame('flId1ps', $pageSetupRel);
$pageSetupPath = $spreadsheet->getUnparsedLoadedData()['sheets']['Worksheet']['printerSettings'][substr($pageSetupRel, 3)]['filePath'] ?? '';
self::assertSame('xl/printerSettings/printerSettings1.bin', $pageSetupPath);
$spreadsheet->disconnectWorksheets();
}
}
tests/data/Reader/XLSX/issue.3654.xlsx
BIN
View File
Binary file not shown.
tests/data/Reader/XLSX/issue.3654c.xlsx
BIN
View File
Binary file not shown.