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.
This commit is contained in:
oleibman
2023-08-10 16:07:38 -07:00
committed by GitHub
parent 3400110e24
commit efd04e6b81
5 changed files with 158 additions and 62 deletions
+20 -7
View File
@@ -793,6 +793,14 @@ class Xlsx extends BaseReader
$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 @@ class Xlsx extends BaseReader
}
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 @@ class Xlsx extends BaseReader
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 @@ class Xlsx extends BaseReader
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 @@ class Xlsx extends BaseReader
$relationshipFilePath = File::realpath($relationshipFilePath);
if ($this->fileExistsInArchive($this->zip, $relationshipFilePath)) {
$tableXml = $this->loadZip($relationshipFilePath);
$tableXml = $this->loadZip($relationshipFilePath, $namespaceTable);
(new TableReader($docSheet, $tableXml))->load();
}
}
+46 -34
View File
@@ -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 @@ class AutoFilter
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 @@ class AutoFilter
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 @@ class AutoFilter
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 @@ class AutoFilter
// 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 @@ class AutoFilter
$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
)
+30 -21
View File
@@ -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 @@ class TableReader
*/
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 @@ class TableReader
}
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 @@ class TableReader
{
$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 @@ class TableReader
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);
}
}
@@ -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();
}
}
Binary file not shown.