Name Clashes Between Parsed and Unparsed Drawings

samples/Reader/22_Reader_issue1767.php

@@ -0,0 +1,21 @@
+<?php
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+
+require __DIR__ . '/../Header.php';
+
+$helper->log('Start');
+
+$inputFileType = 'Xlsx';
+$inputFileName = __DIR__ . '/sampleData/issue.1767.xlsx';
+
+$helper->log('Loading file ' . $inputFileName . ' using IOFactory with a defined reader type of ' . $inputFileType);
+$reader = IOFactory::createReader($inputFileType);
+$helper->log('Loading all WorkSheets');
+$reader->setLoadAllSheets();
+$spreadsheet = $reader->load($inputFileName);
+
+// Save
+$helper->write($spreadsheet, __FILE__);
+
+$helper->log('end');

src/PhpSpreadsheet/Writer/Xlsx.php

@@ -431,8 +431,9 @@ public function save($filename, int $flags = 0): void
                 foreach ($unparsedLoadedData['sheets'][$sheetCodeName]['Drawings'] as $relId => $drawingXml) {
                     $drawingFile = array_search($relId, $unparsedLoadedData['sheets'][$sheetCodeName]['drawingOriginalIds']);
                     if ($drawingFile !== false) {
-                        $drawingFile = ltrim($drawingFile, '.');
-                        $zipContent['xl' . $drawingFile] = $drawingXml;
+                        //$drawingFile = ltrim($drawingFile, '.');
+                        //$zipContent['xl' . $drawingFile] = $drawingXml;
+                        $zipContent['xl/drawings/drawing' . ($i + 1) . '.xml'] = $drawingXml;
                     }
                 }
             }

tests/PhpSpreadsheetTests/Writer/Xlsx/Unparsed2396Test.php

@@ -0,0 +1,90 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader;
+use PhpOffice\PhpSpreadsheet\Shared\File;
+use PhpOffice\PhpSpreadsheet\Worksheet\BaseDrawing;
+use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
+use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
+use PHPUnit\Framework\TestCase;
+
+class Unparsed2396Test extends TestCase
+{
+    /** @var string */
+    private $filename = '';
+
+    protected function tearDown(): void
+    {
+        if ($this->filename !== '') {
+            unlink($this->filename);
+            $this->filename = '';
+        }
+    }
+
+    private function getContents(?BaseDrawing $drawing): string
+    {
+        $contents = '';
+        if ($drawing instanceof Drawing) {
+            $contents = (string) file_get_contents($drawing->getPath());
+        } else {
+            self::fail('Unexpected null or baseDrawing which is not Drawing');
+        }
+        self::assertNotSame('', $contents);
+
+        return $contents;
+    }
+
+    // Don't drop image as in issue 2396.
+    public function testUnparsed2396(): void
+    {
+        $sampleFilename = 'tests/data/Writer/XLSX/issue.2396.xlsx';
+        $reader = new Reader();
+        $excel = $reader->load($sampleFilename);
+        $outputFilename = $this->filename = File::temporaryFilename();
+        $writer = new Writer($excel);
+        $writer->save($outputFilename);
+        //$spreadsheet = $this->writeAndReload($excel, 'Xlsx');
+        $excel->disconnectWorksheets();
+        $reader = new Reader();
+        $spreadsheet = $reader->load($outputFilename);
+        $sheet = $spreadsheet->getSheet(0);
+        $drawing1 = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawing1);
+        $hash = $this->getContents($drawing1[0]);
+
+        $sheet = $spreadsheet->getSheet(1);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        self::assertSame($hash, $this->getContents($drawings[0]));
+
+        $sheet = $spreadsheet->getSheet(2);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(0, $drawings);
+        //self::assertSame($hash, md5(file_get_contents($drawings[0]->getPath())));
+
+        $sheet = $spreadsheet->getSheet(3);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        self::assertSame($hash, $this->getContents($drawings[0]));
+
+        $sheet = $spreadsheet->getSheet(4);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(0, $drawings);
+        //self::assertSame($hash, md5(file_get_contents($drawings[0]->getPath())));
+
+        // The next 2 sheets have 'legacyDrawing', button, and listbox.
+        // If support is added for those, these tests may need adjustment.
+        $sheet = $spreadsheet->getSheet(5);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(0, $drawings);
+        //self::assertSame($hash, md5(file_get_contents($drawings[0]->getPath())));
+
+        $sheet = $spreadsheet->getSheet(6);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        self::assertSame($hash, $this->getContents($drawings[0]));
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}