diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b5a0398c..c22d5bca51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Xlsx loaded an extra empty comment for each real comment - [#375](https://github.com/PHPOffice/PhpSpreadsheet/issues/375) - Xlsx reader do not read rows and columns filtered out in readFilter at all - [#370](https://github.com/PHPOffice/PhpSpreadsheet/issues/370) - Make newer Excel versions properly recalculate formulas on document open - [#456](https://github.com/PHPOffice/PhpSpreadsheet/issues/456) +- `Coordinate::extractAllCellReferencesInRange()` throws an exception for an invalid range – [#519](https://github.com/PHPOffice/PhpSpreadsheet/issues/519) ## [1.2.1] - 2018-04-10 diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php index 1efcb627fc..077e8d0628 100644 --- a/src/PhpSpreadsheet/Cell/Coordinate.php +++ b/src/PhpSpreadsheet/Cell/Coordinate.php @@ -327,7 +327,7 @@ public static function stringFromColumnIndex($columnIndex) } /** - * Extract all cell references in range. + * Extract all cell references in range, which may be comprised of multiple cell ranges. * * @param string $pRange Range (e.g. A1 or A1:C10 or A1:E10 A20:E25) * @@ -335,49 +335,12 @@ public static function stringFromColumnIndex($columnIndex) */ public static function extractAllCellReferencesInRange($pRange) { - // Returnvalue $returnValue = []; // Explode spaces - $cellBlocks = explode(' ', str_replace('$', '', strtoupper($pRange))); + $cellBlocks = self::getCellBlocksFromRangeString($pRange); foreach ($cellBlocks as $cellBlock) { - // Single cell? - if (!self::coordinateIsRange($cellBlock)) { - $returnValue[] = $cellBlock; - - continue; - } - - // Range... - $ranges = self::splitRange($cellBlock); - foreach ($ranges as $range) { - // Single cell? - if (!isset($range[1])) { - $returnValue[] = $range[0]; - - continue; - } - - // Range... - list($rangeStart, $rangeEnd) = $range; - sscanf($rangeStart, '%[A-Z]%d', $startCol, $startRow); - sscanf($rangeEnd, '%[A-Z]%d', $endCol, $endRow); - ++$endCol; - - // Current data - $currentCol = $startCol; - $currentRow = $startRow; - - // Loop cells - while ($currentCol != $endCol) { - while ($currentRow <= $endRow) { - $returnValue[] = $currentCol . $currentRow; - ++$currentRow; - } - ++$currentCol; - $currentRow = $startRow; - } - } + $returnValue = array_merge($returnValue, static::getReferencesForCellBlock($cellBlock)); } // Sort the result by column and row @@ -392,6 +355,72 @@ public static function extractAllCellReferencesInRange($pRange) return array_values($sortKeys); } + /** + * Get all cell references for an individual cell block. + * + * @param string $cellBlock A cell range e.g. A4:B5 + * + * @throws Exception + * + * @return array All individual cells in that range + */ + private static function getReferencesForCellBlock($cellBlock) + { + $returnValue = []; + + // Single cell? + if (!self::coordinateIsRange($cellBlock)) { + return (array) $cellBlock; + } + + // Range... + $ranges = self::splitRange($cellBlock); + foreach ($ranges as $range) { + // Single cell? + if (!isset($range[1])) { + $returnValue[] = $range[0]; + + continue; + } + + // Range... + list($rangeStart, $rangeEnd) = $range; + list($startCol, $startRow) = static::extractColumnAndRow($rangeStart); + list($endCol, $endRow) = static::extractColumnAndRow($rangeEnd); + ++$endCol; + + // Current data + $currentCol = $startCol; + $currentRow = $startRow; + + static::validateRange($cellBlock, $startCol, $endCol, $currentRow, $endRow); + + // Loop cells + while ($currentCol < $endCol) { + while ($currentRow <= $endRow) { + $returnValue[] = $currentCol . $currentRow; + ++$currentRow; + } + ++$currentCol; + $currentRow = $startRow; + } + } + + return $returnValue; + } + + /** + * Extract the column and row from a cell reference in the format [$column, $row]. + * + * @param string $cell + * + * @return array + */ + private static function extractColumnAndRow($cell) + { + return sscanf($cell, '%[A-Z]%d'); + } + /** * Convert an associative array of single cell coordinates to values to an associative array * of cell ranges to values. Only adjacent cell coordinates with the same @@ -477,4 +506,35 @@ public static function mergeRangesInCollection(array $pCoordCollection) return $mergedCoordCollection; } + + /** + * Get the individual cell blocks from a range string, splitting by space and removing any $ characters. + * + * @param string $pRange + * + * @return string[] + */ + private static function getCellBlocksFromRangeString($pRange) + { + return explode(' ', str_replace('$', '', strtoupper($pRange))); + } + + /** + * Check that the given range is valid, i.e. that the start column and row are not greater than the end column and + * row. + * + * @param string $cellBlock The original range, for displaying a meaningful error message + * @param string $startCol + * @param string $endCol + * @param int $currentRow + * @param int $endRow + * + * @throws Exception + */ + private static function validateRange($cellBlock, $startCol, $endCol, $currentRow, $endRow) + { + if ($startCol >= $endCol || $currentRow > $endRow) { + throw new Exception('Invalid range: "' . $cellBlock . '"'); + } + } } diff --git a/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php b/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php index 0b4263bf21..5136226d58 100644 --- a/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php +++ b/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php @@ -315,6 +315,24 @@ public function providerExtractAllCellReferencesInRange() return require 'data/CellExtractAllCellReferencesInRange.php'; } + /** + * @dataProvider providerInvalidRange + * + * @param string $range + */ + public function testExtractAllCellReferencesInRangeInvalidRange($range) + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Invalid range: "' . $range . '"'); + + Coordinate::extractAllCellReferencesInRange($range); + } + + public function providerInvalidRange() + { + return [['Z1:A1'], ['A4:A1'], ['B1:A1'], ['AA1:Z1']]; + } + /** * @dataProvider providerMergeRangesInCollection *