Throw exception for invalid range to prevent infinite loop

Fixes PHPOffice#519 Closes PHPOffice#521
billblume · Jun 13, 2018 · 19c4a0e · 19c4a0e
1 parent ebf26f1
commit 19c4a0e
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,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
@@ -340,7 +340,7 @@ public static function extractAllCellReferencesInRange($pRange)
         // Explode spaces
         $cellBlocks = self::getCellBlocksFromRangeString($pRange);
         foreach ($cellBlocks as $cellBlock) {
-            $returnValue = array_merge($returnValue, self::getReferencesForCellBlock($cellBlock));
+            $returnValue = array_merge($returnValue, static::getReferencesForCellBlock($cellBlock));
         }
 
         //    Sort the result by column and row
@@ -360,6 +360,8 @@ public static function extractAllCellReferencesInRange($pRange)
      *
      * @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)
@@ -383,32 +385,42 @@ private static function getReferencesForCellBlock($cellBlock)
 
             // Range...
             list($rangeStart, $rangeEnd) = $range;
-            list($startColumn, $startRow) = self::coordinateFromString($rangeStart);
-            list($endColumn, $endRow) = self::coordinateFromString($rangeEnd);
-            $startColumnIndex = self::columnIndexFromString($startColumn);
-            $endColumnIndex = self::columnIndexFromString($endColumn);
-            ++$endColumnIndex;
+            list($startCol, $startRow) = static::extractColumnAndRow($rangeStart);
+            list($endCol, $endRow) = static::extractColumnAndRow($rangeEnd);
+            ++$endCol;
 
             // Current data
-            $currentColumnIndex = $startColumnIndex;
+            $currentCol = $startCol;
             $currentRow = $startRow;
 
-            self::validateRange($cellBlock, $startColumnIndex, $endColumnIndex, $currentRow, $endRow);
+            static::validateRange($cellBlock, $startCol, $endCol, $currentRow, $endRow);
 
             // Loop cells
-            while ($currentColumnIndex < $endColumnIndex) {
+            while ($currentCol < $endCol) {
                 while ($currentRow <= $endRow) {
-                    $returnValue[] = self::stringFromColumnIndex($currentColumnIndex) . $currentRow;
+                    $returnValue[] = $currentCol . $currentRow;
                     ++$currentRow;
                 }
-                ++$currentColumnIndex;
+                ++$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
@@ -512,14 +524,16 @@ private static function getCellBlocksFromRangeString($pRange)
      * row.
      *
      * @param string $cellBlock The original range, for displaying a meaningful error message
-     * @param int $startColumnIndex
-     * @param int $endColumnIndex
+     * @param string $startCol
+     * @param string $endCol
      * @param int $currentRow
      * @param int $endRow
+     *
+     * @throws Exception
      */
-    private static function validateRange($cellBlock, $startColumnIndex, $endColumnIndex, $currentRow, $endRow)
+    private static function validateRange($cellBlock, $startCol, $endCol, $currentRow, $endRow)
     {
-        if ($startColumnIndex >= $endColumnIndex || $currentRow > $endRow) {
+        if ($startCol >= $endCol || $currentRow > $endRow) {
             throw new Exception('Invalid range: "' . $cellBlock . '"');
         }
     }