Skip to content

Commit

Permalink
Generation3 Copy With Image in Footer
Browse files Browse the repository at this point in the history 
Fix PHPOffice#3126. A worksheet contained an image in its footer. It could be loaded and saved as another spreadsheet. However, if you tried to load and save that spreadsheet, PhpSpreadsheet would be unable to find the footer image and would therefore throw an exception.

This error was introduced a long time ago, in PhpSpreadsheet 1.3.0. The apparent cause of the problem was PR PHPOffice#435, sometime around June 2018. That change was very useful, but it had problems which exposed themselves only with a third generation copy. An additional contributor to the issue at hand was PR PHPOffice#1690 (December 2020), which again exposed itself with a third generation copy.

The issue from 1690 is easier to explain and deal with. It added a 'ps' suffix to printer settings resources in Xlsx Reader (to avoid name conflicts), but did not limit itself to a single addition (so subseqent generations would have multiple ps's). It also neglected to add the suffix in Reader/Xlsx/PageSetup.

As for 435, it loops through all the worksheet relationships, and uses the last that it finds as the base for header/footer drawings. It has been changed to use only the relationship whose `rId` matches the worksheet's `legacyDrawingHF` `rId`. It also needs a bit extra validation to make sure a drawing exists before adding it to its array of header/footer images. It also meant that Xlsx/Writer/Rels might write an entry with the same rId twice. I have also changed the header/footer image processing to be namespace aware (see PR PHPOffice#3137).
  • Loading branch information
@oleibman
oleibman committed Oct 25, 2022
1 parent 88bbac9 commit 9be666c
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 22 deletions.
43 changes: 28 additions & 15 deletions src/PhpSpreadsheet/Reader/Xlsx.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,14 +1155,21 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
}

