From 69edf61ed652595acfa8b38e994e811ad55003a2 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sat, 23 Apr 2022 13:08:06 +0200 Subject: [PATCH] Extract cell/range validations from Worksheet and move into a separate Worksheet/Validations class as public static methods Extract tryDefinedName logic from Worksheet, nd move into the Worksheet/Validations class as a public static method Apply stricter validation to autofilter range arguments --- src/PhpSpreadsheet/Calculation/Functions.php | 8 +- src/PhpSpreadsheet/Worksheet/AutoFilter.php | 23 ++- src/PhpSpreadsheet/Worksheet/Validations.php | 100 +++++++++++++ src/PhpSpreadsheet/Worksheet/Worksheet.php | 142 +++---------------- 4 files changed, 142 insertions(+), 131 deletions(-) create mode 100644 src/PhpSpreadsheet/Worksheet/Validations.php diff --git a/src/PhpSpreadsheet/Calculation/Functions.php b/src/PhpSpreadsheet/Calculation/Functions.php index 00d8a790..dc6ee82e 100644 --- a/src/PhpSpreadsheet/Calculation/Functions.php +++ b/src/PhpSpreadsheet/Calculation/Functions.php @@ -4,7 +4,6 @@ namespace PhpOffice\PhpSpreadsheet\Calculation; use PhpOffice\PhpSpreadsheet\Cell\Cell; use PhpOffice\PhpSpreadsheet\Shared\Date; -use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class Functions { @@ -686,12 +685,13 @@ class Functions // Uppercase coordinate $pCoordinatex = strtoupper($coordinate); // Eliminate leading equal sign - $pCoordinatex = Worksheet::pregReplace('/^=/', '', $pCoordinatex); + $pCoordinatex = (string) preg_replace('/^=/', '', $pCoordinatex); $defined = $spreadsheet->getDefinedName($pCoordinatex, $worksheet); if ($defined !== null) { $worksheet2 = $defined->getWorkSheet(); if (!$defined->isFormula() && $worksheet2 !== null) { - $coordinate = "'" . $worksheet2->getTitle() . "'!" . Worksheet::pregReplace('/^=/', '', $defined->getValue()); + $coordinate = "'" . $worksheet2->getTitle() . "'!" . + (string) preg_replace('/^=/', '', $defined->getValue()); } } @@ -700,7 +700,7 @@ class Functions public static function trimTrailingRange(string $coordinate): string { - return Worksheet::pregReplace('/:[\\w\$]+$/', '', $coordinate); + return (string) preg_replace('/:[\\w\$]+$/', '', $coordinate); } public static function trimSheetFromCellReference(string $coordinate): string diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter.php b/src/PhpSpreadsheet/Worksheet/AutoFilter.php index 2e6c8289..dd33d5d5 100644 --- a/src/PhpSpreadsheet/Worksheet/AutoFilter.php +++ b/src/PhpSpreadsheet/Worksheet/AutoFilter.php @@ -7,6 +7,7 @@ use DateTimeZone; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Internal\WildcardMatch; +use PhpOffice\PhpSpreadsheet\Cell\AddressRange; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Shared\Date; @@ -50,9 +51,18 @@ class AutoFilter /** * Create a new AutoFilter. + * + * @param AddressRange|array|string $range + * A simple string containing a Cell range like 'A1:E10' is permitted + * or passing in an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 8]), + * or an AddressRange object. */ - public function __construct(string $range = '', ?Worksheet $worksheet = null) + public function __construct($range = '', ?Worksheet $worksheet = null) { + if ($range !== '') { + [, $range] = Worksheet::extractSheetTitle(Validations::validateCellRange($range), true); + } + $this->range = $range; $this->workSheet = $worksheet; } @@ -92,12 +102,19 @@ class AutoFilter /** * Set AutoFilter Cell Range. + * + * @param AddressRange|array|string $range + * A simple string containing a Cell range like 'A1:E10' is permitted + * or passing in an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 8]), + * or an AddressRange object. */ - public function setRange(string $range): self + public function setRange($range = ''): self { $this->evaluated = false; // extract coordinate - [$worksheet, $range] = Worksheet::extractSheetTitle($range, true); + if ($range !== '') { + [, $range] = Worksheet::extractSheetTitle(Validations::validateCellRange($range), true); + } if (empty($range)) { // Discard all column rules $this->columns = []; diff --git a/src/PhpSpreadsheet/Worksheet/Validations.php b/src/PhpSpreadsheet/Worksheet/Validations.php new file mode 100644 index 00000000..b31b0813 --- /dev/null +++ b/src/PhpSpreadsheet/Worksheet/Validations.php @@ -0,0 +1,100 @@ +|CellAddress|string $cellAddress Coordinate of the cell as a string, eg: 'C5'; + * or as an array of [$columnIndex, $row] (e.g. [3, 5]), or a CellAddress object. + */ + public static function validateCellAddress($cellAddress): string + { + if (is_string($cellAddress)) { + [$worksheet, $address] = Worksheet::extractSheetTitle($cellAddress, true); +// if (!empty($worksheet) && $worksheet !== $this->getTitle()) { +// throw new Exception('Reference is not for this worksheet'); +// } + + return empty($worksheet) ? strtoupper($address) : $worksheet . '!' . strtoupper($address); + } + + if (is_array($cellAddress)) { + $cellAddress = CellAddress::fromColumnRowArray($cellAddress); + } + + return (string) $cellAddress; + } + + /** + * Validate a cell address or cell range. + * + * @param AddressRange|array|CellAddress|int|string $cellRange Coordinate of the cells as a string, eg: 'C5:F12'; + * or as an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 12]), + * or as a CellAddress or AddressRange object. + */ + public static function validateCellOrCellRange($cellRange): string + { + if (is_string($cellRange) || is_numeric($cellRange)) { + $cellRange = (string) $cellRange; + // Convert a single column reference like 'A' to 'A:A' + $cellRange = (string) preg_replace('/^([A-Z]+)$/', '${1}:${1}', $cellRange); + // Convert a single row reference like '1' to '1:1' + $cellRange = (string) preg_replace('/^(\d+)$/', '${1}:${1}', $cellRange); + } elseif (is_object($cellRange) && $cellRange instanceof CellAddress) { + $cellRange = new CellRange($cellRange, $cellRange); + } + + return self::validateCellRange($cellRange); + } + + /** + * Validate a cell range. + * + * @param AddressRange|array|string $cellRange Coordinate of the cells as a string, eg: 'C5:F12'; + * or as an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 12]), + * or as an AddressRange object. + */ + public static function validateCellRange($cellRange): string + { + if (is_string($cellRange)) { + [$worksheet, $addressRange] = Worksheet::extractSheetTitle($cellRange, true); + + // Convert Column ranges like 'A:C' to 'A1:C1048576' + $addressRange = (string) preg_replace('/^([A-Z]+):([A-Z]+)$/', '${1}1:${2}1048576', $addressRange); + // Convert Row ranges like '1:3' to 'A1:XFD3' + $addressRange = (string) preg_replace('/^(\\d+):(\\d+)$/', 'A${1}:XFD${2}', $addressRange); + + return empty($worksheet) ? strtoupper($addressRange) : $worksheet . '!' . strtoupper($addressRange); + } + + if (is_array($cellRange)) { + [$from, $to] = array_chunk($cellRange, 2); + $cellRange = new CellRange(CellAddress::fromColumnRowArray($from), CellAddress::fromColumnRowArray($to)); + } + + return (string) $cellRange; + } + + public static function definedNameToCoordinate(string $coordinate, Worksheet $worksheet): string + { + // Uppercase coordinate + $testCoordinate = strtoupper($coordinate); + // Eliminate leading equal sign + $testCoordinate = (string) preg_replace('/^=/', '', $coordinate); + $defined = $worksheet->getParent()->getDefinedName($testCoordinate, $worksheet); + if ($defined !== null) { + if ($defined->getWorksheet() === $worksheet && !$defined->isFormula()) { + $coordinate = (string) preg_replace('/^=/', '', $defined->getValue()); + } + } + + return $coordinate; + } +} diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index e4ce2ac3..70994be5 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -1067,7 +1067,7 @@ class Worksheet implements IComparable */ public function getHighestColumn($row = null) { - if (empty($row)) { + if ($row === null) { return Coordinate::stringFromColumnIndex($this->cachedHighestColumn); } @@ -1097,7 +1097,7 @@ class Worksheet implements IComparable */ public function getHighestRow($column = null) { - if ($column == null) { + if ($column === null) { return $this->cachedHighestRow; } @@ -1127,96 +1127,6 @@ class Worksheet implements IComparable return $this->cellCollection->getHighestRowAndColumn(); } - /** - * Validate a cell address. - * - * @param null|array|CellAddress|string $cellAddress Coordinate of the cell as a string, eg: 'C5'; - * or as an array of [$columnIndex, $row] (e.g. [3, 5]), or a CellAddress object. - */ - protected function validateCellAddress($cellAddress): string - { - if (is_string($cellAddress)) { - [$worksheet, $address] = self::extractSheetTitle($cellAddress, true); -// if (!empty($worksheet) && $worksheet !== $this->getTitle()) { -// throw new Exception('Reference is not for this worksheet'); -// } - - return empty($worksheet) ? strtoupper($address) : $worksheet . '!' . strtoupper($address); - } - - if (is_array($cellAddress)) { - $cellAddress = CellAddress::fromColumnRowArray($cellAddress); - } - - return (string) $cellAddress; - } - - private function tryDefinedName(string $coordinate): string - { - // Uppercase coordinate - $coordinate = strtoupper($coordinate); - // Eliminate leading equal sign - $coordinate = self::pregReplace('/^=/', '', $coordinate); - $defined = $this->parent->getDefinedName($coordinate, $this); - if ($defined !== null) { - if ($defined->getWorksheet() === $this && !$defined->isFormula()) { - $coordinate = self::pregReplace('/^=/', '', $defined->getValue()); - } - } - - return $coordinate; - } - - /** - * Validate a cell address or cell range. - * - * @param AddressRange|array|CellAddress|int|string $cellRange Coordinate of the cells as a string, eg: 'C5:F12'; - * or as an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 12]), - * or as a CellAddress or AddressRange object. - */ - protected function validateCellOrCellRange($cellRange): string - { - if (is_string($cellRange) || is_numeric($cellRange)) { - $cellRange = (string) $cellRange; - // Convert a single column reference like 'A' to 'A:A' - $cellRange = self::pregReplace('/^([A-Z]+)$/', '${1}:${1}', $cellRange); - // Convert a single row reference like '1' to '1:1' - $cellRange = self::pregReplace('/^(\d+)$/', '${1}:${1}', $cellRange); - } elseif (is_object($cellRange) && $cellRange instanceof CellAddress) { - $cellRange = new CellRange($cellRange, $cellRange); - } - - return $this->validateCellRange($cellRange); - } - - /** - * Validate a cell range. - * - * @param AddressRange|array|string $cellRange Coordinate of the cells as a string, eg: 'C5:F12'; - * or as an array of [$fromColumnIndex, $fromRow, $toColumnIndex, $toRow] (e.g. [3, 5, 6, 12]), - * or as an AddressRange object. - */ - protected function validateCellRange($cellRange): string - { - if (is_string($cellRange)) { - [$worksheet, $addressRange] = self::extractSheetTitle($cellRange, true); - - // Convert Column ranges like 'A:C' to 'A1:C1048576' - $addressRange = self::pregReplace('/^([A-Z]+):([A-Z]+)$/', '${1}1:${2}1048576', $addressRange); - // Convert Row ranges like '1:3' to 'A1:XFD3' - $addressRange = self::pregReplace('/^(\\d+):(\\d+)$/', 'A${1}:XFD${2}', $addressRange); - - return empty($worksheet) ? strtoupper($addressRange) : $worksheet . '!' . strtoupper($addressRange); - } - - if (is_array($cellRange)) { - [$from, $to] = array_chunk($cellRange, 2); - $cellRange = new CellRange(CellAddress::fromColumnRowArray($from), CellAddress::fromColumnRowArray($to)); - } - - return (string) $cellRange; - } - /** * Set a cell value. * @@ -1228,7 +1138,7 @@ class Worksheet implements IComparable */ public function setCellValue($coordinate, $value) { - $cellAddress = Functions::trimSheetFromCellReference($this->validateCellAddress($coordinate)); + $cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate)); $this->getCell($cellAddress)->setValue($value); return $this; @@ -1266,7 +1176,7 @@ class Worksheet implements IComparable */ public function setCellValueExplicit($coordinate, $value, $dataType) { - $cellAddress = Functions::trimSheetFromCellReference($this->validateCellAddress($coordinate)); + $cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate)); $this->getCell($cellAddress)->setValueExplicit($value, $dataType); return $this; @@ -1303,7 +1213,7 @@ class Worksheet implements IComparable */ public function getCell($coordinate): Cell { - $cellAddress = Functions::trimSheetFromCellReference($this->validateCellAddress($coordinate)); + $cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate)); // Shortcut for increased performance for the vast majority of simple cases if ($this->cellCollection->has($cellAddress)) { @@ -1454,7 +1364,7 @@ class Worksheet implements IComparable */ public function cellExists($coordinate): bool { - $cellAddress = $this->validateCellAddress($coordinate); + $cellAddress = Validations::validateCellAddress($coordinate); /** @var Worksheet $sheet */ [$sheet, $finalCoordinate] = $this->getWorksheetAndCoordinate($cellAddress); @@ -1546,7 +1456,7 @@ class Worksheet implements IComparable */ public function getStyle($cellCoordinate): Style { - $cellCoordinate = $this->validateCellOrCellRange($cellCoordinate); + $cellCoordinate = Validations::validateCellOrCellRange($cellCoordinate); // set this sheet as active $this->parent->setActiveSheetIndex($this->parent->getIndex($this)); @@ -1784,7 +1694,7 @@ class Worksheet implements IComparable */ public function setBreak($coordinate, $break) { - $cellAddress = Functions::trimSheetFromCellReference($this->validateCellAddress($coordinate)); + $cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate)); if ($break === self::BREAK_NONE) { if (isset($this->breaks[$cellAddress])) { @@ -1836,7 +1746,7 @@ class Worksheet implements IComparable */ public function mergeCells($range) { - $range = Functions::trimSheetFromCellReference($this->validateCellRange($range)); + $range = Functions::trimSheetFromCellReference(Validations::validateCellRange($range)); if (preg_match('/^([A-Z]+)(\\d+):([A-Z]+)(\\d+)$/', $range, $matches) === 1) { $this->mergeCells[$range] = $range; @@ -1945,7 +1855,7 @@ class Worksheet implements IComparable */ public function unmergeCells($range) { - $range = Functions::trimSheetFromCellReference($this->validateCellRange($range)); + $range = Functions::trimSheetFromCellReference(Validations::validateCellRange($range)); if (strpos($range, ':') !== false) { if (isset($this->mergeCells[$range])) { @@ -2023,7 +1933,7 @@ class Worksheet implements IComparable */ public function protectCells($range, $password, $alreadyHashed = false) { - $range = Functions::trimSheetFromCellReference($this->validateCellOrCellRange($range)); + $range = Functions::trimSheetFromCellReference(Validations::validateCellOrCellRange($range)); if (!$alreadyHashed) { $password = Shared\PasswordHasher::hashPassword($password); @@ -2071,7 +1981,7 @@ class Worksheet implements IComparable */ public function unprotectCells($range) { - $range = Functions::trimSheetFromCellReference($this->validateCellOrCellRange($range)); + $range = Functions::trimSheetFromCellReference(Validations::validateCellOrCellRange($range)); if (isset($this->protectedCells[$range])) { unset($this->protectedCells[$range]); @@ -2142,7 +2052,7 @@ class Worksheet implements IComparable if (is_object($autoFilterOrRange) && ($autoFilterOrRange instanceof AutoFilter)) { $this->autoFilter = $autoFilterOrRange; } else { - $cellRange = Functions::trimSheetFromCellReference($this->validateCellRange($autoFilterOrRange)); + $cellRange = Functions::trimSheetFromCellReference(Validations::validateCellRange($autoFilterOrRange)); $this->autoFilter->setRange($cellRange); } @@ -2216,13 +2126,13 @@ class Worksheet implements IComparable public function freezePane($coordinate, $topLeftCell = null) { $cellAddress = ($coordinate !== null) - ? Functions::trimSheetFromCellReference($this->validateCellAddress($coordinate)) + ? Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate)) : null; if ($cellAddress !== null && Coordinate::coordinateIsRange($cellAddress)) { throw new Exception('Freeze pane can not be set on a range of cells.'); } $topLeftCell = ($topLeftCell !== null) - ? Functions::trimSheetFromCellReference($this->validateCellAddress($topLeftCell)) + ? Functions::trimSheetFromCellReference(Validations::validateCellAddress($topLeftCell)) : null; if ($cellAddress !== null && $topLeftCell === null) { @@ -2626,7 +2536,7 @@ class Worksheet implements IComparable */ public function getComment($cellCoordinate) { - $cellAddress = Functions::trimSheetFromCellReference($this->validateCellAddress($cellCoordinate)); + $cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($cellCoordinate)); if (Coordinate::coordinateIsRange($cellAddress)) { throw new Exception('Cell coordinate string can not be a range of cells.'); @@ -2697,22 +2607,6 @@ class Worksheet implements IComparable return $this->setSelectedCells($coordinate); } - /** - * Sigh - Phpstan thinks, correctly, that preg_replace can return null. - * But Scrutinizer doesn't. Try to satisfy both. - * - * @param mixed $str - */ - private static function ensureString($str): string - { - return is_string($str) ? $str : ''; - } - - public static function pregReplace(string $pattern, string $replacement, string $subject): string - { - return self::ensureString(preg_replace($pattern, $replacement, $subject)); - } - /** * Select a range of cells. * @@ -2725,9 +2619,9 @@ class Worksheet implements IComparable public function setSelectedCells($coordinate) { if (is_string($coordinate)) { - $coordinate = $this->tryDefinedName($coordinate); + $coordinate = Validations::definedNameToCoordinate($coordinate, $this); } - $coordinate = $this->validateCellOrCellRange($coordinate); + $coordinate = Validations::validateCellOrCellRange($coordinate); if (Coordinate::coordinateIsRange($coordinate)) { [$first] = Coordinate::splitRange($coordinate);