Reading Xlsx With No or Empty Styles

This is an exact replacement for PR PHPOffice#2247 by @arraintxo. That PR has reached a state in Github where it cannot be merged, and I am not sure how to fix it, other than by re-submitting it as this PR.

See issue PHPOffice#2246. A spreadsheet lacking styles.xml is throwing an exception on Read. Additionally, after fixing that problem, attempting to save the object that has been read throws an exception. This is also true for a spreadsheet with an empty styles.xml.

Author: oleibman

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -395,10 +395,8 @@ public function load(string $pFilename, int $flags = 0): Spreadsheet
         // Initialisations
         $excel = new Spreadsheet();
         $excel->removeSheetByIndex(0);
-        if (!$this->readDataOnly) {
-            $excel->removeCellStyleXfByIndex(0); // remove the default style
-            $excel->removeCellXfByIndex(0); // remove the default style
-        }
+        $addingFirstCellStyleXf = true;
+        $addingFirstCellXf = true;
         $unparsedLoadedData = [];
 
         $this->zip = $zip = new ZipArchive();
@@ -534,8 +532,12 @@ public function load(string $pFilename, int $flags = 0): Spreadsheet
                         . "$xmlNamespaceBase/styles"
                         . "']";
                     $xpath = self::getArrayItem(self::xpathNoFalse($relsWorkbook, $relType));
-                    // I think Nonamespace is okay because I'm using xpath.
-                    $xmlStyles = $this->loadZipNonamespace("$dir/$xpath[Target]", $mainNS);
+                    if ($xpath === null) {
+                        $xmlStyles = self::testSimpleXml(null);
+                    } else {
+                        // I think Nonamespace is okay because I'm using xpath.
+                        $xmlStyles = $this->loadZipNonamespace("$dir/$xpath[Target]", $mainNS);
+                    }
                     $xmlStyles->registerXPathNamespace('smm', Namespaces::MAIN);
                     $fills = self::xpathNoFalse($xmlStyles, 'smm:fills/smm:fill');
                     $fonts = self::xpathNoFalse($xmlStyles, 'smm:fonts/smm:font');
@@ -593,6 +595,10 @@ public function load(string $pFilename, int $flags = 0): Spreadsheet
                             // add style to cellXf collection
                             $objStyle = new Style();
                             self::readStyle($objStyle, $style);
+                            if ($addingFirstCellXf) {
+                                $excel->removeCellXfByIndex(0); // remove the default style
+                                $addingFirstCellXf = false;
+                            }
                             $excel->addCellXf($objStyle);
                         }
 
@@ -624,6 +630,10 @@ public function load(string $pFilename, int $flags = 0): Spreadsheet
                             // add style to cellStyleXf collection
                             $objStyle = new Style();
                             self::readStyle($objStyle, $cellStyle);
+                            if ($addingFirstCellStyleXf) {
+                                $excel->removeCellStyleXfByIndex(0); // remove the default style
+                                $addingFirstCellStyleXf = false;
+                            }
                             $excel->addCellStyleXf($objStyle);
                         }
                     }

tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php

@@ -4,7 +4,7 @@
 
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
 use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
-use PhpOffice\PhpSpreadsheet\Document\Properties;
+use PhpOffice\PhpSpreadsheet\IOFactory;
 use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
 use PhpOffice\PhpSpreadsheet\Shared\File;
 use PhpOffice\PhpSpreadsheet\Style\Conditional;
@@ -37,6 +37,7 @@ public function testLoadXlsxRowColumnAttributes(): void
         }
 
         self::assertFalse($worksheet->getColumnDimension('E')->getVisible());
+        $spreadsheet->disconnectWorksheets();
     }
 
     public function testLoadXlsxWithStyles(): void
@@ -61,6 +62,54 @@ public function testLoadXlsxWithStyles(): void
                 );
             }
         }
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    /**
+     * Test load Xlsx file without styles.xml.
+     */
+    public function testLoadXlsxWithoutStyles(): void
+    {
+        $filename = 'tests/data/Reader/XLSX/issue.2246a.xlsx';
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+
+        $tempFilename = File::temporaryFilename();
+        $writer = IOFactory::createWriter($spreadsheet, 'Xlsx');
+        $writer->save($tempFilename);
+
+        $reader = new Xlsx();
+        $reloadedSpreadsheet = $reader->load($tempFilename);
+        unlink($tempFilename);
+
+        $reloadedWorksheet = $reloadedSpreadsheet->getActiveSheet();
+
+        self::assertEquals('TipoDato', $reloadedWorksheet->getCell('A1')->getValue());
+        $spreadsheet->disconnectWorksheets();
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+
+    /**
+     * Test load Xlsx file with empty styles.xml.
+     */
+    public function testLoadXlsxWithEmptyStyles(): void
+    {
+        $filename = 'tests/data/Reader/XLSX/issue.2246b.xlsx';
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+
+        $tempFilename = File::temporaryFilename();
+        $writer = IOFactory::createWriter($spreadsheet, 'Xlsx');
+        $writer->save($tempFilename);
+
+        $reader = new Xlsx();
+        $reloadedSpreadsheet = $reader->load($tempFilename);
+        unlink($tempFilename);
+
+        $reloadedWorksheet = $reloadedSpreadsheet->getActiveSheet();
+        self::assertEquals('TipoDato', $reloadedWorksheet->getCell('A1')->getValue());
+        $spreadsheet->disconnectWorksheets();
+        $reloadedSpreadsheet->disconnectWorksheets();
     }
 
     public function testLoadXlsxAutofilter(): void
@@ -78,6 +127,7 @@ public function testLoadXlsxAutofilter(): void
             AutoFilter\Column::AUTOFILTER_FILTERTYPE_FILTER,
             $autofilter->getColumn('A')->getFilterType()
         );
