Unexpected Namespacing in rels File (#3722)

oleibman · web-flow · commit c5527ba53400 · 2023-09-12T19:13:03.000-07:00
* Unexpected Namespacing in rels File Fix #3720. Third-party product created a spreadsheet which PhpSpreadsheet could not read because of unexpected namespacing in workbook.xml.rels. The file which demonstrated the problem was attached to #3423, however I do not believe it was related to the original problem. Nevertheless, the original issue specifically called out Protection, so I put some Protection tests in the validation test for the fix. In doing so, I found that Style/Protection is particularly confusing. Its properties will often have the value `inherit`, which isn't all that helpful; and, even when the `locked` value is `protected`, the cell won't actually be locked unless the sheet is protected as well. The `hidden` property is even more obscure - it applies only to formulas, and refers to hiding the property on the formula bar, not in the cell. I have added methods `isLocked` and `isHiddenOnFormulaBar` to `Cell`. I corrected the docs to explain this. And, as long as I was looking at the docs, I corrected some examples to use `getHighestDataRow/Column` rather than `getHighestRow/Column`, a frequent problem for users (e.g. #3721). As a side note, the change to Cell.php is my first use of the nullsafe operator. This is one of many new options available now that we require Php8.0+. * Minor Simplifications * Scrutinizer It's being silly again. In many tests, we test a variable for non-null, then use that variable later and Scrutinizer knows it's not null. Not here. Oh well. * Add Methods Test if protected without allocating cell if it doesn't exist.
diff --git a/docs/topics/accessing-cells.md b/docs/topics/accessing-cells.md
@@ -479,8 +479,8 @@ $spreadsheet = $reader->load("test.xlsx");
 
 $worksheet = $spreadsheet->getActiveSheet();
 // Get the highest row and column numbers referenced in the worksheet
-$highestRow = $worksheet->getHighestRow(); // e.g. 10
-$highestColumn = $worksheet->getHighestColumn(); // e.g 'F'
+$highestRow = $worksheet->getHighestDataRow(); // e.g. 10
+$highestColumn = $worksheet->getHighestDataColumn(); // e.g 'F'
 $highestColumnIndex = \PhpOffice\PhpSpreadsheet\Cell\Coordinate::columnIndexFromString($highestColumn); // e.g. 5
 
 echo '<table>' . "\n";
@@ -505,8 +505,8 @@ $spreadsheet = $reader->load("test.xlsx");
 
 $worksheet = $spreadsheet->getActiveSheet();
 // Get the highest row number and column letter referenced in the worksheet
-$highestRow = $worksheet->getHighestRow(); // e.g. 10
-$highestColumn = $worksheet->getHighestColumn(); // e.g 'F'
+$highestRow = $worksheet->getHighestDataRow(); // e.g. 10
+$highestColumn = $worksheet->getHighestDataColumn(); // e.g 'F'
 // Increment the highest column letter
 $highestColumn++;
 
diff --git a/docs/topics/recipes.md b/docs/topics/recipes.md
@@ -1309,12 +1309,17 @@ when setting a new password.
 
 ### Cell
 
-An example on setting cell security:
+An example on setting cell security.
+Note that cell security is honored only when sheet is protected.
+Also note that the `hidden` property applies only to formulas,
+and tells whether the formula is hidden on the formula bar,
+not in the cell.
 
 ```php
 $spreadsheet->getActiveSheet()->getStyle('B1')
     ->getProtection()
-    ->setLocked(\PhpOffice\PhpSpreadsheet\Style\Protection::PROTECTION_UNPROTECTED);
+    ->setLocked(\PhpOffice\PhpSpreadsheet\Style\Protection::PROTECTION_UNPROTECTED)
+    ->setHidden(\PhpOffice\PhpSpreadsheet\Style\Protection::PROTECTION_PROTECTED);
 ```
 
 ## Reading protected spreadsheet
diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php
@@ -11,6 +11,7 @@
 use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
 use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\CellStyleAssessor;
 use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
+use PhpOffice\PhpSpreadsheet\Style\Protection;
 use PhpOffice\PhpSpreadsheet\Style\Style;
 use PhpOffice\PhpSpreadsheet\Worksheet\Table;
 use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
@@ -778,4 +779,29 @@ public function getIgnoredErrors(): IgnoredErrors
     {
         return $this->ignoredErrors;
     }
+
+    public function isLocked(): bool
+    {
+        $protected = $this->parent?->getParent()?->getProtection()?->getSheet();
+        if ($protected !== true) {
+            return false;
+        }
+        $locked = $this->getStyle()->getProtection()->getLocked();
+
+        return $locked !== Protection::PROTECTION_UNPROTECTED;
+    }
+
+    public function isHiddenOnFormulaBar(): bool
+    {
+        if ($this->getDataType() !== DataType::TYPE_FORMULA) {
+            return false;
+        }
+        $protected = $this->parent?->getParent()?->getProtection()?->getSheet();
+        if ($protected !== true) {
+            return false;
+        }
+        $hidden = $this->getStyle()->getProtection()->getHidden();
+
+        return $hidden !== Protection::PROTECTION_UNPROTECTED;
+    }
 }
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -220,7 +220,7 @@ public function listWorksheetInfo(string $filename): array
                 $relTarget = (string) $rel['Target'];
                 $dir = dirname($relTarget);
                 $namespace = dirname($relType);
