Sheet title can contain exclamation mark (in formulas)

When extracting sheet title from string reference (like `"Work!sheet1!A1"`), PHP function `explode()` divide this string into three parts: `['Work', 'sheet1', 'A1']`. And then these wrong values are used in formulas, ranges, etc. This change fix that problem by using special function `Worksheet::extractSheetTitle()`. This function also has been changed to make sure that worksheet title can contain "!" character. So, that function search last position of "!" in reference string and divide it to 2 parts correctly: `['Work!sheet1', 'A1']`. Fixes PHPOffice#325 Fixes PHPOffice#662
Dfred · Oct 29, 2018 · a07be5f · a07be5f
1 parent a83b948
commit a07be5f
Show file tree

Hide file tree

Showing 16 changed files with 78 additions and 73 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+
+### Fixed
+
+- Sheet title can contain exclamation mark - [#325](https://github.com/PHPOffice/PhpSpreadsheet/issues/325)
+
 ## [1.4.1] - 2018-09-30
 
 ### Fixed

diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php
@@ -3488,19 +3488,15 @@ private function _parseFormula($formula, Cell $pCell = null)
                     $testPrevOp = $stack->last(1);
                     if ($testPrevOp['value'] == ':') {
                         $startRowColRef = $output[count($output) - 1]['value'];
-                        $rangeWS1 = '';
-                        if (strpos('!', $startRowColRef) !== false) {
-                            list($rangeWS1, $startRowColRef) = explode('!', $startRowColRef);
-                        }
+                        list($rangeWS1, $startRowColRef) = Worksheet::extractSheetTitle($startRowColRef, true);
                         if ($rangeWS1 != '') {
                             $rangeWS1 .= '!';
                         }
-                        $rangeWS2 = $rangeWS1;
-                        if (strpos('!', $val) !== false) {
-                            list($rangeWS2, $val) = explode('!', $val);
-                        }
+                        list($rangeWS2, $val) = Worksheet::extractSheetTitle($val, true);
                         if ($rangeWS2 != '') {
                             $rangeWS2 .= '!';
+                        } else {
+                            $rangeWS2 = $rangeWS1;
                         }
                         if ((is_int($startRowColRef)) && (ctype_digit($val)) &&
                             ($startRowColRef <= 1048576) && ($val <= 1048576)) {
@@ -3679,13 +3675,12 @@ private function processTokenStack($tokens, $cellID = null, Cell $pCell = null)
                     case ':':            //    Range
                         $sheet1 = $sheet2 = '';
                         if (strpos($operand1Data['reference'], '!') !== false) {
-                            list($sheet1, $operand1Data['reference']) = explode('!', $operand1Data['reference']);
+                            list($sheet1, $operand1Data['reference']) = Worksheet::extractSheetTitle($operand1Data['reference'], true);
                         } else {
                             $sheet1 = ($pCellParent !== null) ? $pCellWorksheet->getTitle() : '';
                         }
-                        if (strpos($operand2Data['reference'], '!') !== false) {
-                            list($sheet2, $operand2Data['reference']) = explode('!', $operand2Data['reference']);
-                        } else {
+                        list($sheet2, $operand2Data['reference']) = Worksheet::extractSheetTitle($operand2Data['reference'], true);
+                        if (empty($sheet2)) {
                             $sheet2 = $sheet1;
                         }
                         if ($sheet1 == $sheet2) {

diff --git a/src/PhpSpreadsheet/Calculation/LookupRef.php b/src/PhpSpreadsheet/Calculation/LookupRef.php
@@ -4,6 +4,7 @@
 
 use PhpOffice\PhpSpreadsheet\Cell\Cell;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 
 class LookupRef
 {
@@ -96,9 +97,7 @@ public static function COLUMN($cellAddress = null)
                 return (int) Coordinate::columnIndexFromString($columnKey);
             }
         } else {
-            if (strpos($cellAddress, '!') !== false) {
-                list($sheet, $cellAddress) = explode('!', $cellAddress);
-            }
+            list($sheet, $cellAddress) = Worksheet::extractSheetTitle($cellAddress, true);
             if (strpos($cellAddress, ':') !== false) {
                 list($startAddress, $endAddress) = explode(':', $cellAddress);
                 $startAddress = preg_replace('/[^a-z]/i', '', $startAddress);
@@ -175,9 +174,7 @@ public static function ROW($cellAddress = null)
                 }
             }
         } else {
-            if (strpos($cellAddress, '!') !== false) {
-                list($sheet, $cellAddress) = explode('!', $cellAddress);
-            }
+            list($sheet, $cellAddress) = Worksheet::extractSheetTitle($cellAddress, true);
             if (strpos($cellAddress, ':') !== false) {
                 list($startAddress, $endAddress) = explode(':', $cellAddress);
                 $startAddress = preg_replace('/\D/', '', $startAddress);
@@ -297,7 +294,7 @@ public static function INDIRECT($cellAddress = null, Cell $pCell = null)
             }
 
             if (strpos($cellAddress, '!') !== false) {
-                list($sheetName, $cellAddress) = explode('!', $cellAddress);
+                list($sheetName, $cellAddress) = Worksheet::extractSheetTitle($cellAddress, true);
                 $sheetName = trim($sheetName, "'");
                 $pSheet = $pCell->getWorksheet()->getParent()->getSheetByName($sheetName);
             } else {
@@ -308,7 +305,7 @@ public static function INDIRECT($cellAddress = null, Cell $pCell = null)
         }
 
         if (strpos($cellAddress, '!') !== false) {
-            list($sheetName, $cellAddress) = explode('!', $cellAddress);
+            list($sheetName, $cellAddress) = Worksheet::extractSheetTitle($cellAddress, true);
             $sheetName = trim($sheetName, "'");
             $pSheet = $pCell->getWorksheet()->getParent()->getSheetByName($sheetName);
         } else {
@@ -361,7 +358,7 @@ public static function OFFSET($cellAddress = null, $rows = 0, $columns = 0, $hei
 
         $sheetName = null;
         if (strpos($cellAddress, '!')) {
-            list($sheetName, $cellAddress) = explode('!', $cellAddress);
+            list($sheetName, $cellAddress) = Worksheet::extractSheetTitle($cellAddress, true);
             $sheetName = trim($sheetName, "'");
         }
         if (strpos($cellAddress, ':')) {

diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php
@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheet\Cell;
 
 use PhpOffice\PhpSpreadsheet\Exception;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 
 /**
  * Helper class to manipulate cell coordinates.
@@ -70,11 +71,7 @@ public static function absoluteReference($pCoordinateString)
         }
 
         // Split out any worksheet name from the reference
-        $worksheet = '';
-        $cellAddress = explode('!', $pCoordinateString);
-        if (count($cellAddress) > 1) {
-            list($worksheet, $pCoordinateString) = $cellAddress;
-        }
+        list($worksheet, $pCoordinateString) = Worksheet::extractSheetTitle($pCoordinateString, true);
         if ($worksheet > '') {
             $worksheet .= '!';
         }
@@ -105,11 +102,7 @@ public static function absoluteCoordinate($pCoordinateString)
         }
 
         // Split out any worksheet name from the coordinate
-        $worksheet = '';
-        $cellAddress = explode('!', $pCoordinateString);
-        if (count($cellAddress) > 1) {
-            list($worksheet, $pCoordinateString) = $cellAddress;
-        }
+        list($worksheet, $pCoordinateString) = Worksheet::extractSheetTitle($pCoordinateString, true);
         if ($worksheet > '') {
             $worksheet .= '!';
         }

diff --git a/src/PhpSpreadsheet/Chart/DataSeriesValues.php b/src/PhpSpreadsheet/Chart/DataSeriesValues.php
@@ -354,11 +354,7 @@ public function refresh(Worksheet $worksheet, $flatten = true)
                 }
                 unset($dataValue);
             } else {
-                $cellRange = explode('!', $this->dataSource);
-                if (count($cellRange) > 1) {
-                    list(, $cellRange) = $cellRange;
-                }
-
+                list($worksheet, $cellRange) = Worksheet::extractSheetTitle($this->dataSource, true);
                 $dimensions = Coordinate::rangeDimension(str_replace('$', '', $cellRange));
                 if (($dimensions[0] == 1) || ($dimensions[1] == 1)) {
                     $this->dataValues = Functions::flattenArray($newDataValues);

diff --git a/src/PhpSpreadsheet/Reader/Gnumeric.php b/src/PhpSpreadsheet/Reader/Gnumeric.php
@@ -16,6 +16,7 @@
 use PhpOffice\PhpSpreadsheet\Style\Borders;
 use PhpOffice\PhpSpreadsheet\Style\Fill;
 use PhpOffice\PhpSpreadsheet\Style\Font;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 use XMLReader;
 
 class Gnumeric extends BaseReader
@@ -784,7 +785,7 @@ public function loadIntoExisting($pFilename, Spreadsheet $spreadsheet)
                     continue;
                 }
 
-                $range = explode('!', $range);
+                $range = Worksheet::extractSheetTitle($range, true);
                 $range[0] = trim($range[0], "'");
                 if ($worksheet = $spreadsheet->getSheetByName($range[0])) {
                     $extractedRange = str_replace('$', '', $range[1]);

diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php
@@ -1213,7 +1213,7 @@ public function load($pFilename)
                             // $range should look like one of these
                             //        Foo!$C$7:$J$66
                             //        Bar!$A$1:$IV$2
-                            $explodes = explode('!', $range); // FIXME: what if sheetname contains exclamation mark?
+                            $explodes = Worksheet::extractSheetTitle($range, true);
                             $sheetName = trim($explodes[0], "'");
                             if (count($explodes) == 2) {
                                 if (strpos($explodes[1], ':') === false) {
@@ -1243,8 +1243,8 @@ public function load($pFilename)
                             // $range should look like this one of these
                             //        Sheet!$A$1:$B$65536
                             //        Sheet!$A$1:$IV$2
-                            $explodes = explode('!', $range);
-                            if (count($explodes) == 2) {
+                            if (strpos($range, '!') !== false) {
+                                $explodes = Worksheet::extractSheetTitle($range, true);
                                 if ($docSheet = $this->spreadsheet->getSheetByName($explodes[0])) {
                                     $extractedRange = $explodes[1];
                                     $extractedRange = str_replace('$', '', $extractedRange);
@@ -1270,9 +1270,8 @@ public function load($pFilename)
                 }
             } else {
                 // Extract range
-                $explodes = explode('!', $definedName['formula']);
-
-                if (count($explodes) == 2) {
+                if (strpos($definedName['formula'], '!') !== false) {
+                    $explodes = Worksheet::extractSheetTitle($definedName['formula'], true);
                     if (($docSheet = $this->spreadsheet->getSheetByName($explodes[0])) ||
                         ($docSheet = $this->spreadsheet->getSheetByName(trim($explodes[0], "'")))) {
                         $extractedRange = $explodes[1];

diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -1848,8 +1848,7 @@ public function load($pFilename)
                                                 $rangeSets = preg_split("/'(.*?)'(?:![A-Z0-9]+:[A-Z0-9]+,?)/", $extractedRange, PREG_SPLIT_NO_EMPTY);
                                                 $newRangeSets = [];
                                                 foreach ($rangeSets as $rangeSet) {
-                                                    $range = explode('!', $rangeSet); // FIXME: what if sheetname contains exclamation mark?
-                                                    $rangeSet = isset($range[1]) ? $range[1] : $range[0];
+                                                    list($sheetName, $rangeSet) = Worksheet::extractSheetTitle($rangeSet, true);
                                                     if (strpos($rangeSet, ':') === false) {
                                                         $rangeSet = $rangeSet . ':' . $rangeSet;
                                                     }
@@ -1896,8 +1895,8 @@ public function load($pFilename)
                                             break;
                                         default:
                                             if ($mapSheetId[(int) $definedName['localSheetId']] !== null) {
-                                                $range = explode('!', (string) $definedName);
-                                                if (count($range) == 2) {
+                                                if (strpos((string) $definedName, '!') !== false) {
+                                                    $range = Worksheet::extractSheetTitle((string) $definedName, true);
                                                     $range[0] = str_replace("''", "'", $range[0]);
                                                     $range[0] = str_replace("'", '', $range[0]);
                                                     if ($worksheet = $docSheet->getParent()->getSheetByName($range[0])) {
@@ -1923,8 +1922,7 @@ public function load($pFilename)
                                         $locatedSheet = $excel->getSheetByName($extractedSheetName);
 
                                         // Modify range
-                                        $range = explode('!', $extractedRange);
-                                        $extractedRange = isset($range[1]) ? $range[1] : $range[0];
+                                        list($worksheetName, $extractedRange) = Worksheet::extractSheetTitle($extractedRange, true);
                                     }
 
                                     if ($locatedSheet !== null) {

diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter.php b/src/PhpSpreadsheet/Worksheet/AutoFilter.php
@@ -89,11 +89,8 @@ public function getRange()
      */
     public function setRange($pRange)
     {
-        // Uppercase coordinate
-        $cellAddress = explode('!', strtoupper($pRange));
-        if (count($cellAddress) > 1) {
-            list($worksheet, $pRange) = $cellAddress;
-        }
+        // extract coordinate
+        list($worksheet, $pRange) = Worksheet::extractSheetTitle($pRange, true);
 
         if (strpos($pRange, ':') !== false) {
             $this->range = $pRange;

diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -2751,12 +2751,12 @@ public function getHashCode()
     public static function extractSheetTitle($pRange, $returnRange = false)
     {
         // Sheet title included?
-        if (($sep = strpos($pRange, '!')) === false) {
-            return '';
+        if (($sep = strrpos($pRange, '!')) === false) {
+            return $returnRange ? ['', $pRange] : '';
         }
 
         if ($returnRange) {
-            return [trim(substr($pRange, 0, $sep), "'"), substr($pRange, $sep + 1)];
+            return [substr($pRange, 0, $sep), substr($pRange, $sep + 1)];
         }
 
         return substr($pRange, $sep + 1);

diff --git a/src/PhpSpreadsheet/Writer/Xls/Parser.php b/src/PhpSpreadsheet/Writer/Xls/Parser.php
@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheet\Writer\Xls;
 
 use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet as PhpspreadsheetWorksheet;
 use PhpOffice\PhpSpreadsheet\Writer\Exception as WriterException;
 
 // Original file header of PEAR::Spreadsheet_Excel_Writer_Parser (used as the base for this class):
@@ -643,7 +644,7 @@ private function convertRange2d($range, $class = 0)
     private function convertRange3d($token)
     {
         // Split the ref at the ! symbol
-        list($ext_ref, $range) = explode('!', $token);
+        list($ext_ref, $range) = PhpspreadsheetWorksheet::extractSheetTitle($token, true);
 
         // Convert the external reference part (different for BIFF8)
         $ext_ref = $this->getRefIndex($ext_ref);
@@ -695,7 +696,7 @@ private function convertRef2d($cell)
     private function convertRef3d($cell)
     {
         // Split the ref at the ! symbol
-        list($ext_ref, $cell) = explode('!', $cell);
+        list($ext_ref, $cell) = PhpspreadsheetWorksheet::extractSheetTitle($cell, true);
 
         // Convert the external reference part (different for BIFF8)
         $ext_ref = $this->getRefIndex($ext_ref);

diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php
@@ -339,9 +339,7 @@ private function writeDefinedNameForAutofilter(XMLWriter $objWriter, Worksheet $
             $range = Coordinate::splitRange($autoFilterRange);
             $range = $range[0];
             //    Strip any worksheet ref so we can make the cell ref absolute
-            if (strpos($range[0], '!') !== false) {
-                list($ws, $range[0]) = explode('!', $range[0]);
-            }
+            list($ws, $range[0]) = Worksheet::extractSheetTitle($range[0], true);
 
             $range[0] = Coordinate::absoluteCoordinate($range[0]);
             $range[1] = Coordinate::absoluteCoordinate($range[1]);

diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
@@ -748,9 +748,7 @@ private function writeAutoFilter(XMLWriter $objWriter, PhpspreadsheetWorksheet $
             $range = Coordinate::splitRange($autoFilterRange);
             $range = $range[0];
             //    Strip any worksheet ref
-            if (strpos($range[0], '!') !== false) {
-                list($ws, $range[0]) = explode('!', $range[0]);
-            }
+            list($ws, $range[0]) = PhpspreadsheetWorksheet::extractSheetTitle($range[0], true);
             $range = implode(':', $range);
 
             $objWriter->writeAttribute('ref', str_replace('$', '', $range));

diff --git a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php
@@ -130,4 +130,30 @@ public function testFreezePaneSelectedCell()
         $worksheet->freezePane('B2');
         self::assertSame('B2', $worksheet->getTopLeftCell());
     }
+
+    public function extractSheetTitleProvider()
+    {
+        return [
+            ['B2', '', '', 'B2'],
+            ['testTitle!B2', 'testTitle', 'B2', 'B2'],
+            ['test!Title!B2', 'test!Title', 'B2', 'B2'],
+        ];
+    }
+
+    /**
+     * @param string $range
+     * @param string $expectTitle
+     * @param string $expectCell
+     * @param string $expectCell2
+     * @dataProvider extractSheetTitleProvider
+     */
+    public function testExtractSheetTitle($range, $expectTitle, $expectCell, $expectCell2)
+    {
+        // only cell reference
+        self::assertSame($expectCell, Worksheet::extractSheetTitle($range));
+        // with title in array
+        $arRange = Worksheet::extractSheetTitle($range, true);
+        self::assertSame($expectTitle, $arRange[0]);
+        self::assertSame($expectCell2, $arRange[1]);
+    }
 }
diff --git a/tests/data/CellAbsoluteCoordinate.php b/tests/data/CellAbsoluteCoordinate.php
@@ -42,11 +42,11 @@
         '\'Worksheet1\'!$AI256',
     ],
     [
-        '\'Worksheet1\'!$AI$256',
-        '\'Worksheet1\'!AI$256',
+        '\'Work!sheet1!\'!$AI$256',
+        '\'Work!sheet1!\'!AI$256',
     ],
     [
-        '\'Worksheet1\'!$AI$256',
-        '\'Worksheet1\'!$AI$256',
+        '\'Work!sheet1!\'!$AI$256',
+        '\'Work!sheet1!\'!$AI$256',
     ],
 ];
diff --git a/tests/data/CellAbsoluteReference.php b/tests/data/CellAbsoluteReference.php
@@ -26,12 +26,12 @@
         'AI2012',
     ],
     [
-        '\'Worksheet1\'!$AI$256',
-        '\'Worksheet1\'!AI256',
+        '\'Work!sheet1\'!$AI$256',
+        '\'Work!sheet1\'!AI256',
     ],
     [
-        'Worksheet1!$AI$256',
-        'Worksheet1!AI256',
+        'Work!sheet1!$AI$256',
+        'Work!sheet1!AI256',
     ],
     [
         '\'Data Worksheet\'!$AI$256',