Bugfix #1858; Apply stricter scoping rules to named range/cell access

src/PhpSpreadsheet/Worksheet/Worksheet.php

@@ -1195,11 +1195,12 @@ public function getCell($pCoordinate, $createIfNotExists = true)
             (!preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $pCoordinate, $matches)) &&
             (preg_match('/^' . Calculation::CALCULATION_REGEXP_DEFINEDNAME . '$/i', $pCoordinate, $matches))
         ) {
-            $namedRange = DefinedName::resolveName($pCoordinate, $this);
+            $namedRange = $this->validateNamedRange($pCoordinate, true);
             if ($namedRange !== null) {
-                $pCoordinate = str_replace('$', '', $namedRange->getValue());
+                $cellCoordinate = ltrim(substr($namedRange->getValue(), strrpos($namedRange->getValue(), '!')), '!');
+                $cellCoordinate = str_replace('$', '', $cellCoordinate);
 
-                return $namedRange->getWorksheet()->getCell($pCoordinate, $createIfNotExists);
+                return $namedRange->getWorksheet()->getCell($cellCoordinate, $createIfNotExists);
             }
         }
 
@@ -1295,18 +1296,12 @@ public function cellExists($pCoordinate)
             (!preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $pCoordinate, $matches)) &&
             (preg_match('/^' . Calculation::CALCULATION_REGEXP_DEFINEDNAME . '$/i', $pCoordinate, $matches))
         ) {
-            $namedRange = DefinedName::resolveName($pCoordinate, $this);
+            $namedRange = $this->validateNamedRange($pCoordinate, true);
             if ($namedRange !== null) {
-                $pCoordinate = str_replace('$', '', $namedRange->getValue());
-                if ($this->getHashCode() != $namedRange->getWorksheet()->getHashCode()) {
-                    if (!$namedRange->getLocalOnly()) {
-                        return $namedRange->getWorksheet()->cellExists($pCoordinate);
-                    }
+                $cellCoordinate = ltrim(substr($namedRange->getValue(), strrpos($namedRange->getValue(), '!')), '!');
+                $cellCoordinate = str_replace('$', '', $cellCoordinate);
 
-                    throw new Exception('Named range ' . $namedRange->getName() . ' is not accessible from within sheet ' . $this->getTitle());
-                }
-            } else {
-                return false;
+                return $namedRange->getWorksheet()->cellExists($cellCoordinate);
             }
         }
 
@@ -2551,10 +2546,42 @@ public function rangeToArray($pRange, $nullValue = null, $calculateFormulas = tr
         return $returnValue;
     }
 
+    private function validateNamedRange(string $definedName, bool $returnNullIfInvalid = false): ?DefinedName
+    {
+        $namedRange = DefinedName::resolveName($definedName, $this);
+        if ($namedRange === null) {
+            if ($returnNullIfInvalid) {
+                return null;
+            }
+
+            throw new Exception('Named Range ' . $definedName . ' does not exist.');
+        }
+
+        if ($namedRange->isFormula()) {
+            if ($returnNullIfInvalid) {
+                return null;
+            }
+
+            throw new Exception('Defined Named ' . $definedName . ' is a formula, not a range or cell.');
+        }
+
+        if ($namedRange->getLocalOnly() && $this->getHashCode() !== $namedRange->getWorksheet()->getHashCode()) {
+            if ($returnNullIfInvalid) {
+                return null;
+            }
+
+            throw new Exception(
+                'Named range ' . $definedName . ' is not accessible from within sheet ' . $this->getTitle()
+            );
+        }
+
+        return $namedRange;
+    }
+
     /**
      * Create array from a range of cells.
      *
-     * @param string $pNamedRange Name of the Named Range
+     * @param string $definedName The Named Range that should be returned
      * @param mixed $nullValue Value returned in the array entry if a cell doesn't exist
      * @param bool $calculateFormulas Should formulas be calculated?
      * @param bool $formatData Should formatting be applied to cell values?
@@ -2563,17 +2590,14 @@ public function rangeToArray($pRange, $nullValue = null, $calculateFormulas = tr
      *
      * @return array
      */
