diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 120a4f4cef..e107b26679 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -287,7 +287,7 @@ parameters: - message: "#^Offset 'mime' does not exist on array\\{\\}\\|array\\{0\\: int\\<0, max\\>, 1\\: int\\<0, max\\>, 2\\: int, 3\\: string, mime\\: string, channels\\?\\: int, bits\\?\\: int\\}\\.$#" - count: 2 + count: 1 path: src/PhpSpreadsheet/Writer/Html.php - diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 1c33fd6811..5f63743d08 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1291,7 +1291,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $hfImages[$shapeId]->setName((string) $imageData['title']); } - $hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false); + $hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false, $zip); $hfImages[$shapeId]->setResizeProportional(false); $hfImages[$shapeId]->setWidth($style['width']); $hfImages[$shapeId]->setHeight($style['height']); @@ -1401,7 +1401,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $objDrawing->setPath( 'zip://' . File::realpath($filename) . '#' . $images[$embedImageKey], - false + false, + $zip ); } else { $linkImageKey = (string) self::getArrayItem( @@ -1412,6 +1413,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $url = str_replace('xl/drawings/', '', $images[$linkImageKey]); $objDrawing->setPath($url); } + if ($objDrawing->getPath() === '') { + continue; + } } $objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $oneCellAnchor->from->col) + 1) . ($oneCellAnchor->from->row + 1)); @@ -1486,7 +1490,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $objDrawing->setPath( 'zip://' . File::realpath($filename) . '#' . $images[$embedImageKey], - false + false, + $zip ); } else { $linkImageKey = (string) self::getArrayItem( @@ -1497,6 +1502,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $url = str_replace('xl/drawings/', '', $images[$linkImageKey]); $objDrawing->setPath($url); } + if ($objDrawing->getPath() === '') { + continue; + } } $objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $twoCellAnchor->from->col) + 1) . ($twoCellAnchor->from->row + 1)); diff --git a/src/PhpSpreadsheet/Worksheet/BaseDrawing.php b/src/PhpSpreadsheet/Worksheet/BaseDrawing.php index 5001346ad5..49e2eff104 100644 --- a/src/PhpSpreadsheet/Worksheet/BaseDrawing.php +++ b/src/PhpSpreadsheet/Worksheet/BaseDrawing.php @@ -220,7 +220,7 @@ public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = f { if ($this->worksheet === null) { // Add drawing to \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet - if ($worksheet !== null) { + if ($worksheet !== null && !($this instanceof Drawing && $this->getPath() === '')) { $this->worksheet = $worksheet; $this->worksheet->getCell($this->coordinates); $this->worksheet->getDrawingCollection()->append($this); diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php index 7d957539ef..bb65db2b97 100644 --- a/src/PhpSpreadsheet/Worksheet/Drawing.php +++ b/src/PhpSpreadsheet/Worksheet/Drawing.php @@ -106,40 +106,72 @@ public function getPath() */ public function setPath($path, $verifyFile = true, $zip = null) { - if ($verifyFile && preg_match('~^data:image/[a-z]+;base64,~', $path) !== 1) { - // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979 - if (filter_var($path, FILTER_VALIDATE_URL)) { - $this->path = $path; - // Implicit that it is a URL, rather store info than running check above on value in other places. - $this->isUrl = true; - $imageContents = file_get_contents($path); + $this->isUrl = false; + if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) { + $this->path = $path; + + return $this; + } + + $this->path = ''; + // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979 + if (filter_var($path, FILTER_VALIDATE_URL)) { + if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) { + throw new PhpSpreadsheetException('Invalid protocol for linked drawing'); + } + // Implicit that it is a URL, rather store info than running check above on value in other places. + $this->isUrl = true; + $imageContents = @file_get_contents($path); + if ($imageContents !== false) { $filePath = tempnam(sys_get_temp_dir(), 'Drawing'); if ($filePath) { - file_put_contents($filePath, $imageContents); - if (file_exists($filePath)) { - $this->setSizesAndType($filePath); + $put = @file_put_contents($filePath, $imageContents); + if ($put !== false) { + if ($this->isImage($filePath)) { + $this->path = $path; + $this->setSizesAndType($filePath); + } unlink($filePath); } } - } elseif (file_exists($path)) { - $this->path = $path; - $this->setSizesAndType($path); - } elseif ($zip instanceof ZipArchive) { - $zipPath = explode('#', $path)[1]; - if ($zip->locateName($zipPath) !== false) { + } + } elseif ($zip instanceof ZipArchive) { + $zipPath = explode('#', $path)[1]; + $locate = @$zip->locateName($zipPath); + if ($locate !== false) { + if ($this->isImage($path)) { $this->path = $path; $this->setSizesAndType($path); } - } else { - throw new PhpSpreadsheetException("File $path not found!"); } } else { - $this->path = $path; + $exists = @file_exists($path); + if ($exists !== false && $this->isImage($path)) { + $this->path = $path; + $this->setSizesAndType($path); + } + } + if ($this->path === '' && $verifyFile) { + throw new PhpSpreadsheetException("File $path not found!"); } return $this; } + private function isImage(string $path): bool + { + $mime = (string) @mime_content_type($path); + $retVal = false; + if (str_starts_with($mime, 'image/')) { + $retVal = true; + } elseif ($mime === 'application/octet-stream') { + $extension = pathinfo($path, PATHINFO_EXTENSION); + $retVal = in_array($extension, ['bin', 'emf'], true); + } + + return $retVal; + } + /** * Get isURL. */ diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 978d1764c1..66a0ec89b6 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -612,6 +612,9 @@ private function extendRowsForChartsAndImages(Worksheet $worksheet, int $row): s [$rowMax, $colMax, $anyfound] = $this->extendRowsForCharts($worksheet, $row); foreach ($worksheet->getDrawingCollection() as $drawing) { + if ($drawing instanceof Drawing && $drawing->getPath() === '') { + continue; + } $anyfound = true; $imageTL = Coordinate::coordinateFromString($drawing->getCoordinates()); $imageCol = Coordinate::columnIndexFromString($imageTL[0]); @@ -687,7 +690,7 @@ private function writeImageInCell(Worksheet $worksheet, $coordinates) } $filedesc = $drawing->getDescription(); $filedesc = $filedesc ? htmlspecialchars($filedesc, ENT_QUOTES) : 'Embedded image'; - if ($drawing instanceof Drawing) { + if ($drawing instanceof Drawing && $drawing->getPath() !== '') { $filename = $drawing->getPath(); // Strip off eventual '.' @@ -706,12 +709,15 @@ private function writeImageInCell(Worksheet $worksheet, $coordinates) $imageData = self::winFileToUrl($filename, $this->isMPdf); if ($this->embedImages || substr($imageData, 0, 6) === 'zip://') { + $imageData = 'data:,'; $picture = @file_get_contents($filename); if ($picture !== false) { - $imageDetails = getimagesize($filename) ?: []; - // base64 encode the binary data - $base64 = base64_encode($picture); - $imageData = 'data:' . $imageDetails['mime'] . ';base64,' . $base64; + $mimeContentType = (string) @mime_content_type($filename); + if (substr($mimeContentType, 0, 6) === 'image/') { + // base64 encode the binary data + $base64 = base64_encode($picture); + $imageData = 'data:' . $mimeContentType . ';base64,' . $base64; + } } } diff --git a/src/PhpSpreadsheet/Writer/Xls.php b/src/PhpSpreadsheet/Writer/Xls.php index 983414fccc..e0480a5a36 100644 --- a/src/PhpSpreadsheet/Writer/Xls.php +++ b/src/PhpSpreadsheet/Writer/Xls.php @@ -486,7 +486,7 @@ private function processDrawing(BstoreContainer &$bstoreContainer, Drawing $draw private function processBaseDrawing(BstoreContainer &$bstoreContainer, BaseDrawing $drawing): void { - if ($drawing instanceof Drawing) { + if ($drawing instanceof Drawing && $drawing->getPath() !== '') { $this->processDrawing($bstoreContainer, $drawing); } elseif ($drawing instanceof MemoryDrawing) { $this->processMemoryDrawing($bstoreContainer, $drawing, $drawing->getRenderingFunction()); diff --git a/src/PhpSpreadsheet/Writer/Xlsx.php b/src/PhpSpreadsheet/Writer/Xlsx.php index 6ed12d4aa1..601f0f5c4c 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx.php +++ b/src/PhpSpreadsheet/Writer/Xlsx.php @@ -495,7 +495,9 @@ public function save($filename, int $flags = 0): void // Media foreach ($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages() as $image) { - $zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath()); + if ($image->getPath() !== '') { + $zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath()); + } } } @@ -511,6 +513,9 @@ public function save($filename, int $flags = 0): void if ($this->getDrawingHashTable()->getByIndex($i) instanceof WorksheetDrawing) { $imageContents = null; $imagePath = $this->getDrawingHashTable()->getByIndex($i)->getPath(); + if ($imagePath === '') { + continue; + } if (strpos($imagePath, 'zip://') !== false) { $imagePath = substr($imagePath, 6); $imagePathSplitted = explode('#', $imagePath); @@ -712,6 +717,9 @@ private function processDrawing(WorksheetDrawing $drawing) { $data = null; $filename = $drawing->getPath(); + if ($filename === '') { + return null; + } $imageData = getimagesize($filename); if (!empty($imageData)) { diff --git a/src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php b/src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php index 73657fc380..56f062b562 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php @@ -6,6 +6,7 @@ use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Shared\XMLWriter; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Worksheet\Drawing as WorksheetDrawing; use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing; use PhpOffice\PhpSpreadsheet\Writer\Exception as WriterException; @@ -132,18 +133,23 @@ public function writeContentTypes(Spreadsheet $spreadsheet, $includeCharts = fal $extension = ''; $mimeType = ''; - if ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof \PhpOffice\PhpSpreadsheet\Worksheet\Drawing) { - $extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getExtension()); - $mimeType = $this->getImageMimeType($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getPath()); - } elseif ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof MemoryDrawing) { - $extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType()); + $drawing = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i); + if ($drawing instanceof WorksheetDrawing && $drawing->getPath() !== '') { + $extension = strtolower($drawing->getExtension()); + if ($drawing->getIsUrl()) { + $mimeType = image_type_to_mime_type($drawing->getType()); + } else { + $mimeType = $this->getImageMimeType($drawing->getPath()); + } + } elseif ($drawing instanceof MemoryDrawing) { + $extension = strtolower($drawing->getMimeType()); $extension = explode('/', $extension); $extension = $extension[1]; - $mimeType = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType(); + $mimeType = $drawing->getMimeType(); } - if (!isset($aMediaContentTypes[$extension])) { + if ($mimeType !== '' && !isset($aMediaContentTypes[$extension])) { $aMediaContentTypes[$extension] = $mimeType; $this->writeDefaultContentType($objWriter, $extension, $mimeType); @@ -162,7 +168,7 @@ public function writeContentTypes(Spreadsheet $spreadsheet, $includeCharts = fal for ($i = 0; $i < $sheetCount; ++$i) { if (count($spreadsheet->getSheet($i)->getHeaderFooter()->getImages()) > 0) { foreach ($spreadsheet->getSheet($i)->getHeaderFooter()->getImages() as $image) { - if (!isset($aMediaContentTypes[strtolower($image->getExtension())])) { + if ($image->getPath() !== '' && !isset($aMediaContentTypes[strtolower($image->getExtension())])) { $aMediaContentTypes[strtolower($image->getExtension())] = $this->getImageMimeType($image->getPath()); $this->writeDefaultContentType($objWriter, strtolower($image->getExtension()), $aMediaContentTypes[strtolower($image->getExtension())]); diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php index e7f9901013..bf86eb69c0 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx; +use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; use PhpOffice\PhpSpreadsheet\IOFactory; use PhpOffice\PhpSpreadsheet\Worksheet\Drawing; use PhpOffice\PhpSpreadsheetTests\Reader\Utility\File; @@ -9,7 +10,8 @@ class URLImageTest extends TestCase { - public function testURLImageSource(): void + // https://github.com/readthedocs/readthedocs.org/issues/11615 + public function xtestURLImageSource(): void { if (getenv('SKIP_URL_IMAGE_TEST') === '1') { self::markTestSkipped('Skipped due to setting of environment variable'); @@ -39,4 +41,28 @@ public function testURLImageSource(): void self::assertSame('png', $extension); } } + + public function xtestURLImageSourceNotFound(): void + { + if (getenv('SKIP_URL_IMAGE_TEST') === '1') { + self::markTestSkipped('Skipped due to setting of environment variable'); + } + $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.notfound.xlsx'); + self::assertNotFalse($filename); + $reader = IOFactory::createReader('Xlsx'); + $spreadsheet = $reader->load($filename); + $worksheet = $spreadsheet->getActiveSheet(); + $collection = $worksheet->getDrawingCollection(); + self::assertCount(0, $collection); + } + + public function testURLImageSourceBadProtocol(): void + { + $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.bad.dontuse'); + self::assertNotFalse($filename); + $this->expectException(SpreadsheetException::class); + $this->expectExceptionMessage('Invalid protocol for linked drawing'); + $reader = IOFactory::createReader('Xlsx'); + $reader->load($filename); + } } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php b/tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php index e3d23230c1..f2db5c5904 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Html; +use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Drawing; use PhpOffice\PhpSpreadsheet\Writer\Html; @@ -57,7 +58,9 @@ public function testSheetWithImageBelowData(): void // Add a drawing to the worksheet $drawing = new Drawing(); - $drawing->setPath('foo.png', false); + $path = 'tests/data/Writer/XLSX/blue_square.png'; + $drawing->setPath($path); + self::assertSame($path, $drawing->getPath()); $drawing->setCoordinates('A5'); $drawing->setWorksheet($sheet); @@ -72,13 +75,51 @@ public function testSheetWithImageRightOfData(): void // Add a drawing to the worksheet $drawing = new Drawing(); - $drawing->setPath('foo.png', false); + $path = 'tests/data/Writer/XLSX/blue_square.png'; + $drawing->setPath($path); + self::assertSame($path, $drawing->getPath()); $drawing->setCoordinates('E1'); $drawing->setWorksheet($sheet); $this->assertMaxColumnAndMaxRow($spreadsheet, 5, 3); } + public function testSheetWithBadImageRightOfData(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->setCellValue('B3', 'foo'); + + // Add a drawing to the worksheet + $drawing = new Drawing(); + $path = 'tests/data/Writer/XLSX/xblue_square.png'; + $drawing->setPath($path, false); + self::assertSame('', $drawing->getPath()); + $drawing->setCoordinates('E1'); + $drawing->setWorksheet($sheet); + + $this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3); + } + + public function testSheetWithBadImageRightOfDataThrow(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('not found!'); + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->setCellValue('B3', 'foo'); + + // Add a drawing to the worksheet + $drawing = new Drawing(); + $path = 'tests/data/Writer/XLSX/xblue_square.png'; + $drawing->setPath($path); + self::assertSame('', $drawing->getPath()); + $drawing->setCoordinates('E1'); + $drawing->setWorksheet($sheet); + + $this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3); + } + private function assertMaxColumnAndMaxRow(Spreadsheet $spreadsheet, int $expectedColumnCount, int $expectedRowCount): void { $writer = new Html($spreadsheet); diff --git a/tests/PhpSpreadsheetTests/Writer/Html/ImageEmbedTest.php b/tests/PhpSpreadsheetTests/Writer/Html/ImageEmbedTest.php new file mode 100644 index 0000000000..bc9e40ed4c --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Html/ImageEmbedTest.php @@ -0,0 +1,45 @@ +getActiveSheet(); + + $drawing = new Drawing(); + $drawing->setName('Not an image'); + $drawing->setDescription('Non-image'); + $drawing->setPath(__FILE__, false); + $drawing->setCoordinates('A1'); + $drawing->setCoordinates2('E4'); + $drawing->setWorksheet($sheet); + + $drawing = new Drawing(); + $drawing->setName('Blue Square'); + $drawing->setPath('tests/data/Writer/XLSX/blue_square.png'); + $drawing->setCoordinates('A5'); + $drawing->setCoordinates2('E8'); + $drawing->setWorksheet($sheet); + + $writer = new HtmlWriter($spreadsheet); + $writer->setEmbedImages(true); + $html = $writer->generateHTMLAll(); + self::assertSame(1, substr_count($html, 'disconnectWorksheets(); + } +} diff --git a/tests/data/Reader/XLSX/urlImage.bad.dontuse b/tests/data/Reader/XLSX/urlImage.bad.dontuse new file mode 100644 index 0000000000..3e39a809ff Binary files /dev/null and b/tests/data/Reader/XLSX/urlImage.bad.dontuse differ diff --git a/tests/data/Reader/XLSX/urlImage.notfound.xlsx b/tests/data/Reader/XLSX/urlImage.notfound.xlsx new file mode 100644 index 0000000000..e335a80de1 Binary files /dev/null and b/tests/data/Reader/XLSX/urlImage.notfound.xlsx differ