+        $spreadsheet->disconnectWorksheets();
     }
 
     public function testLoadXlsxPageSetup(): void
@@ -97,6 +147,7 @@ public function testLoadXlsxPageSetup(): void
 
         self::assertEquals(PageSetup::PAPERSIZE_A4, $worksheet->getPageSetup()->getPaperSize());
         self::assertEquals(['A10', 'A20', 'A30', 'A40', 'A50'], array_keys($worksheet->getBreaks()));
+        $spreadsheet->disconnectWorksheets();
     }
 
     public function testLoadXlsxConditionalFormatting(): void
@@ -116,6 +167,7 @@ public function testLoadXlsxConditionalFormatting(): void
         self::assertEquals(Conditional::OPERATOR_BETWEEN, $conditionalRule->getOperatorType());
         self::assertEquals(['200', '400'], $conditionalRule->getConditions());
         self::assertInstanceOf(Style::class, $conditionalRule->getStyle());
+        $spreadsheet->disconnectWorksheets();
     }
 
     public function testLoadXlsxDataValidation(): void
@@ -127,6 +179,7 @@ public function testLoadXlsxDataValidation(): void
         $worksheet = $spreadsheet->getActiveSheet();
 
         self::assertTrue($worksheet->getCell('B3')->hasDataValidation());
+        $spreadsheet->disconnectWorksheets();
     }
 
     /*
@@ -152,6 +205,7 @@ public function testLoadXlsxDataValidationOfAnotherSheet(): void
         self::assertTrue($validationCell->hasDataValidation());
         self::assertSame(DataValidation::TYPE_LIST, $validationCell->getDataValidation()->getType());
         self::assertSame('Feuil2!$A$3:$A$5', $validationCell->getDataValidation()->getFormula1());
+        $spreadsheet->disconnectWorksheets();
     }
 
     /**
@@ -163,7 +217,8 @@ public function testLoadXlsxWithoutCellReference(): void
     {
         $filename = 'tests/data/Reader/XLSX/without_cell_reference.xlsx';
         $reader = new Xlsx();
-        $reader->load($filename);
+        $spreadsheet = $reader->load($filename);
+        $spreadsheet->disconnectWorksheets();
     }
 
     /**
@@ -174,12 +229,14 @@ public function testLoadWithReadFilter(): void
         $filename = 'tests/data/Reader/XLSX/without_cell_reference.xlsx';
         $reader = new Xlsx();
         $reader->setReadFilter(new OddColumnReadFilter());
-        $data = $reader->load($filename)->getActiveSheet()->toArray();
+        $spreadsheet = $reader->load($filename);
+        $data = $spreadsheet->getActiveSheet()->toArray();
         $ref = [1.0, null, 3.0, null, 5.0, null, 7.0, null, 9.0, null];
 
         for ($i = 0; $i < 10; ++$i) {
             self::assertEquals($ref, \array_slice($data[$i], 0, 10, true));
         }
+        $spreadsheet->disconnectWorksheets();
     }
 
     /**
@@ -191,7 +248,8 @@ public function testLoadXlsxWithDoubleAttrDrawing(): void
     {
         $filename = 'tests/data/Reader/XLSX/double_attr_drawing.xlsx';
         $reader = new Xlsx();
-        $reader->load($filename);
+        $spreadsheet = $reader->load($filename);
+        $spreadsheet->disconnectWorksheets();
     }
 
     /**
@@ -206,10 +264,12 @@ public function testLoadSaveWithEmptyDrawings(): void
         $resultFilename = File::temporaryFilename();
         $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($excel);
         $writer->save($resultFilename);
+        $excel->disconnectWorksheets();
         $excel = $reader->load($resultFilename);
         unlink($resultFilename);
         // Fake assert. The only thing we need is to ensure the file is loaded without exception
         self::assertNotNull($excel);
+        $excel->disconnectWorksheets();
     }
 
     /**