Skip to content

Commit 4b8a929

Browse files
oleibmanoleibman
authored
Merge commit from fork
* Validate Mime Type for Images * Extend Change to Xlsx Reader * Attach Drawing to Sheet Only If Valid * Suppress Some Theoretical Warning Messages * Minor Tweak
1 parent c91c8c9 commit 4b8a929

File tree

12 files changed

+211
-40
lines changed

12 files changed

+211
-40
lines changed

‎src/PhpSpreadsheet/Reader/Xlsx.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
13291329
$hfImages[$shapeId]->setName((string) $imageData['title']);
13301330
}
13311331

1332-
$hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false);
1332+
$hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false, $zip);
13331333
$hfImages[$shapeId]->setResizeProportional(false);
13341334
$hfImages[$shapeId]->setWidth($style['width']);
13351335
$hfImages[$shapeId]->setHeight($style['height']);
@@ -1441,7 +1441,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
14411441
$objDrawing->setPath(
14421442
'zip://' . File::realpath($filename) . '#'
14431443
. $images[$embedImageKey],
1444-
false
1444+
false,
1445+
$zip
14451446
);
14461447
} else {
14471448
$linkImageKey = (string) self::getArrayItem(
@@ -1452,6 +1453,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
14521453
$url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
14531454
$objDrawing->setPath($url);
14541455
}
1456+
if ($objDrawing->getPath() === '') {
1457+
continue;
1458+
}
14551459
}
14561460
$objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $oneCellAnchor->from->col) + 1) . ($oneCellAnchor->from->row + 1));
14571461

@@ -1530,7 +1534,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
15301534
$objDrawing->setPath(
15311535
'zip://' . File::realpath($filename) . '#'
15321536
. $images[$embedImageKey],
1533-
false
1537+
false,
1538+
$zip
15341539
);
15351540
} else {
15361541
$linkImageKey = (string) self::getArrayItem(
@@ -1541,6 +1546,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
15411546
$url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
15421547
$objDrawing->setPath($url);
15431548
}
1549+
if ($objDrawing->getPath() === '') {
1550+
continue;
1551+
}
15441552
}
15451553
$objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $twoCellAnchor->from->col) + 1) . ($twoCellAnchor->from->row + 1));
15461554

