From 0a98d923bb35067c275e647deebbbed1e1223f9a Mon Sep 17 00:00:00 2001 From: Helmut Hummel <typo3@helhum.io> Date: Tue, 27 Jul 2021 12:35:01 +0200 Subject: [PATCH] [BUGFIX] Allow slashes as TS keys and escape dots for generated TS With the introduction of site settings being exposed to TypoScript with https://review.typo3.org/64128 it has become important to allow more characters as TypoScript keys. Allowing a slash in addition should cover cases where paths are exposed as keys. Additionally the above change reveals, that the ArrayUtility::flatten method does not properly handle array keys that has dots in between, as those must be escaped to produce a correct result. This change introduces a new flatten method after several failed attempts to guess what fixes might not be breaking. The new method is used in places where no TypoScript is used, as in those the weird edge case behaviour of the flatten method isn't expected anyway. The new method is marked internal until it is decided how to proceed with the flatten method. Resolves: #94646 Releases: master, 10.4 Change-Id: I4ebad8a0beece975702d8601a343aa1fdaaa285c Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/70115 Tested-by: core-ci <typo3@b13.com> Tested-by: Benni Mack <benni@typo3.org> Tested-by: Oliver Bartsch <bo@cedev.de> Tested-by: Helmut Hummel <typo3@helhum.io> Reviewed-by: Benni Mack <benni@typo3.org> Reviewed-by: Oliver Bartsch <bo@cedev.de> Reviewed-by: Helmut Hummel <typo3@helhum.io> --- .../Parser/PageTsConfigParser.php | 2 +- .../TypoScript/Parser/TypoScriptParser.php | 4 +- .../Classes/TypoScript/TemplateService.php | 2 +- .../core/Classes/Utility/ArrayUtility.php | 29 +++ .../Parser/TypoScriptParserTest.php | 8 +- .../Tests/Unit/Utility/ArrayUtilityTest.php | 176 ++++++++++++++++++ .../Be/Menus/ActionMenuItemViewHelper.php | 4 +- .../ArrayProcessing/ArrayProcessor.php | 2 +- 8 files changed, 219 insertions(+), 8 deletions(-) diff --git a/typo3/sysext/core/Classes/Configuration/Parser/PageTsConfigParser.php b/typo3/sysext/core/Classes/Configuration/Parser/PageTsConfigParser.php index a57d6022c5d4..871903bfc616 100644 --- a/typo3/sysext/core/Classes/Configuration/Parser/PageTsConfigParser.php +++ b/typo3/sysext/core/Classes/Configuration/Parser/PageTsConfigParser.php @@ -65,7 +65,7 @@ class PageTsConfigParser if ($site) { $siteSettings = $site->getConfiguration()['settings'] ?? []; if (!empty($siteSettings)) { - $siteSettings = ArrayUtility::flatten($siteSettings); + $siteSettings = ArrayUtility::flattenPlain($siteSettings); } if (!empty($siteSettings)) { // Recursive substitution of site settings (up to 10 nested levels) diff --git a/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php b/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php index b39f7be87997..3ffeb0bba596 100644 --- a/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php +++ b/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php @@ -292,8 +292,8 @@ class TypoScriptParser $objStrName = substr($line, 0, $varL); if ($objStrName !== '') { $r = []; - if (preg_match('/[^[:alnum:]_\\\\\\.:-]/i', $objStrName, $r)) { - $this->error('Line ' . ($this->lineNumberOffset + $this->rawP - 1) . ': Object Name String, "' . htmlspecialchars($objStrName) . '" contains invalid character "' . $r[0] . '". Must be alphanumeric or one of: "_:-\\."'); + if (preg_match('/[^[:alnum:]\\/_\\\\\\.:-]/i', $objStrName, $r)) { + $this->error('Line ' . ($this->lineNumberOffset + $this->rawP - 1) . ': Object Name String, "' . htmlspecialchars($objStrName) . '" contains invalid character "' . $r[0] . '". Must be alphanumeric or one of: "_:-/\\."'); } else { $line = ltrim(substr($line, $varL)); if ($line === '') { diff --git a/typo3/sysext/core/Classes/TypoScript/TemplateService.php b/typo3/sysext/core/Classes/TypoScript/TemplateService.php index 47d120ed65ed..e8a268014ad5 100644 --- a/typo3/sysext/core/Classes/TypoScript/TemplateService.php +++ b/typo3/sysext/core/Classes/TypoScript/TemplateService.php @@ -1200,7 +1200,7 @@ class TemplateService if ($site instanceof Site) { $siteSettings = $site->getConfiguration()['settings'] ?? []; if (!empty($siteSettings)) { - $siteSettings = ArrayUtility::flatten($siteSettings); + $siteSettings = ArrayUtility::flattenPlain($siteSettings); foreach ($siteSettings as $k => $v) { $siteConstants .= $k . ' = ' . $v . LF; } diff --git a/typo3/sysext/core/Classes/Utility/ArrayUtility.php b/typo3/sysext/core/Classes/Utility/ArrayUtility.php index b5d87f8110de..746c2b93fc8a 100644 --- a/typo3/sysext/core/Classes/Utility/ArrayUtility.php +++ b/typo3/sysext/core/Classes/Utility/ArrayUtility.php @@ -449,6 +449,8 @@ class ArrayUtility /** * Converts a multidimensional array to a flat representation. + * @todo: The current implementation isn't a generic array flatten method, but tailored for TypoScript flattening + * @todo: It should be deprecated and removed and the required specialities should be put under the domain of TypoScript parsing * * See unit tests for more details * @@ -502,6 +504,33 @@ class ArrayUtility return $flatArray; } + /** + * Just like flatten, but not tailored for TypoScript but for plain simple arrays + * It is internal for now, as it needs to be decided how to deprecate/ rename flatten + * + * @param array $array + * @return array + * @internal + */ + public static function flattenPlain(array $array): array + { + $flattenRecursive = static function (array $array, string $prefix = '') use (&$flattenRecursive) { + $flatArray = []; + foreach ($array as $key => $value) { + $key = addcslashes((string)$key, '.'); + if (!is_array($value)) { + $flatArray[] = [$prefix . $key => $value]; + } else { + $flatArray[] = $flattenRecursive($value, $prefix . $key . '.'); + } + } + + return array_merge(...$flatArray); + }; + + return $flattenRecursive($array); + } + /** * Determine the intersections between two arrays, recursively comparing keys * A complete sub array of $source will be preserved, if the key exists in $mask. diff --git a/typo3/sysext/core/Tests/Unit/TypoScript/Parser/TypoScriptParserTest.php b/typo3/sysext/core/Tests/Unit/TypoScript/Parser/TypoScriptParserTest.php index 03b4b159c581..cf6e24dc2401 100644 --- a/typo3/sysext/core/Tests/Unit/TypoScript/Parser/TypoScriptParserTest.php +++ b/typo3/sysext/core/Tests/Unit/TypoScript/Parser/TypoScriptParserTest.php @@ -385,7 +385,7 @@ class TypoScriptParserTest extends UnitTestCase $typoScript = '$.10 = invalid'; $this->typoScriptParser->parse($typoScript); - $expected = 'Line 0: Object Name String, "$.10" contains invalid character "$". Must be alphanumeric or one of: "_:-\."'; + $expected = 'Line 0: Object Name String, "$.10" contains invalid character "$". Must be alphanumeric or one of: "_:-/\."'; self::assertEquals($expected, $this->typoScriptParser->errors[0][0]); } @@ -724,6 +724,12 @@ test.TYPO3Forever.TypoScript = 1 'key' => 'value', ], ], + 'simple assignment with slash in key' => [ + 'lib/key = value', + [ + 'lib/key' => 'value', + ], + ], 'simple assignment with escaped dot at the beginning' => [ '\\.key = value', [ diff --git a/typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php b/typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php index 079a97f5d5ec..a977c56239f5 100644 --- a/typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php +++ b/typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php @@ -1366,6 +1366,182 @@ class ArrayUtilityTest extends UnitTestCase self::assertEquals($expected, ArrayUtility::flatten($array)); } + /////////////////////// + // Tests concerning flattenPlain + /////////////////////// + + /** + * @return array + */ + public function flattenPlainCalculatesExpectedResultDataProvider(): array + { + return [ + 'plain array' => [ + [ + 'first' => 1, + 'second' => 2, + ], + [ + 'first' => 1, + 'second' => 2, + ], + ], + 'plain array with trailing dots' => [ + [ + 'first.' => 1, + 'second.' => 2, + ], + [ + 'first\.' => 1, + 'second\.' => 2, + ], + ], + 'nested array of 2 levels' => [ + [ + 'first' => [ + 'firstSub' => 1, + ], + 'second' => [ + 'secondSub' => 2, + ], + ], + [ + 'first.firstSub' => 1, + 'second.secondSub' => 2, + ], + ], + 'nested array of 2 levels with dots in keys' => [ + [ + 'first.el' => [ + 'firstSub.' => 1, + ], + 'second.el' => [ + 'secondSub.' => 2, + ], + ], + [ + 'first\.el.firstSub\.' => 1, + 'second\.el.secondSub\.' => 2, + ], + ], + 'nested array of 2 levels with dots inside keys' => [ + [ + 'first' => [ + 'first.sub' => 1, + ], + 'second' => [ + 'second.sub' => 2, + ], + ], + [ + 'first.first\.sub' => 1, + 'second.second\.sub' => 2, + ], + ], + 'nested array of 3 levels' => [ + [ + 'first' => [ + 'firstSub' => [ + 'firstSubSub' => 1, + ], + ], + 'second' => [ + 'secondSub' => [ + 'secondSubSub' => 2, + ], + ], + ], + [ + 'first.firstSub.firstSubSub' => 1, + 'second.secondSub.secondSubSub' => 2, + ], + ], + 'nested array of 3 levels with dots in keys' => [ + [ + 'first.' => [ + 'firstSub.' => [ + 'firstSubSub.' => 1, + ], + ], + 'second.' => [ + 'secondSub.' => [ + 'secondSubSub.' => 2, + ], + ], + ], + [ + 'first\..firstSub\..firstSubSub\.' => 1, + 'second\..secondSub\..secondSubSub\.' => 2, + ], + ], + 'duplicate keys, one with dot, one without' => [ + [ + 'foo' => 'node', + 'foo.' => [ + 'bar' => 'bla', + ], + ], + [ + 'foo' => 'node', + 'foo\..bar' => 'bla', + ], + ], + 'duplicate keys, one with dot with scalar value, one without, last wins' => [ + [ + 'foo.' => 'dot', + 'foo' => 'node', + ], + [ + 'foo\.' => 'dot', + 'foo' => 'node', + ], + ], + 'empty key' => [ + [ + '' => 'node', + ], + [ + '' => 'node', + ], + ], + 'dot key' => [ + [ + '.' => 'node', + ], + [ + '\.' => 'node', + ], + ], + 'empty array' => [ + [], + [], + ], + 'nested lists' => [ + [ + ['foo', 'bar'], + ['bla', 'baz'], + ], + [ + '0.0' => 'foo', + '0.1' => 'bar', + '1.0' => 'bla', + '1.1' => 'baz', + ], + ], + ]; + } + + /** + * @test + * @param array $array + * @param array $expected + * @dataProvider flattenPlainCalculatesExpectedResultDataProvider + */ + public function flattenPlainCalculatesExpectedResult(array $array, array $expected): void + { + self::assertEquals($expected, ArrayUtility::flattenPlain($array)); + } + /** * @return array */ diff --git a/typo3/sysext/fluid/Classes/ViewHelpers/Be/Menus/ActionMenuItemViewHelper.php b/typo3/sysext/fluid/Classes/ViewHelpers/Be/Menus/ActionMenuItemViewHelper.php index 4a50551ec192..d46439d732f2 100644 --- a/typo3/sysext/fluid/Classes/ViewHelpers/Be/Menus/ActionMenuItemViewHelper.php +++ b/typo3/sysext/fluid/Classes/ViewHelpers/Be/Menus/ActionMenuItemViewHelper.php @@ -103,13 +103,13 @@ class ActionMenuItemViewHelper extends AbstractTagBasedViewHelper protected function evaluateSelectItemState(string $controller, string $action, array $arguments): void { $currentRequest = $this->renderingContext->getRequest(); - $flatRequestArguments = ArrayUtility::flatten( + $flatRequestArguments = ArrayUtility::flattenPlain( array_merge([ 'controller' => $currentRequest->getControllerName(), 'action' => $currentRequest->getControllerActionName(), ], $currentRequest->getArguments()) ); - $flatViewHelperArguments = ArrayUtility::flatten( + $flatViewHelperArguments = ArrayUtility::flattenPlain( array_merge(['controller' => $controller, 'action' => $action], $arguments) ); if ( diff --git a/typo3/sysext/form/Classes/Domain/Configuration/ArrayProcessing/ArrayProcessor.php b/typo3/sysext/form/Classes/Domain/Configuration/ArrayProcessing/ArrayProcessor.php index 64bee86bff5e..aa884b92f65b 100644 --- a/typo3/sysext/form/Classes/Domain/Configuration/ArrayProcessing/ArrayProcessor.php +++ b/typo3/sysext/form/Classes/Domain/Configuration/ArrayProcessing/ArrayProcessor.php @@ -39,7 +39,7 @@ class ArrayProcessor */ public function __construct(array $data) { - $this->data = ArrayUtility::flatten($data); + $this->data = ArrayUtility::flattenPlain($data); } /** -- GitLab