-                $relsWorkbook = $this->loadZip("$dir/_rels/" . basename($relTarget) . '.rels', '');
+                $relsWorkbook = $this->loadZip("$dir/_rels/" . basename($relTarget) . '.rels', Namespaces::RELATIONSHIPS);
 
                 $worksheets = [];
                 foreach ($relsWorkbook->Relationship as $elex) {
@@ -533,7 +533,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                     $dir = dirname($relTarget);
 
                     // Do not specify namespace in next stmt - do it in Xpath
-                    $relsWorkbook = $this->loadZip("$dir/_rels/" . basename($relTarget) . '.rels', '');
+                    $relsWorkbook = $this->loadZip("$dir/_rels/" . basename($relTarget) . '.rels', Namespaces::RELATIONSHIPS);
                     $relsWorkbook->registerXPathNamespace('rel', Namespaces::RELATIONSHIPS);
 
                     $worksheets = [];
@@ -1319,9 +1319,10 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                 $drawingFilename = substr($drawingFilename, 5);
                             }
                             if ($zip->locateName($drawingFilename) !== false) {
-                                $relsWorksheet = $this->loadZipNoNamespace($drawingFilename, Namespaces::RELATIONSHIPS);
+                                $relsWorksheet = $this->loadZip($drawingFilename, Namespaces::RELATIONSHIPS);
                                 $drawings = [];
-                                foreach ($relsWorksheet->Relationship as $ele) {
+                                foreach ($relsWorksheet->Relationship as $elex) {
+                                    $ele = self::getAttributes($elex);
                                     if ((string) $ele['Type'] === "$xmlNamespaceBase/drawing") {
                                         $eleTarget = (string) $ele['Target'];
                                         if (str_starts_with($eleTarget, '/xl/')) {
@@ -1339,12 +1340,13 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                         $drawingRelId = (string) self::getArrayItem(self::getAttributes($drawing, $xmlNamespaceBase), 'id');
                                         $fileDrawing = $drawings[$drawingRelId];
                                         $drawingFilename = dirname($fileDrawing) . '/_rels/' . basename($fileDrawing) . '.rels';
-                                        $relsDrawing = $this->loadZipNoNamespace($drawingFilename, $xmlNamespaceBase);
+                                        $relsDrawing = $this->loadZip($drawingFilename, Namespaces::RELATIONSHIPS);
 
                                         $images = [];
                                         $hyperlinks = [];
                                         if ($relsDrawing && $relsDrawing->Relationship) {
-                                            foreach ($relsDrawing->Relationship as $ele) {
+                                            foreach ($relsDrawing->Relationship as $elex) {
+                                                $ele = self::getAttributes($elex);
                                                 $eleType = (string) $ele['Type'];
                                                 if ($eleType === Namespaces::HYPERLINK) {
                                                     $hyperlinks[(string) $ele['Id']] = (string) $ele['Target'];
@@ -1584,7 +1586,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
 
                                     // store original rId of drawing files
                                     $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['drawingOriginalIds'] = [];
-                                    foreach ($relsWorksheet->Relationship as $ele) {
+                                    foreach ($relsWorksheet->Relationship as $elex) {
+                                        $ele = self::getAttributes($elex);
                                         if ((string) $ele['Type'] === "$xmlNamespaceBase/drawing") {
                                             $drawingRelId = (string) $ele['Id'];
                                             $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['drawingOriginalIds'][(string) $ele['Target']] = $drawingRelId;
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -29,6 +29,7 @@
 use PhpOffice\PhpSpreadsheet\Style\Color;
 use PhpOffice\PhpSpreadsheet\Style\Conditional;
 use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
+use PhpOffice\PhpSpreadsheet\Style\Protection as StyleProtection;
 use PhpOffice\PhpSpreadsheet\Style\Style;
 
 class Worksheet implements IComparable
@@ -1486,6 +1487,11 @@ public function rowDimensionExists(int $row): bool
         return isset($this->rowDimensions[$row]);
     }
 
+    public function columnDimensionExists(string $column): bool
+    {
+        return isset($this->columnDimensions[$column]);
+    }
+
     /**
      * Get column dimension at a specific column.
      *
@@ -3842,4 +3848,53 @@ public function isRowVisible(int $row): bool
     {
         return !$this->rowDimensionExists($row) || $this->getRowDimension($row)->getVisible();
     }
+
+    /**
+     * Same as Cell->isLocked, but without creating cell if it doesn't exist.
+     */
+    public function isCellLocked(string $coordinate): bool
+    {
+        if ($this->getProtection()->getsheet() !== true) {
+            return false;
+        }
+        if ($this->cellExists($coordinate)) {
+            return $this->getCell($coordinate)->isLocked();
+        }
+        $spreadsheet = $this->parent;
+        $xfIndex = $this->getXfIndex($coordinate);
+        if ($spreadsheet === null || $xfIndex === null) {
+            return true;
+        }
+
+        return $spreadsheet->getCellXfByIndex($xfIndex)->getProtection()->getLocked() !== StyleProtection::PROTECTION_UNPROTECTED;
+    }
+
+    /**
+     * Same as Cell->isHiddenOnFormulaBar, but without creating cell if it doesn't exist.
+     */
+    public function isCellHiddenOnFormulaBar(string $coordinate): bool
+    {
+        if ($this->cellExists($coordinate)) {
+            return $this->getCell($coordinate)->isHiddenOnFormulaBar();
+        }
+
+        // cell doesn't exist, therefore isn't a formula,
+        // therefore isn't hidden on formula bar.
+        return false;
+    }
+
+    private function getXfIndex(string $coordinate): ?int
+    {
+        [$column, $row] = Coordinate::coordinateFromString($coordinate);
+        $row = (int) $row;
+        $xfIndex = null;
+        if ($this->rowDimensionExists($row)) {
+            $xfIndex = $this->getRowDimension($row)->getXfIndex();
+        }
+        if ($xfIndex === null && $this->ColumnDimensionExists($column)) {
+            $xfIndex = $this->getColumnDimension($column)->getXfIndex();
+        }
+
+        return $xfIndex;
+    }
 }
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3720Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3720Test.php
@@ -0,0 +1,113 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PhpOffice\PhpSpreadsheet\Style\Protection;
+use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
+
+class Issue3720Test extends \PHPUnit\Framework\TestCase
+{
+    private static string $testbook = 'tests/data/Reader/XLSX/issue.3720.xlsx';
+
+    public function testPreliminaries(): void
+    {
+        $file = 'zip://';
+        $file .= self::$testbook;
+        $file .= '#xl/_rels/workbook.xml.rels';
+        $data = file_get_contents($file);
+        // confirm that file contains expected namespaced xml tag
+        if ($data === false) {
+            self::fail('Unable to read file');
+        } else {
+            self::assertStringContainsString('<ns3:Relationships ', $data);
+        }
+    }
+
+    public function testInfo(): void
+    {
+        $reader = new Xlsx();
+        $workSheetInfo = $reader->listWorkSheetInfo(self::$testbook);
+        $info1 = $workSheetInfo[1];
+        self::assertEquals('Welcome', $info1['worksheetName']);
+        self::assertEquals('H', $info1['lastColumnLetter']);
+        self::assertEquals(7, $info1['lastColumnIndex']);
+        self::assertEquals(49, $info1['totalRows']);
+        self::assertEquals(8, $info1['totalColumns']);
+    }
+
+    public function testSheetNames(): void
+    {
+        $reader = new Xlsx();
+        $worksheetNames = $reader->listWorksheetNames(self::$testbook);
+        $expected = [
+            'Data',
+            'Welcome',
+            'Sheet 1',
+            'Sheet 2',
+            'Sheet 3',
+            'Sheet 4',
+            'Sheet 5',
+            'Sheet 6',
+            'Sheet 7',
+            'Sheet 8',
+            'Sheet 9',
+            'Sheet 10',
+        ];
+        self::assertEquals($expected, $worksheetNames);
+    }
+
+    public function testLoadXlsx(): void
+    {
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load(self::$testbook);
+        $sheets = $spreadsheet->getAllSheets();
+        self::assertCount(12, $sheets);
+        $sheet = $spreadsheet->getSheetByNameOrThrow('Sheet 1');
+        $sheetProtection = $sheet->getProtection();
+        self::assertTrue($sheetProtection->getSheet());
+        self::assertSame(' FILL IN WHITE CELLS ONLY', $sheet->getCell('B3')->getValue());
+        // inherit because cell style is inherit.
+        // effectively protected because sheet is locked.
+        self::assertTrue($sheet->cellExists('A12'));
+        self::assertSame(Protection::PROTECTION_INHERIT, $sheet->getStyle('A12')->getProtection()->getLocked());
+        self::assertTrue($sheet->getCell('A12')->isLocked());
+        // unprotected because column is unprotected (no cell or row dimension style)
+        self::assertFalse($sheet->cellExists('B12'));
+        self::assertFalse($sheet->rowDimensionExists(12));
+        self::assertTrue($sheet->columnDimensionExists('B'));
+        $dxf = $sheet->getColumnDimension('B')->getXfIndex();
+        if ($dxf === null) {
+            self::fail('Unexpected null column xfIndex');
+        } else {
+            self::assertSame(Protection::PROTECTION_UNPROTECTED, $spreadsheet->getCellXfByIndex($dxf)->getProtection()->getLocked());
+        }
+        self::assertFalse($sheet->getCell('B12')->isLocked());
+        // inherit because cell doesn't exist, no row dimension, no column dimension.
+        // effectively protected because sheet is locked.
+        self::assertFalse($sheet->cellExists('W8'));
+        self::assertFalse($sheet->rowDimensionExists(8));
+        self::assertFalse($sheet->columnDimensionExists('W'));
+        self::assertTrue($sheet->getCell('W8')->isLocked());
+        // inherit because cell doesn't exist, row dimension exists without style, no column dimension.
+        // effectively protected because sheet is locked.
+        self::assertFalse($sheet->cellExists('X11'));
+        self::assertTrue($sheet->rowDimensionExists(11));
+        $dxf = $sheet->getRowDimension(11)->getXfIndex();
+        self::assertNull($dxf);
+        self::assertFalse($sheet->columnDimensionExists('X'));
+        self::assertTrue($sheet->getCell('X11')->isLocked());
+
+        $sheet = $spreadsheet->getSheetByNameOrThrow('Welcome');
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $draw0 = $drawings[0] ?? new Drawing();
+        self::assertSame('Picture 1', $draw0->getName());
+        self::assertSame('C3', $draw0->getCoordinates());
+        self::assertSame('C10', $draw0->getCoordinates2());
+        self::assertSame('oneCell', $draw0->getEditAs());
+        $spreadsheet->disconnectWorksheets();
+    }
+}
diff --git a/tests/PhpSpreadsheetTests/Worksheet/Protection2Test.php b/tests/PhpSpreadsheetTests/Worksheet/Protection2Test.php
diff --git a/tests/data/Reader/XLSX/issue.3720.xlsx b/tests/data/Reader/XLSX/issue.3720.xlsx