‎src/PhpSpreadsheet/Worksheet/BaseDrawing.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = f
192192
{
193193
if ($this->worksheet === null) {
194194
// Add drawing to \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
195-
if ($worksheet !== null) {
195+
if ($worksheet !== null && !($this instanceof Drawing && $this->getPath() === '')) {
196196
$this->worksheet = $worksheet;
197197
$this->worksheet->getCell($this->coordinates);
198198
$this->worksheet->getDrawingCollection()->append($this);

‎src/PhpSpreadsheet/Worksheet/Drawing.php‎

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -94,40 +94,72 @@ public function getPath(): string
9494
*/
9595
public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null): static
9696
{
97-
if ($verifyFile && preg_match('~^data:image/[a-z]+;base64,~', $path) !== 1) {
98-
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
99-
if (filter_var($path, FILTER_VALIDATE_URL)) {
100-
$this->path = $path;
101-
// Implicit that it is a URL, rather store info than running check above on value in other places.
102-
$this->isUrl = true;
103-
$imageContents = file_get_contents($path);
97+
$this->isUrl = false;
98+
if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {
99+
$this->path = $path;
100+
101+
return $this;
102+
}
103+
104+
$this->path = '';
105+
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
106+
if (filter_var($path, FILTER_VALIDATE_URL)) {
107+
if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
108+
throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
109+
}
110+
// Implicit that it is a URL, rather store info than running check above on value in other places.
111+
$this->isUrl = true;
112+
$imageContents = @file_get_contents($path);
113+
if ($imageContents !== false) {
104114
$filePath = tempnam(sys_get_temp_dir(), 'Drawing');
105115
if ($filePath) {
106-
file_put_contents($filePath, $imageContents);
107-
if (file_exists($filePath)) {
108-
$this->setSizesAndType($filePath);
116+
$put = @file_put_contents($filePath, $imageContents);
117+
if ($put !== false) {
118+
if ($this->isImage($filePath)) {
119+
$this->path = $path;
120+
$this->setSizesAndType($filePath);
121+
}
109122
unlink($filePath);
110123
}
111124
}
112-
} elseif (file_exists($path)) {
113-
$this->path = $path;
114-
$this->setSizesAndType($path);
115-
} elseif ($zip instanceof ZipArchive) {
116-
$zipPath = explode('#', $path)[1];
117-
if ($zip->locateName($zipPath) !== false) {
125+
}
126+
} elseif ($zip instanceof ZipArchive) {
127+
$zipPath = explode('#', $path)[1];
128+
$locate = @$zip->locateName($zipPath);
129+
if ($locate !== false) {
130+
if ($this->isImage($path)) {
118131
$this->path = $path;
119132
$this->setSizesAndType($path);
120133
}
121-
} else {
122-
throw new PhpSpreadsheetException("File $path not found!");
123134
}
124135
} else {
125-
$this->path = $path;
136+
$exists = @file_exists($path);
137+
if ($exists !== false && $this->isImage($path)) {
138+
$this->path = $path;
139+
$this->setSizesAndType($path);
140+
}
141+
}
142+
if ($this->path === '' && $verifyFile) {
143+
throw new PhpSpreadsheetException("File $path not found!");
126144
}
127145

128146
return $this;
129147
}
130148

149+
private function isImage(string $path): bool
150+
{
151+
$mime = (string) @mime_content_type($path);
152+
$retVal = false;
153+
if (str_starts_with($mime, 'image/')) {
154+
$retVal = true;
155+
} elseif ($mime === 'application/octet-stream') {
156+
$extension = pathinfo($path, PATHINFO_EXTENSION);
157+
$retVal = in_array($extension, ['bin', 'emf'], true);
158+
}
159+
160+
return $retVal;
161+
}
162+
131163
/**
132164
* Get isURL.
133165
*/

‎src/PhpSpreadsheet/Writer/Html.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,9 @@ private function extendRowsAndColumns(Worksheet $worksheet, int &$colMax, int &$
579579
}
580580
}
581581
foreach ($worksheet->getDrawingCollection() as $drawing) {
582+
if ($drawing instanceof Drawing && $drawing->getPath() === '') {
583+
continue;
584+
}
582585
$imageTL = Coordinate::indexesFromString($drawing->getCoordinates());
583586
$this->sheetDrawings[$drawing->getCoordinates()] = $drawing;
584587
if ($imageTL[1] > $rowMax) {
@@ -621,7 +624,7 @@ private function writeImageInCell(string $coordinates): string
621624
if ($drawing !== null) {
622625
$filedesc = $drawing->getDescription();
623626
$filedesc = $filedesc ? htmlspecialchars($filedesc, ENT_QUOTES) : 'Embedded image';
624-
if ($drawing instanceof Drawing) {
627+
if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
625628
$filename = $drawing->getPath();
626629

627630
// Strip off eventual '.'
@@ -640,12 +643,15 @@ private function writeImageInCell(string $coordinates): string
640643
$imageData = self::winFileToUrl($filename, $this instanceof Pdf\Mpdf);
641644

642645
if ($this->embedImages || str_starts_with($imageData, 'zip://')) {
646+
$imageData = 'data:,';
643647
$picture = @file_get_contents($filename);
644648
if ($picture !== false) {
645-
$imageDetails = getimagesize($filename) ?: ['mime' => ''];
646-
// base64 encode the binary data
647-
$base64 = base64_encode($picture);
648-
$imageData = 'data:' . $imageDetails['mime'] . ';base64,' . $base64;
649+
$mimeContentType = (string) @mime_content_type($filename);
650+
if (str_starts_with($mimeContentType, 'image/')) {
651+
// base64 encode the binary data
652+
$base64 = base64_encode($picture);
653+
$imageData = 'data:' . $mimeContentType . ';base64,' . $base64;
654+
}
649655
}
650656
}
651657

‎src/PhpSpreadsheet/Writer/Xls.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ private function processDrawing(BstoreContainer &$bstoreContainer, Drawing $draw
467467

468468
private function processBaseDrawing(BstoreContainer &$bstoreContainer, BaseDrawing $drawing): void
469469
{
470-
if ($drawing instanceof Drawing) {
470+
if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
471471
$this->processDrawing($bstoreContainer, $drawing);
472472
} elseif ($drawing instanceof MemoryDrawing) {
473473
$this->processMemoryDrawing($bstoreContainer, $drawing, $drawing->getRenderingFunction());

‎src/PhpSpreadsheet/Writer/Xlsx.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,9 @@ public function save($filename, int $flags = 0): void
455455

456456
// Media
457457
foreach ($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
458-
$zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
458+
if ($image->getPath() !== '') {
459+
$zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
460+
}
459461
}
460462
}
461463

@@ -471,6 +473,9 @@ public function save($filename, int $flags = 0): void
471473
if ($this->getDrawingHashTable()->getByIndex($i) instanceof WorksheetDrawing) {
472474
$imageContents = null;
473475
$imagePath = $this->getDrawingHashTable()->getByIndex($i)->getPath();
476+
if ($imagePath === '') {
477+
continue;
478+
}
474479
if (str_contains($imagePath, 'zip://')) {
475480
$imagePath = substr($imagePath, 6);
476481
$imagePathSplitted = explode('#', $imagePath);
@@ -664,6 +669,9 @@ private function processDrawing(WorksheetDrawing $drawing): string|null|false
664669
{
665670
$data = null;
666671
$filename = $drawing->getPath();
672+
if ($filename === '') {
673+
return null;
674+
}
667675
$imageData = getimagesize($filename);
668676

669677
if (!empty($imageData)) {

‎src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PhpOffice\PhpSpreadsheet\Shared\File;
77
use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
88
use PhpOffice\PhpSpreadsheet\Spreadsheet;
9+
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing as WorksheetDrawing;
910
use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing;
1011
use PhpOffice\PhpSpreadsheet\Writer\Exception as WriterException;
1112

@@ -132,18 +133,23 @@ public function writeContentTypes(Spreadsheet $spreadsheet, bool $includeCharts
132133
$extension = '';
133134
$mimeType = '';
134135

135-
if ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof \PhpOffice\PhpSpreadsheet\Worksheet\Drawing) {
136-
$extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getExtension());
137-
$mimeType = $this->getImageMimeType($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getPath());
138-
} elseif ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof MemoryDrawing) {
139-
$extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType());
136+
$drawing = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i);
137+
if ($drawing instanceof WorksheetDrawing && $drawing->getPath() !== '') {
138+
$extension = strtolower($drawing->getExtension());
139+
if ($drawing->getIsUrl()) {
140+
$mimeType = image_type_to_mime_type($drawing->getType());
141+
} else {
142+
$mimeType = $this->getImageMimeType($drawing->getPath());
143+
}
144+
} elseif ($drawing instanceof MemoryDrawing) {
145+
$extension = strtolower($drawing->getMimeType());
140146
$extension = explode('/', $extension);
141147
$extension = $extension[1];
142148

143-
$mimeType = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType();
149+
$mimeType = $drawing->getMimeType();
144150
}
145151

146-
if (!isset($aMediaContentTypes[$extension])) {
152+
if ($mimeType !== '' && !isset($aMediaContentTypes[$extension])) {
147153
$aMediaContentTypes[$extension] = $mimeType;
148154

149155
$this->writeDefaultContentType($objWriter, $extension, $mimeType);
@@ -162,7 +168,7 @@ public function writeContentTypes(Spreadsheet $spreadsheet, bool $includeCharts
162168
for ($i = 0; $i < $sheetCount; ++$i) {
163169
if (count($spreadsheet->getSheet($i)->getHeaderFooter()->getImages()) > 0) {
164170
foreach ($spreadsheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
165-
if (!isset($aMediaContentTypes[strtolower($image->getExtension())])) {
171+
if ($image->getPath() !== '' && !isset($aMediaContentTypes[strtolower($image->getExtension())])) {
166172
$aMediaContentTypes[strtolower($image->getExtension())] = $this->getImageMimeType($image->getPath());
167173

168174
$this->writeDefaultContentType($objWriter, strtolower($image->getExtension()), $aMediaContentTypes[strtolower($image->getExtension())]);

‎tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
66

7+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
78
use PhpOffice\PhpSpreadsheet\IOFactory;
89
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
910
use PhpOffice\PhpSpreadsheetTests\Reader\Utility\File;
@@ -41,4 +42,28 @@ public function testURLImageSource(): void
4142
self::assertSame('png', $extension);
4243
}
4344
}
45+
46+
public function testURLImageSourceNotFound(): void
47+
{
48+
if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
49+
self::markTestSkipped('Skipped due to setting of environment variable');
50+
}
51+
$filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.notfound.xlsx');
52+
self::assertNotFalse($filename);
53+
$reader = IOFactory::createReader('Xlsx');
54+
$spreadsheet = $reader->load($filename);
55+
$worksheet = $spreadsheet->getActiveSheet();
56+
$collection = $worksheet->getDrawingCollection();
57+
self::assertCount(0, $collection);
58+
}
59+
60+
public function testURLImageSourceBadProtocol(): void
61+
{
62+
$filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.bad.dontuse');
63+
self::assertNotFalse($filename);
64+
$this->expectException(SpreadsheetException::class);
65+
$this->expectExceptionMessage('Invalid protocol for linked drawing');
66+
$reader = IOFactory::createReader('Xlsx');
67+
$reader->load($filename);
68+
}
4469
}

‎tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
66

7+
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
78
use PhpOffice\PhpSpreadsheet\Spreadsheet;
89
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
910
use PhpOffice\PhpSpreadsheet\Writer\Html;
@@ -59,7 +60,9 @@ public function testSheetWithImageBelowData(): void
5960

6061
// Add a drawing to the worksheet
6162
$drawing = new Drawing();
62-
$drawing->setPath('foo.png', false);
63+
$path = 'tests/data/Writer/XLSX/blue_square.png';
64+
$drawing->setPath($path);
65+
self::assertSame($path, $drawing->getPath());
6366
$drawing->setCoordinates('A5');
6467
$drawing->setWorksheet($sheet);
6568

@@ -74,13 +77,51 @@ public function testSheetWithImageRightOfData(): void
7477

7578
// Add a drawing to the worksheet
7679
$drawing = new Drawing();
77-
$drawing->setPath('foo.png', false);
80+
$path = 'tests/data/Writer/XLSX/blue_square.png';
81+
$drawing->setPath($path);
82+
self::assertSame($path, $drawing->getPath());
7883
$drawing->setCoordinates('E1');
7984
$drawing->setWorksheet($sheet);
8085

8186
$this->assertMaxColumnAndMaxRow($spreadsheet, 5, 3);
8287
}
8388

89+
public function testSheetWithBadImageRightOfData(): void
90+
{
91+
$spreadsheet = new Spreadsheet();
92+
$sheet = $spreadsheet->getActiveSheet();
93+
$sheet->setCellValue('B3', 'foo');
94+
95+
// Add a drawing to the worksheet
96+
$drawing = new Drawing();
97+
$path = 'tests/data/Writer/XLSX/xblue_square.png';
98+
$drawing->setPath($path, false);
99+
self::assertSame('', $drawing->getPath());
100+
$drawing->setCoordinates('E1');
101+
$drawing->setWorksheet($sheet);
102+
103+
$this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3);
104+
}
105+
106+
public function testSheetWithBadImageRightOfDataThrow(): void
107+
{
108+
$this->expectException(PhpSpreadsheetException::class);
109+
$this->expectExceptionMessage('not found!');
110+
$spreadsheet = new Spreadsheet();
111+
$sheet = $spreadsheet->getActiveSheet();
112+
$sheet->setCellValue('B3', 'foo');
113+
114+
// Add a drawing to the worksheet
115+
$drawing = new Drawing();
116+
$path = 'tests/data/Writer/XLSX/xblue_square.png';
117+
$drawing->setPath($path);
118+
self::assertSame('', $drawing->getPath());
119+
$drawing->setCoordinates('E1');
120+
$drawing->setWorksheet($sheet);
121+
122+
$this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3);
123+
}
124+
84125
private function assertMaxColumnAndMaxRow(Spreadsheet $spreadsheet, int $expectedColumnCount, int $expectedRowCount): void
85126
{
86127
$writer = new Html($spreadsheet);

0 commit comments

Comments
 (0)