-    public function namedRangeToArray($pNamedRange, $nullValue = null, $calculateFormulas = true, $formatData = true, $returnCellRef = false)
+    public function namedRangeToArray(string $definedName, $nullValue = null, $calculateFormulas = true, $formatData = true, $returnCellRef = false)
     {
-        $namedRange = DefinedName::resolveName($pNamedRange, $this);
-        if ($namedRange !== null) {
-            $pWorkSheet = $namedRange->getWorksheet();
-            $pCellRange = str_replace('$', '', $namedRange->getValue());
-
-            return $pWorkSheet->rangeToArray($pCellRange, $nullValue, $calculateFormulas, $formatData, $returnCellRef);
-        }
+        $namedRange = $this->validateNamedRange($definedName);
+        $workSheet = $namedRange->getWorksheet();
+        $cellRange = ltrim(substr($namedRange->getValue(), strrpos($namedRange->getValue(), '!')), '!');
+        $cellRange = str_replace('$', '', $cellRange);
 
-        throw new Exception('Named Range ' . $pNamedRange . ' does not exist.');
+        return $workSheet->rangeToArray($cellRange, $nullValue, $calculateFormulas, $formatData, $returnCellRef);
     }
 
     /**

tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php

@@ -0,0 +1,143 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Exception;
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PhpOffice\PhpSpreadsheet\Settings;
+use PHPUnit\Framework\TestCase;
+
+class WorksheetNamedRangesTest extends TestCase
+{
+    protected $spreadsheet;
+
+    protected function setUp(): void
+    {
+        Settings::setLibXmlLoaderOptions(null); // reset to default options
+
+        $reader = new Xlsx();
+        $this->spreadsheet = $reader->load('tests/data/Worksheet/namedRangeTest.xlsx');
+    }
+
+    public function testCellExists(): void
+    {
+        $namedCell = 'GREETING';
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cellExists = $worksheet->cellExists($namedCell);
+        self::assertTrue($cellExists);
+    }
+
+    public function testCellNotExists(): void
+    {
+        $namedCell = 'GOODBYE';
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cellExists = $worksheet->cellExists($namedCell);
+        self::assertFalse($cellExists);
+    }
+
+    public function testCellExistsInvalidScope(): void
+    {
+        $namedCell = 'Result';
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cellExists = $worksheet->cellExists($namedCell);
+        self::assertFalse($cellExists);
+    }
+
+    public function testCellExistsRange(): void
+    {
+        $namedRange = 'Range1';
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Cell coordinate can not be a range of cells');
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $worksheet->cellExists($namedRange);
+    }
+
+    public function testGetCell(): void
+    {
+        $namedCell = 'GREETING';
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cell = $worksheet->getCell($namedCell);
+        self::assertSame('Hello', $cell->getValue());
+    }
+
+    public function testGetCellNotExists(): void
+    {
+        $namedCell = 'GOODBYE';
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage("Invalid cell coordinate {$namedCell}");
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $worksheet->getCell($namedCell);
+    }
+
+    public function testGetCellInvalidScope(): void
+    {
+        $namedCell = 'Result';
+        $ucNamedCell = strtoupper($namedCell);
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage("Invalid cell coordinate {$ucNamedCell}");
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $worksheet->getCell($namedCell);
+    }
+
+    public function testGetCellLocalScoped(): void
+    {
+        $namedCell = 'Result';
+
+        $this->spreadsheet->setActiveSheetIndexByName('Sheet2');
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cell = $worksheet->getCell($namedCell);
+        self::assertSame(8, $cell->getCalculatedValue());
+    }
+
+    public function testGetCellNamedFormula(): void
+    {
+        $namedCell = 'Result';
+
+        $this->spreadsheet->setActiveSheetIndexByName('Sheet2');
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $cell = $worksheet->getCell($namedCell);
+        self::assertSame(8, $cell->getCalculatedValue());
+    }
+
+    public function testGetCellWithNamedRange(): void
+    {
+        $namedCell = 'Range1';
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Cell coordinate can not be a range of cells');
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $worksheet->getCell($namedCell);
+    }
+
+    public function testNamedRangeToArray(): void
+    {
+        $namedRange = 'Range1';
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $rangeData = $worksheet->namedRangeToArray($namedRange);
+        self::assertSame([[1, 2, 3]], $rangeData);
+    }
+
+    public function testInvalidNamedRangeToArray(): void
+    {
+        $namedRange = 'Range2';
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage("Named Range {$namedRange} does not exist");
+
+        $worksheet = $this->spreadsheet->getActiveSheet();
+        $rangeData = $worksheet->namedRangeToArray($namedRange);
+        self::assertSame([[1, 2, 3]], $rangeData);
+    }
+}