// Header/footer images
if ($xmlSheet && $xmlSheet->legacyDrawingHF) {
if ($xmlSheetNS && $xmlSheetNS->legacyDrawingHF) {
$vmlHfRid = '';
$vmlHfRidAttr = $xmlSheetNS->legacyDrawingHF->attributes(Namespaces::SCHEMA_OFFICE_DOCUMENT);
if ($vmlHfRidAttr !== null && isset($vmlHfRidAttr['id'])) {
$vmlHfRid = (string) $vmlHfRidAttr['id'][0];
}
if ($zip->locateName(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels')) {
$relsWorksheet = $this->loadZipNoNamespace(dirname("$dir/$fileWorksheet") . '/_rels/' . basename($fileWorksheet) . '.rels', Namespaces::RELATIONSHIPS);
$vmlRelationship = '';

foreach ($relsWorksheet->Relationship as $ele) {
if ($ele['Type'] == Namespaces::VML) {
if ((string) $ele['Type'] == Namespaces::VML && (string) $ele['Id'] === $vmlHfRid) {
$vmlRelationship = self::dirAdd("$dir/$fileWorksheet", $ele['Target']);

break;
}
}

Expand Down Expand Up @@ -1197,20 +1204,23 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$imageData = self::getAttributes($imageData, Namespaces::URN_MSOFFICE);
$style = self::toCSSArray((string) $shape['style']);

$hfImages[(string) $shape['id']] = new HeaderFooterDrawing();
if (isset($imageData['title'])) {
$hfImages[(string) $shape['id']]->setName((string) $imageData['title']);
}
if (array_key_exists((string) $imageData['relid'], $drawings)) {
$shapeId = (string) $shape['id'];
$hfImages[$shapeId] = new HeaderFooterDrawing();
if (isset($imageData['title'])) {
$hfImages[$shapeId]->setName((string) $imageData['title']);
}

$hfImages[(string) $shape['id']]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false);
$hfImages[(string) $shape['id']]->setResizeProportional(false);
$hfImages[(string) $shape['id']]->setWidth($style['width']);
$hfImages[(string) $shape['id']]->setHeight($style['height']);
if (isset($style['margin-left'])) {
$hfImages[(string) $shape['id']]->setOffsetX($style['margin-left']);
$hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false);
$hfImages[$shapeId]->setResizeProportional(false);
$hfImages[$shapeId]->setWidth($style['width']);
$hfImages[$shapeId]->setHeight($style['height']);
if (isset($style['margin-left'])) {
$hfImages[$shapeId]->setOffsetX($style['margin-left']);
}
$hfImages[$shapeId]->setOffsetY($style['margin-top']);
$hfImages[$shapeId]->setResizeProportional(true);
}
$hfImages[(string) $shape['id']]->setOffsetY($style['margin-top']);
$hfImages[(string) $shape['id']]->setResizeProportional(true);
}

$docSheet->getHeaderFooter()->setImages($hfImages);
Expand Down Expand Up @@ -2028,7 +2038,10 @@ private function readPrinterSettings(Spreadsheet $excel, string $dir, string $fi

$unparsedPrinterSettings = &$unparsedLoadedData['sheets'][$docSheet->getCodeName()]['printerSettings'];
foreach ($sheetPrinterSettings as $rId => $printerSettings) {
$rId = substr($rId, 3) . 'ps'; // rIdXXX, add 'ps' suffix to avoid identical resource identifier collision with unparsed vmlDrawing
$rId = substr($rId, 3); // rIdXXX
if (substr($rId, -2) !== 'ps') {
$rId = substr($rId, 3) . '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'];
Expand Down
6 changes: 5 additions & 1 deletion src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ private function pageSetup(SimpleXMLElement $xmlSheet, Worksheet $worksheet, arr

$relAttributes = $xmlSheet->pageSetup->attributes(Namespaces::SCHEMA_OFFICE_DOCUMENT);
if (isset($relAttributes['id'])) {
$unparsedLoadedData['sheets'][$worksheet->getCodeName()]['pageSetupRelId'] = (string) $relAttributes['id'];
$relid = (string) $relAttributes['id'];
if (substr($relid, -2) !== 'ps') {
$relid .= 'ps';
}
$unparsedLoadedData['sheets'][$worksheet->getCodeName()]['pageSetupRelId'] = $relid;
}
}

Expand Down
14 changes: 8 additions & 6 deletions src/PhpSpreadsheet/Writer/Xlsx/Rels.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,14 @@ private function writeUnparsedRelationship(\PhpOffice\PhpSpreadsheet\Worksheet\W
}

foreach ($unparsedLoadedData['sheets'][$worksheet->getCodeName()][$relationship] as $rId => $value) {
$this->writeRelationship(
$objWriter,
$rId,
$type,
$value['relFilePath']
);
if (substr($rId, 0, 17) !== '_headerfooter_vml') {
$this->writeRelationship(
$objWriter,
$rId,
$type,
$value['relFilePath']
);
}
}
}

Expand Down
66 changes: 66 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3126Test.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
use PHPUnit\Framework\TestCase;

class Issue3126Test extends TestCase
{
public function testReloadXlsxWorkbookProperties(): void
{
$filename = 'tests/data/Reader/XLSX/issue.3126.xlsx';
$reader = new XlsxReader();
$generation1 = $reader->load($filename);
$gen1sheet = $generation1->getActiveSheet();
$gen1hf = $gen1sheet->getHeaderFooter();
$gen1Images = $gen1hf->getImages();
self::assertCount(1, $gen1Images);
$gen1hfName = array_key_exists('LF', $gen1Images) ? $gen1Images['LF']->getName() : '';
self::assertSame('fleche-verte-up-right', $gen1hfName);
$pageSetupRel = $generation1->getUnparsedLoadedData()['sheets']['Worksheet']['pageSetupRelId'] ?? '';
self::assertSame('rId1ps', $pageSetupRel);
$pageSetupPath = $generation1->getUnparsedLoadedData()['sheets']['Worksheet']['printerSettings']['ps']['filePath'] ?? '';
self::assertSame('xl/printerSettings/printerSettings1.bin', $pageSetupPath);

$generation2Name = File::temporaryFilename();
$writer = new XlsxWriter($generation1);
$writer->save($generation2Name);
$generation1->disconnectWorksheets();
$reader2 = new XlsxReader();
$generation2 = $reader2->load($generation2Name);
$gen2sheet = $generation2->getActiveSheet();
$gen2hf = $gen2sheet->getHeaderFooter();
$gen2Images = $gen2hf->getImages();
self::assertCount(1, $gen2Images);
$gen2hfName = array_key_exists('LF', $gen2Images) ? $gen2Images['LF']->getName() : '';
self::assertSame('fleche-verte-up-right', $gen2hfName);
$pageSetupRel = $generation2->getUnparsedLoadedData()['sheets']['Worksheet']['pageSetupRelId'] ?? '';
self::assertSame('rId1ps', $pageSetupRel);
$pageSetupPath = $generation2->getUnparsedLoadedData()['sheets']['Worksheet']['printerSettings']['ps']['filePath'] ?? '';
self::assertSame('xl/printerSettings/printerSettings1.bin', $pageSetupPath);

$generation3Name = File::temporaryFilename();
$writer = new XlsxWriter($generation2);
$writer->save($generation3Name);
$generation2->disconnectWorksheets();
$reader3 = new XlsxReader();
$generation3 = $reader3->load($generation3Name);
$gen3sheet = $generation3->getActiveSheet();
$gen3hf = $gen3sheet->getHeaderFooter();
$gen3Images = $gen3hf->getImages();
self::assertCount(1, $gen3Images);
$gen3hfName = array_key_exists('LF', $gen3Images) ? $gen3Images['LF']->getName() : '';
self::assertSame('fleche-verte-up-right', $gen3hfName);
$pageSetupRel = $generation3->getUnparsedLoadedData()['sheets']['Worksheet']['pageSetupRelId'] ?? '';
self::assertSame('rId1ps', $pageSetupRel);
$pageSetupPath = $generation3->getUnparsedLoadedData()['sheets']['Worksheet']['printerSettings']['ps']['filePath'] ?? '';
self::assertSame('xl/printerSettings/printerSettings1.bin', $pageSetupPath);

unlink($generation2Name);
unlink($generation3Name);
$generation3->disconnectWorksheets();
}
}
Binary file added tests/data/Reader/XLSX/issue.3126.xlsx
View file
Binary file not shown.

0 comments on commit 9be666c

Please sign in to comment.