From 16953e27d860b5b39f523e637c5858ca6d9b0d62 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Tue, 22 Feb 2022 15:55:59 +0100 Subject: [PATCH] Adjust Cell Reference regexp in Calculation Engine to handle worksheet names containing quotes (#2617) * Capture of worksheet name in Calculation Engine cell references modified to handle apostrophe and quote marks, and made non-greedy to avoid ensure that multiple quoted worksheet names in a formula aren't all captured in one go * Split some of the unit tests into separate test classes --- .../Calculation/Calculation.php | 13 +- .../CalclationFunctionListTest.php | 76 ++++++++++++ .../Calculation/CalculationSettingsTest.php | 76 ++++++++++++ .../Calculation/CalculationTest.php | 117 ++++++------------ 4 files changed, 195 insertions(+), 87 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/CalclationFunctionListTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/CalculationSettingsTest.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 2cf57915..2b82f65e 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -31,17 +31,17 @@ class Calculation // Function (allow for the old @ symbol that could be used to prefix a function, but we'll ignore it) const CALCULATION_REGEXP_FUNCTION = '@?(?:_xlfn\.)?([\p{L}][\p{L}\p{N}\.]*)[\s]*\('; // Cell reference (cell or range of cells, with or without a sheet reference) - const CALCULATION_REGEXP_CELLREF = '((([^\s,!&%^\/\*\+<>=-]*)|(\'[^\']*\')|(\"[^\"]*\"))!)?\$?\b([a-z]{1,3})\$?(\d{1,7})(?![\w.])'; + const CALCULATION_REGEXP_CELLREF = '((([^\s,!&%^\/\*\+<>=-]*)|(\'.*?\')|(\".*?\"))!)?\$?\b([a-z]{1,3})\$?(\d{1,7})(?![\w.])'; // Cell reference (with or without a sheet reference) ensuring absolute/relative - const CALCULATION_REGEXP_CELLREF_RELATIVE = '((([^\s\(,!&%^\/\*\+<>=-]*)|(\'[^\']*\')|(\"[^\"]*\"))!)?(\$?\b[a-z]{1,3})(\$?\d{1,7})(?![\w.])'; - const CALCULATION_REGEXP_COLUMN_RANGE = '(((([^\s\(,!&%^\/\*\+<>=-]*)|(\'[^\']*\')|(\"[^\"]*\"))!)?(\$?[a-z]{1,3})):(?![.*])'; - const CALCULATION_REGEXP_ROW_RANGE = '(((([^\s\(,!&%^\/\*\+<>=-]*)|(\'[^\']*\')|(\"[^\"]*\"))!)?(\$?[1-9][0-9]{0,6})):(?![.*])'; + const CALCULATION_REGEXP_CELLREF_RELATIVE = '((([^\s\(,!&%^\/\*\+<>=-]*)|(\'.*?\')|(\".*?\"))!)?(\$?\b[a-z]{1,3})(\$?\d{1,7})(?![\w.])'; + const CALCULATION_REGEXP_COLUMN_RANGE = '(((([^\s\(,!&%^\/\*\+<>=-]*)|(\'.*?\')|(\".*?\"))!)?(\$?[a-z]{1,3})):(?![.*])'; + const CALCULATION_REGEXP_ROW_RANGE = '(((([^\s\(,!&%^\/\*\+<>=-]*)|(\'.*?\')|(\".*?\"))!)?(\$?[1-9][0-9]{0,6})):(?![.*])'; // Cell reference (with or without a sheet reference) ensuring absolute/relative // Cell ranges ensuring absolute/relative const CALCULATION_REGEXP_COLUMNRANGE_RELATIVE = '(\$?[a-z]{1,3}):(\$?[a-z]{1,3})'; const CALCULATION_REGEXP_ROWRANGE_RELATIVE = '(\$?\d{1,7}):(\$?\d{1,7})'; // Defined Names: Named Range of cells, or Named Formulae - const CALCULATION_REGEXP_DEFINEDNAME = '((([^\s,!&%^\/\*\+<>=-]*)|(\'[^\']*\')|(\"[^\"]*\"))!)?([_\p{L}][_\p{L}\p{N}\.]*)'; + const CALCULATION_REGEXP_DEFINEDNAME = '((([^\s,!&%^\/\*\+<>=-]*)|(\'.*?\')|(\".*?\"))!)?([_\p{L}][_\p{L}\p{N}\.]*)'; // Error const CALCULATION_REGEXP_ERROR = '\#[A-Z][A-Z0_\/]*[!\?]?'; @@ -4199,7 +4199,8 @@ class Calculation $worksheet = $pCellParent->getTitle(); $val = "'{$worksheet}'!{$val}"; } - + // unescape any apostrophes or double quotes in worksheet name + $val = str_replace(["''", '""'], ["'", '"'], $val); $outputItem = $stack->getStackItem('Cell Reference', $val, $val, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); $output[] = $outputItem; diff --git a/tests/PhpSpreadsheetTests/Calculation/CalclationFunctionListTest.php b/tests/PhpSpreadsheetTests/Calculation/CalclationFunctionListTest.php new file mode 100644 index 00000000..8fde09ce --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/CalclationFunctionListTest.php @@ -0,0 +1,76 @@ +compatibilityMode = Functions::getCompatibilityMode(); + $calculation = Calculation::getInstance(); + $this->locale = $calculation->getLocale(); + Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); + } + + protected function tearDown(): void + { + Functions::setCompatibilityMode($this->compatibilityMode); + $calculation = Calculation::getInstance(); + $calculation->setLocale($this->locale); + } + + /** + * @dataProvider providerGetFunctions + * + * @param string $category + * @param array|string $functionCall + * @param string $argumentCount + */ + public function testGetFunctions($category, $functionCall, $argumentCount): void + { + self::assertIsCallable($functionCall); + } + + public function providerGetFunctions(): array + { + return Calculation::getInstance()->getFunctions(); + } + + public function testIsImplemented(): void + { + $calculation = Calculation::getInstance(); + self::assertFalse($calculation->isImplemented('non-existing-function')); + self::assertFalse($calculation->isImplemented('AREAS')); + self::assertTrue($calculation->isImplemented('coUNt')); + self::assertTrue($calculation->isImplemented('abs')); + } + + public function testUnknownFunction(): void + { + $workbook = new Spreadsheet(); + $sheet = $workbook->getActiveSheet(); + $sheet->setCellValue('A1', '=gzorg()'); + $sheet->setCellValue('A2', '=mode.gzorg(1)'); + $sheet->setCellValue('A3', '=gzorg(1,2)'); + $sheet->setCellValue('A4', '=3+IF(gzorg(),1,2)'); + self::assertEquals('#NAME?', $sheet->getCell('A1')->getCalculatedValue()); + self::assertEquals('#NAME?', $sheet->getCell('A2')->getCalculatedValue()); + self::assertEquals('#NAME?', $sheet->getCell('A3')->getCalculatedValue()); + self::assertEquals('#NAME?', $sheet->getCell('A4')->getCalculatedValue()); + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationSettingsTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationSettingsTest.php new file mode 100644 index 00000000..0652d315 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationSettingsTest.php @@ -0,0 +1,76 @@ +compatibilityMode = Functions::getCompatibilityMode(); + $calculation = Calculation::getInstance(); + $this->locale = $calculation->getLocale(); + Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); + } + + protected function tearDown(): void + { + Functions::setCompatibilityMode($this->compatibilityMode); + $calculation = Calculation::getInstance(); + $calculation->setLocale($this->locale); + } + + /** + * @dataProvider providerCanLoadAllSupportedLocales + * + * @param string $locale + */ + public function testCanLoadAllSupportedLocales($locale): void + { + $calculation = Calculation::getInstance(); + self::assertTrue($calculation->setLocale($locale)); + } + + public function testInvalidLocaleReturnsFalse(): void + { + $calculation = Calculation::getInstance(); + self::assertFalse($calculation->setLocale('xx')); + } + + public function providerCanLoadAllSupportedLocales(): array + { + return [ + ['bg'], + ['cs'], + ['da'], + ['de'], + ['en_us'], + ['es'], + ['fi'], + ['fr'], + ['hu'], + ['it'], + ['nl'], + ['nb'], + ['pl'], + ['pt'], + ['pt_br'], + ['ru'], + ['sv'], + ['tr'], + ]; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php index 296a3b0f..6a03fa1d 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php @@ -58,73 +58,6 @@ class CalculationTest extends TestCase return require 'tests/data/CalculationBinaryComparisonOperation.php'; } - /** - * @dataProvider providerGetFunctions - * - * @param string $category - * @param array|string $functionCall - * @param string $argumentCount - */ - public function testGetFunctions($category, $functionCall, $argumentCount): void - { - self::assertIsCallable($functionCall); - } - - public function providerGetFunctions(): array - { - return Calculation::getInstance()->getFunctions(); - } - - public function testIsImplemented(): void - { - $calculation = Calculation::getInstance(); - self::assertFalse($calculation->isImplemented('non-existing-function')); - self::assertFalse($calculation->isImplemented('AREAS')); - self::assertTrue($calculation->isImplemented('coUNt')); - self::assertTrue($calculation->isImplemented('abs')); - } - - /** - * @dataProvider providerCanLoadAllSupportedLocales - * - * @param string $locale - */ - public function testCanLoadAllSupportedLocales($locale): void - { - $calculation = Calculation::getInstance(); - self::assertTrue($calculation->setLocale($locale)); - } - - public function testInvalidLocaleReturnsFalse(): void - { - $calculation = Calculation::getInstance(); - self::assertFalse($calculation->setLocale('xx')); - } - - public function providerCanLoadAllSupportedLocales(): array - { - return [ - ['bg'], - ['cs'], - ['da'], - ['de'], - ['en_us'], - ['es'], - ['fi'], - ['fr'], - ['hu'], - ['it'], - ['nl'], - ['nb'], - ['pl'], - ['pt'], - ['pt_br'], - ['ru'], - ['sv'], - ['tr'], - ]; - } - public function testDoesHandleXlfnFunctions(): void { $calculation = Calculation::getInstance(); @@ -195,6 +128,42 @@ class CalculationTest extends TestCase self::assertEquals("=cmd|'/C calc'!A0", $cell->getCalculatedValue()); } + public function testFormulaReferencingWorksheetWithEscapedApostrophe(): void + { + $spreadsheet = new Spreadsheet(); + $workSheet = $spreadsheet->getActiveSheet(); + $workSheet->setTitle("Catégorie d'absence"); + + $workSheet->setCellValue('A1', 'HELLO'); + $workSheet->setCellValue('B1', ' '); + $workSheet->setCellValue('C1', 'WORLD'); + $workSheet->setCellValue( + 'A2', + "=CONCAT('Catégorie d''absence'!A1, 'Catégorie d''absence'!B1, 'Catégorie d''absence'!C1)" + ); + + $cellValue = $workSheet->getCell('A2')->getCalculatedValue(); + self::assertSame('HELLO WORLD', $cellValue); + } + + public function testFormulaReferencingWorksheetWithUnescapedApostrophe(): void + { + $spreadsheet = new Spreadsheet(); + $workSheet = $spreadsheet->getActiveSheet(); + $workSheet->setTitle("Catégorie d'absence"); + + $workSheet->setCellValue('A1', 'HELLO'); + $workSheet->setCellValue('B1', ' '); + $workSheet->setCellValue('C1', 'WORLD'); + $workSheet->setCellValue( + 'A2', + "=CONCAT('Catégorie d'absence'!A1, 'Catégorie d'absence'!B1, 'Catégorie d'absence'!C1)" + ); + + $cellValue = $workSheet->getCell('A2')->getCalculatedValue(); + self::assertSame('HELLO WORLD', $cellValue); + } + public function testCellWithFormulaTwoIndirect(): void { $spreadsheet = new Spreadsheet(); @@ -390,18 +359,4 @@ class CalculationTest extends TestCase { return require 'tests/data/Calculation/Calculation.php'; } - - public function testUnknownFunction(): void - { - $workbook = new Spreadsheet(); - $sheet = $workbook->getActiveSheet(); - $sheet->setCellValue('A1', '=gzorg()'); - $sheet->setCellValue('A2', '=mode.gzorg(1)'); - $sheet->setCellValue('A3', '=gzorg(1,2)'); - $sheet->setCellValue('A4', '=3+IF(gzorg(),1,2)'); - self::assertEquals('#NAME?', $sheet->getCell('A1')->getCalculatedValue()); - self::assertEquals('#NAME?', $sheet->getCell('A2')->getCalculatedValue()); - self::assertEquals('#NAME?', $sheet->getCell('A3')->getCalculatedValue()); - self::assertEquals('#NAME?', $sheet->getCell('A4')->getCalculatedValue()); - } }