Xlsx Reader Namespacing for Tables, AutoFilters (#3668)

* Xlsx Reader Namespacing for Tables, AutoFilters

Fix #3665. The original issue was the use of an absolute path in the rels file pointing to the comments file. That was easy to take care of, but a bigger problem with the spreadsheet accompanying the problem report was that it used unexpected spacing for AutoFilters and Tables. AutoFilters were already known not to be covered, but Tables appeared after the namespacing changes, but without namespacing support. This PR fixes the absolute path problem and adds namespacing support for Tables and AutoFilters.

Remaining areas which are still namespace unaware, mainly because of the absence of test samples which use them with unexpected namespacing, include conditional formatting (internal or external), sheet view options, sheet protection, unparsed loaded data, data validation (internal or external), alternate content, and header/footer images.

* Mysterious Warning for Php7.4 Only

Node no longer exists, doesn't affect result. Suppress warning.

* Scrutinizer

What would a change be without some new false positives?

* Scrutinizer

A gift that keeps on giving :-( Now it's deciding that things that it didn't report in a scan from a few minutes ago are worth reporting, even with no relevant code changes.

* Scrutinizer

It is an idiot. Let me see if I can fix one false positive and succeed in guessing where it might report another one, even though it didn't do so with this last run.

Author: oleibman

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -793,6 +793,14 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                             $docSheet->setTitle((string) $eleSheetAttr['name'], false, false);
 
                             $fileWorksheet = (string) $worksheets[$sheetReferenceId];
+                            // issue 3665 adds test for /.
+                            // This broke XlsxRootZipFilesTest,
+                            //  but Excel reports an error with that file.
+                            //  Testing dir for . avoids this problem.
+                            //  It might be better just to drop the test.
+                            if ($fileWorksheet[0] == '/' && $dir !== '.') {
+                                $fileWorksheet = substr($fileWorksheet, strlen($dir) + 2);
+                            }
                             $xmlSheet = $this->loadZipNoNamespace("$dir/$fileWorksheet", $mainNS);
                             $xmlSheetNS = $this->loadZip("$dir/$fileWorksheet", $mainNS);
 
@@ -980,8 +988,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                             }
 
                             if ($this->readDataOnly === false) {
-                                $this->readAutoFilter($xmlSheet, $docSheet);
-                                $this->readTables($xmlSheet, $docSheet, $dir, $fileWorksheet, $zip);
+                                $this->readAutoFilter($xmlSheetNS, $docSheet);
+                                $this->readTables($xmlSheetNS, $docSheet, $dir, $fileWorksheet, $zip, $mainNS);
                             }
 
                             if ($xmlSheetNS && $xmlSheetNS->mergeCells && $xmlSheetNS->mergeCells->mergeCell && !$this->readDataOnly) {
@@ -2206,10 +2214,14 @@ private function readTables(
         Worksheet $docSheet,
         string $dir,
         string $fileWorksheet,
-        ZipArchive $zip
+        ZipArchive $zip,
+        string $namespaceTable
     ): void {
-        if ($xmlSheet && $xmlSheet->tableParts && (int) $xmlSheet->tableParts['count'] > 0) {
-            $this->readTablesInTablesFile($xmlSheet, $dir, $fileWorksheet, $zip, $docSheet);
+        if ($xmlSheet && $xmlSheet->tableParts) {
+            $attributes = $xmlSheet->tableParts->attributes() ?? ['count' => 0];
+            if (((int) $attributes['count']) > 0) {
+                $this->readTablesInTablesFile($xmlSheet, $dir, $fileWorksheet, $zip, $docSheet, $namespaceTable);
+            }
         }
     }
 
@@ -2218,7 +2230,8 @@ private function readTablesInTablesFile(
         string $dir,
         string $fileWorksheet,
         ZipArchive $zip,
-        Worksheet $docSheet
+        Worksheet $docSheet,
+        string $namespaceTable
     ): void {
         foreach ($xmlSheet->tableParts->tablePart as $tablePart) {
             $relation = self::getAttributes($tablePart, Namespaces::SCHEMA_OFFICE_DOCUMENT);
@@ -2236,7 +2249,7 @@ private function readTablesInTablesFile(
                         $relationshipFilePath = File::realpath($relationshipFilePath);
 
                         if ($this->fileExistsInArchive($this->zip, $relationshipFilePath)) {
-                            $tableXml = $this->loadZip($relationshipFilePath);
+                            $tableXml = $this->loadZip($relationshipFilePath, $namespaceTable);
                             (new TableReader($docSheet, $tableXml))->load();
                         }
                     }

src/PhpSpreadsheet/Reader/Xlsx/AutoFilter.php

@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader\Xlsx;
 
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
 use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column;
 use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule;
 use PhpOffice\PhpSpreadsheet\Worksheet\Table;
@@ -32,36 +33,40 @@ public function __construct($parent, SimpleXMLElement $worksheetXml)
     public function load(): void
     {
         // Remove all "$" in the auto filter range
-        $autoFilterRange = (string) preg_replace('/\$/', '', $this->worksheetXml->autoFilter['ref'] ?? '');
+        $attrs = $this->worksheetXml->autoFilter->attributes() ?? [];
+        // Mysterious 'Node no longer exists' warning for Php7.4 only.
+        $autoFilterRange = (string) @preg_replace('/\$/', '', $attrs['ref'] ?? '');
         if (strpos($autoFilterRange, ':') !== false) {
-            $this->readAutoFilter($autoFilterRange, $this->worksheetXml);
+            $this->readAutoFilter($autoFilterRange);
         }
     }
 
-    private function readAutoFilter(string $autoFilterRange, SimpleXMLElement $xmlSheet): void
+    private function readAutoFilter(string $autoFilterRange): void
     {
         $autoFilter = $this->parent->getAutoFilter();
         $autoFilter->setRange($autoFilterRange);
 
-        foreach ($xmlSheet->autoFilter->filterColumn as $filterColumn) {
-            $column = $autoFilter->getColumnByOffset((int) $filterColumn['colId']);
+        foreach ($this->worksheetXml->autoFilter->filterColumn as $filterColumn) {
+            $attributes = $filterColumn->/** @scrutinizer ignore-call */ attributes() ?? [];
+            $column = $autoFilter->getColumnByOffset((int) ($attributes['colId'] ?? 0));
             //    Check for standard filters
             if ($filterColumn->filters) {
                 $column->setFilterType(Column::AUTOFILTER_FILTERTYPE_FILTER);
-                $filters = $filterColumn->filters;
+                $filters = Xlsx::testSimpleXml($filterColumn->filters->attributes());
                 if ((isset($filters['blank'])) && ((int) $filters['blank'] == 1)) {
                     //    Operator is undefined, but always treated as EQUAL
                     $column->createRule()->setRule('', '')->setRuleType(Rule::AUTOFILTER_RULETYPE_FILTER);
                 }
                 //    Standard filters are always an OR join, so no join rule needs to be set
                 //    Entries can be either filter elements
-                foreach ($filters->filter as $filterRule) {
+                foreach ($filterColumn->filters->filter as $filterRule) {
                     //    Operator is undefined, but always treated as EQUAL
-                    $column->createRule()->setRule('', (string) $filterRule['val'])->setRuleType(Rule::AUTOFILTER_RULETYPE_FILTER);
+                    $attr2 = $filterRule->/** @scrutinizer ignore-call */ attributes() ?? ['val' => ''];
+                    $column->createRule()->setRule('', (string) $attr2['val'])->setRuleType(Rule::AUTOFILTER_RULETYPE_FILTER);
                 }
 
                 //    Or Date Group elements
-                $this->readDateRangeAutoFilter($filters, $column);
+                $this->readDateRangeAutoFilter($filterColumn->filters, $column);
             }
 
             //    Check for custom filters
@@ -76,20 +81,23 @@ private function readAutoFilter(string $autoFilterRange, SimpleXMLElement $xmlSh
 
     private function readDateRangeAutoFilter(SimpleXMLElement $filters, Column $column): void
     {
-        foreach ($filters->dateGroupItem as $dateGroupItem) {
+        foreach ($filters->dateGroupItem as $dateGroupItemx) {
             //    Operator is undefined, but always treated as EQUAL
-            $column->createRule()->setRule(
-                '',
-                [
-                    'year' => (string) $dateGroupItem['year'],
-                    'month' => (string) $dateGroupItem['month'],
-                    'day' => (string) $dateGroupItem['day'],
-                    'hour' => (string) $dateGroupItem['hour'],
-                    'minute' => (string) $dateGroupItem['minute'],
-                    'second' => (string) $dateGroupItem['second'],
-                ],
-                (string) $dateGroupItem['dateTimeGrouping']
-            )->setRuleType(Rule::AUTOFILTER_RULETYPE_DATEGROUP);
+            $dateGroupItem = $dateGroupItemx->/** @scrutinizer ignore-call */ attributes();
+            if ($dateGroupItem !== null) {
+                $column->createRule()->setRule(
+                    '',
+                    [
+                        'year' => (string) $dateGroupItem['year'],
+                        'month' => (string) $dateGroupItem['month'],
+                        'day' => (string) $dateGroupItem['day'],
+                        'hour' => (string) $dateGroupItem['hour'],
+                        'minute' => (string) $dateGroupItem['minute'],
+                        'second' => (string) $dateGroupItem['second'],
+                    ],
+                    (string) $dateGroupItem['dateTimeGrouping']
+                )->setRuleType(Rule::AUTOFILTER_RULETYPE_DATEGROUP);
+            }
         }
     }
 
@@ -98,15 +106,17 @@ private function readCustomAutoFilter(?SimpleXMLElement $filterColumn, Column $c
         if (isset($filterColumn, $filterColumn->customFilters)) {
             $column->setFilterType(Column::AUTOFILTER_FILTERTYPE_CUSTOMFILTER);
             $customFilters = $filterColumn->customFilters;
+            $attributes = $customFilters->attributes();
             //    Custom filters can an AND or an OR join;
             //        and there should only ever be one or two entries
-            if ((isset($customFilters['and'])) && ((string) $customFilters['and'] === '1')) {
+            if ((isset($attributes['and'])) && ((string) $attributes['and'] === '1')) {
                 $column->setJoin(Column::AUTOFILTER_COLUMN_JOIN_AND);
             }
             foreach ($customFilters->customFilter as $filterRule) {
+                $attr2 = $filterRule->/** @scrutinizer ignore-call */ attributes() ?? ['operator' => '', 'val' => ''];
                 $column->createRule()->setRule(
-                    (string) $filterRule['operator'],
-                    (string) $filterRule['val']
+                    (string) $attr2['operator'],
+                    (string) $attr2['val']
                 )->setRuleType(Rule::AUTOFILTER_RULETYPE_CUSTOMFILTER);
             }
         }
@@ -119,16 +129,17 @@ private function readDynamicAutoFilter(?SimpleXMLElement $filterColumn, Column $
             //    We should only ever have one dynamic filter
             foreach ($filterColumn->dynamicFilter as $filterRule) {
                 //    Operator is undefined, but always treated as EQUAL
+                $attr2 = $filterRule->/** @scrutinizer ignore-call */ attributes() ?? [];
                 $column->createRule()->setRule(
                     '',
-                    (string) $filterRule['val'],
-                    (string) $filterRule['type']
+                    (string) ($attr2['val'] ?? ''),
+                    (string) ($attr2['type'] ?? '')
                 )->setRuleType(Rule::AUTOFILTER_RULETYPE_DYNAMICFILTER);
-                if (isset($filterRule['val'])) {
-                    $column->setAttribute('val', (string) $filterRule['val']);
+                if (isset($attr2['val'])) {
+                    $column->setAttribute('val', (string) $attr2['val']);
                 }
-                if (isset($filterRule['maxVal'])) {
-                    $column->setAttribute('maxVal', (string) $filterRule['maxVal']);
+                if (isset($attr2['maxVal'])) {
+                    $column->setAttribute('maxVal', (string) $attr2['maxVal']);
                 }
             }
         }
@@ -140,15 +151,16 @@ private function readTopTenAutoFilter(?SimpleXMLElement $filterColumn, Column $c
             $column->setFilterType(Column::AUTOFILTER_FILTERTYPE_TOPTENFILTER);
             //    We should only ever have one top10 filter
             foreach ($filterColumn->top10 as $filterRule) {
+                $attr2 = $filterRule->/** @scrutinizer ignore-call */ attributes() ?? [];
                 $column->createRule()->setRule(
                     (
-                        ((isset($filterRule['percent'])) && ((string) $filterRule['percent'] === '1'))
+                        ((isset($attr2['percent'])) && ((string) $attr2['percent'] === '1'))
                         ? Rule::AUTOFILTER_COLUMN_RULE_TOPTEN_PERCENT
                         : Rule::AUTOFILTER_COLUMN_RULE_TOPTEN_BY_VALUE
                     ),
-                    (string) $filterRule['val'],
+                    (string) ($attr2['val'] ?? ''),
                     (
-                        ((isset($filterRule['top'])) && ((string) $filterRule['top'] === '1'))
+                        ((isset($attr2['top'])) && ((string) $attr2['top'] === '1'))
                         ? Rule::AUTOFILTER_COLUMN_RULE_TOPTEN_TOP
                         : Rule::AUTOFILTER_COLUMN_RULE_TOPTEN_BOTTOM
                     )

src/PhpSpreadsheet/Reader/Xlsx/TableReader.php

@@ -19,6 +19,9 @@ class TableReader
      */
     private $tableXml;
 
+    /** @var array|SimpleXMLElement */
+    private $tableAttributes;
+
     public function __construct(Worksheet $workSheet, SimpleXMLElement $tableXml)
     {
         $this->worksheet = $workSheet;
@@ -30,28 +33,29 @@ public function __construct(Worksheet $workSheet, SimpleXMLElement $tableXml)
      */
     public function load(): void
     {
+        $this->tableAttributes = $this->tableXml->attributes() ?? [];
         // Remove all "$" in the table range
-        $tableRange = (string) preg_replace('/\$/', '', $this->tableXml['ref'] ?? '');
+        $tableRange = (string) preg_replace('/\$/', '', $this->tableAttributes['ref'] ?? '');
         if (strpos($tableRange, ':') !== false) {
-            $this->readTable($tableRange, $this->tableXml);
+            $this->readTable($tableRange);
         }
     }
 
     /**
      * Read Table from xml.
      */
-    private function readTable(string $tableRange, SimpleXMLElement $tableXml): void
+    private function readTable(string $tableRange): void
     {
         $table = new Table($tableRange);
-        $table->setName((string) $tableXml['displayName']);
-        $table->setShowHeaderRow((string) $tableXml['headerRowCount'] !== '0');
-        $table->setShowTotalsRow((string) $tableXml['totalsRowCount'] === '1');
+        $table->setName((string) ($this->tableAttributes['displayName'] ?? ''));
+        $table->setShowHeaderRow(((string) ($this->tableAttributes['headerRowCount'] ?? '')) !== '0');
+        $table->setShowTotalsRow(((string) ($this->tableAttributes['totalsRowCount'] ?? '')) === '1');
 
-        $this->readTableAutoFilter($table, $tableXml->autoFilter);
-        $this->readTableColumns($table, $tableXml->tableColumns);
-        $this->readTableStyle($table, $tableXml->tableStyleInfo);
+        $this->readTableAutoFilter($table, $this->tableXml->autoFilter);
+        $this->readTableColumns($table, $this->tableXml->tableColumns);
+        $this->readTableStyle($table, $this->tableXml->tableStyleInfo);
 
-        (new AutoFilter($table, $tableXml))->load();
+        (new AutoFilter($table, $this->tableXml))->load();
         $this->worksheet->addTable($table);
     }
 
@@ -67,8 +71,9 @@ private function readTableAutoFilter(Table $table, SimpleXMLElement $autoFilterX
         }
 
         foreach ($autoFilterXml->filterColumn as $filterColumn) {
-            $column = $table->getColumnByOffset((int) $filterColumn['colId']);
-            $column->setShowFilterButton((string) $filterColumn['hiddenButton'] !== '1');
+            $attributes = $filterColumn->/** @scrutinizer ignore-call */ attributes() ?? ['colId' => 0, 'hiddenButton' => 0];
+            $column = $table->getColumnByOffset((int) $attributes['colId']);
+            $column->setShowFilterButton(((string) $attributes['hiddenButton']) !== '1');
         }
     }
 
@@ -79,15 +84,16 @@ private function readTableColumns(Table $table, SimpleXMLElement $tableColumnsXm
     {
         $offset = 0;
         foreach ($tableColumnsXml->tableColumn as $tableColumn) {
+            $attributes = $tableColumn->/** @scrutinizer ignore-call */ attributes() ?? ['totalsRowLabel' => 0, 'totalsRowFunction' => 0];
             $column = $table->getColumnByOffset($offset++);
 
             if ($table->getShowTotalsRow()) {
-                if ($tableColumn['totalsRowLabel']) {
-                    $column->setTotalsRowLabel((string) $tableColumn['totalsRowLabel']);
+                if ($attributes['totalsRowLabel']) {
+                    $column->setTotalsRowLabel((string) $attributes['totalsRowLabel']);
                 }
 
-                if ($tableColumn['totalsRowFunction']) {
-                    $column->setTotalsRowFunction((string) $tableColumn['totalsRowFunction']);
+                if ($attributes['totalsRowFunction']) {
+                    $column->setTotalsRowFunction((string) $attributes['totalsRowFunction']);
                 }
             }
 
@@ -103,11 +109,14 @@ private function readTableColumns(Table $table, SimpleXMLElement $tableColumnsXm
     private function readTableStyle(Table $table, SimpleXMLElement $tableStyleInfoXml): void
     {
         $tableStyle = new TableStyle();
-        $tableStyle->setTheme((string) $tableStyleInfoXml['name']);
-        $tableStyle->setShowRowStripes((string) $tableStyleInfoXml['showRowStripes'] === '1');
-        $tableStyle->setShowColumnStripes((string) $tableStyleInfoXml['showColumnStripes'] === '1');
-        $tableStyle->setShowFirstColumn((string) $tableStyleInfoXml['showFirstColumn'] === '1');
-        $tableStyle->setShowLastColumn((string) $tableStyleInfoXml['showLastColumn'] === '1');
+        $attributes = $tableStyleInfoXml->attributes();
+        if ($attributes !== null) {
+            $tableStyle->setTheme((string) $attributes['name']);
+            $tableStyle->setShowRowStripes((string) $attributes['showRowStripes'] === '1');
+            $tableStyle->setShowColumnStripes((string) $attributes['showColumnStripes'] === '1');
+            $tableStyle->setShowFirstColumn((string) $attributes['showFirstColumn'] === '1');
+            $tableStyle->setShowLastColumn((string) $attributes['showLastColumn'] === '1');
+        }
         $table->setStyle($tableStyle);
     }
 }

tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3665Test.php

@@ -0,0 +1,62 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+
+class Issue3665Test extends \PHPUnit\Framework\TestCase
+{
+    /**
+     * @var string
+     */
+    private static $testbook = 'tests/data/Reader/XLSX/issue.3665.xlsx';
+
+    public function testPreliminaries(): void
+    {
+        $file = 'zip://';
+        $file .= self::$testbook;
+        $file .= '#xl/tables/table1.xml';
+        $data = file_get_contents($file);
+        // confirm that file contains expected namespaced xml tag
+        if ($data === false) {
+            self::fail('Unable to read table file');
+        } else {
+            self::assertStringContainsString('<x:table id="5" name="Table5" displayName="Table5" ref="A3:M4" tableType="xml" totalsRowShown="0" xmlns:x="http://schemas.openxmlformats.org/spreadsheetml/2006/main">', $data);
+            self::assertStringContainsString('<x:autoFilter ref="A3:M4" />', $data);
+        }
+
+        $file = 'zip://';
+        $file .= self::$testbook;
+        $file .= '#xl/worksheets/_rels/sheet1.xml.rels';
+        $data = file_get_contents($file);
+        // confirm absolute path as reference
+        if ($data === false) {
+            self::fail('Unable to read rels file');
+        } else {
+            self::assertStringContainsString('Target="/xl/comments1.xml"', $data);
+        }
+    }
+
+    public function testTable(): void
+    {
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load(self::$testbook);
+        $sheet = $spreadsheet->getActiveSheet();
+        $tables = $sheet->getTableCollection();
+        self::assertCount(1, $tables);
+        $table = $tables->offsetGet(0);
+        if ($table === null) {
+            self::fail('Unexpected failure obtaining table');
+        } else {
+            self::assertEquals('Table5', $table->getName());
+            self::assertEquals('A3:M4', $table->getRange());
+            self::assertTrue($table->getAllowFilter());
+            self::assertSame('A3:M4', $table->getAutoFilter()->getRange());
+        }
+        $comment = $sheet->getComment('A3');
+        self::assertSame('Code20', (string) $comment);
+        $comment = $sheet->getComment('B3');
+        self::assertSame('Text100', (string) $comment);
+        $spreadsheet->disconnectWorksheets();
+    }
+}