From 50a9bc83abe140fc23d77e48e5819c7835b90dc9 Mon Sep 17 00:00:00 2001 From: Timur Date: Wed, 3 Oct 2018 06:52:51 +0300 Subject: [PATCH] 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 #325 Fixes #662 --- CHANGELOG.md | 6 +++++ .../Calculation/Calculation.php | 19 +++++--------- src/PhpSpreadsheet/Calculation/LookupRef.php | 15 +++++------ src/PhpSpreadsheet/Cell/Coordinate.php | 13 +++------- src/PhpSpreadsheet/Chart/DataSeriesValues.php | 6 +---- src/PhpSpreadsheet/Reader/Gnumeric.php | 3 ++- src/PhpSpreadsheet/Reader/Xls.php | 11 ++++---- src/PhpSpreadsheet/Reader/Xlsx.php | 10 +++---- src/PhpSpreadsheet/Worksheet/AutoFilter.php | 7 ++--- src/PhpSpreadsheet/Worksheet/Worksheet.php | 6 ++--- src/PhpSpreadsheet/Writer/Xls/Parser.php | 5 ++-- src/PhpSpreadsheet/Writer/Xlsx/Workbook.php | 4 +-- src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 4 +-- .../Worksheet/WorksheetTest.php | 26 +++++++++++++++++++ tests/data/CellAbsoluteCoordinate.php | 8 +++--- tests/data/CellAbsoluteReference.php | 8 +++--- 16 files changed, 78 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd3e9a1..f37d67bc 100644 --- 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 index 79b98b95..5dc74ef0 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -3478,19 +3478,15 @@ class Calculation $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)) { @@ -3665,13 +3661,12 @@ class Calculation 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 index 71550a7b..8bd778d9 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation; use PhpOffice\PhpSpreadsheet\Cell\Cell; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class LookupRef { @@ -96,9 +97,7 @@ class LookupRef 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 @@ class LookupRef } } } 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 @@ class LookupRef } 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 @@ class LookupRef } 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 @@ class LookupRef $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 index 366ec492..1bca6f44 100644 --- 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 @@ abstract class Coordinate } // 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 @@ abstract class Coordinate } // 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 index a726219c..fda6ccdf 100644 --- a/src/PhpSpreadsheet/Chart/DataSeriesValues.php +++ b/src/PhpSpreadsheet/Chart/DataSeriesValues.php @@ -354,11 +354,7 @@ class DataSeriesValues } 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 index af2ed8e4..dd92f153 100644 --- a/src/PhpSpreadsheet/Reader/Gnumeric.php +++ b/src/PhpSpreadsheet/Reader/Gnumeric.php @@ -16,6 +16,7 @@ use PhpOffice\PhpSpreadsheet\Style\Border; 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 @@ class Gnumeric extends BaseReader 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 index e6803c78..d08d85e8 100644 --- a/src/PhpSpreadsheet/Reader/Xls.php +++ b/src/PhpSpreadsheet/Reader/Xls.php @@ -1213,7 +1213,7 @@ class Xls extends BaseReader // $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 @@ class Xls extends BaseReader // $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 @@ class Xls extends BaseReader } } 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 index a7709852..dfb4b825 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1833,8 +1833,7 @@ class Xlsx extends BaseReader $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; } @@ -1881,8 +1880,8 @@ class Xlsx extends BaseReader 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])) { @@ -1908,8 +1907,7 @@ class Xlsx extends BaseReader $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 index b92c986e..333aa1b8 100644 --- a/src/PhpSpreadsheet/Worksheet/AutoFilter.php +++ b/src/PhpSpreadsheet/Worksheet/AutoFilter.php @@ -89,11 +89,8 @@ class AutoFilter */ 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 index 42dcd69b..f4ee4b60 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -2729,12 +2729,12 @@ class Worksheet implements IComparable 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 index 00ead3c2..e87d09a2 100644 --- 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 @@ class Parser 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 @@ class Parser 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 index 43a1916f..e3ddb03c 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php @@ -339,9 +339,7 @@ class Workbook extends WriterPart $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 index cb46a121..78a62e9e 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php @@ -748,9 +748,7 @@ class Worksheet extends WriterPart $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 index 5dfa8259..46df9405 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php @@ -130,4 +130,30 @@ class WorksheetTest extends TestCase $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 index d5eefca1..86487585 100644 --- a/tests/data/CellAbsoluteCoordinate.php +++ b/tests/data/CellAbsoluteCoordinate.php @@ -42,11 +42,11 @@ return [ '\'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 index ccf4f33f..71dac545 100644 --- a/tests/data/CellAbsoluteReference.php +++ b/tests/data/CellAbsoluteReference.php @@ -26,12 +26,12 @@ return [ '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',