[BUGFIX] Make TypoScriptParser sortList more strict 12/42112/5
authorBenjamin Mack <benni@typo3.org>
Wed, 29 Jul 2015 18:01:24 +0000 (20:01 +0200)
committerSusanne Moog <typo3@susannemoog.de>
Thu, 6 Aug 2015 11:07:06 +0000 (13:07 +0200)
When calling the := sortList() with a "numeric" modifier
of the TypoScript parser with a string, the sort() method
differs between PHP versions. In order to make this behaviour
more strict, a check is done before the elements are sorted
to only have numeric values in the list, otherwise an Exception
is now thrown.

As this changes behaviour, the test should be excluded for
PHP7 in 6.2, the behaviour cannot be modified in 6.2 without
possibly breaking the output of a Frontend site.

Resolves: #65317
Releases: master
Change-Id: Ife4f0de367398e6e5e35b6df9f1c0ea980597773
Reviewed-on: http://review.typo3.org/42112
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Susanne Moog <typo3@susannemoog.de>
Tested-by: Susanne Moog <typo3@susannemoog.de>
typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php
typo3/sysext/core/Documentation/Changelog/master/Breaking-65317-TypoScriptParserSortListSanitizesInputOnNumericalSort.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/TypoScript/Parser/TypoScriptParserTest.php

index 9ea69f9..842400e 100644 (file)
@@ -562,6 +562,15 @@ class TypoScriptParser {
                                $sort_flags = SORT_REGULAR;
                                if (in_array('numeric', $arguments)) {
                                        $sort_flags = SORT_NUMERIC;
+                                       // If the sorting modifier "numeric" is given, all values
+                                       // are checked and an exception is thrown if a non-numeric value is given
+                                       // otherwise there is a different behaviour between PHP7 and PHP 5.x
+                                       // See also the warning on http://us.php.net/manual/en/function.sort.php
+                                       foreach ($elements as $element) {
+                                               if (!is_numeric($element)) {
+                                                       throw new \InvalidArgumentException('The list "' . $currentValue . '" should be sorted numerically but contains a non-numeric value', 1438191758);
+                                               }
+                                       }
                                }
                                sort($elements, $sort_flags);
                                if (in_array('descending', $arguments)) {
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-65317-TypoScriptParserSortListSanitizesInputOnNumericalSort.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-65317-TypoScriptParserSortListSanitizesInputOnNumericalSort.rst
new file mode 100644 (file)
index 0000000..3bf5b81
--- /dev/null
@@ -0,0 +1,28 @@
+==============================================================================
+Breaking: #65317 - TypoScriptParser sortList sanitizes input on numerical sort
+==============================================================================
+
+Description
+===========
+
+When calling the := sortList() with a "numeric" modifier of the TypoScript parser with a string, the sort() method
+differs between PHP versions. In order to make this behavior more strict, a check is done before the elements are
+sorted to only have numeric values in the list, otherwise an Exception is now thrown.
+
+
+Impact
+======
+
+An exception is thrown if non-numerical values are given for a numeric sort in TypoScript sortList.
+
+
+Affected Installations
+======================
+
+All installations using sortList numeric with non-numerical values.
+
+
+Migration
+=========
+
+Either remove the non-numerical values from the list or change the sort order to be non-numerical (ascending / descending).
index 5e9e8f5..e4c04a1 100644 (file)
@@ -193,9 +193,9 @@ class TypoScriptParserTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                        ),
                        'sortList sorts a list numeric' => array(
                                'sortList',
-                               '10,0,100,-20,abc',
+                               '10,0,100,-20',
                                'numeric',
-                               '-20,0,abc,10,100',
+                               '-20,0,10,100',
                        ),
                        'sortList sorts a list descending' => array(
                                'sortList',
@@ -205,9 +205,9 @@ class TypoScriptParserTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                        ),
                        'sortList sorts a list numeric descending' => array(
                                'sortList',
-                               '10,100,0,20,abc,-20',
+                               '10,100,0,20,-20',
                                'descending,numeric',
-                               '100,20,10,0,abc,-20',
+                               '100,20,10,0,-20',
                        ),
                        'sortList ignores invalid modifier arguments' => array(
                                'sortList',
@@ -227,6 +227,36 @@ class TypoScriptParserTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertEquals($expected, $actualValue);
        }
 
+
+       /**
+        * Data provider for executeValueModifierThrowsException
+        *
+        * @return array modifier name, modifier arguments, current value, expected result
+        */
+       public function executeValueModifierInvalidDataProvider() {
+               return array(
+                       'sortList sorts a list numeric' => array(
+                               'sortList',
+                               '10,0,100,-20,abc',
+                               'numeric',
+                       ),
+                       'sortList sorts a list numeric descending' => array(
+                               'sortList',
+                               '10,100,0,20,abc,-20',
+                               'descending,numeric',
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @dataProvider executeValueModifierInvalidDataProvider
+        */
+       public function executeValueModifierThrowsException($modifierName, $currentValue, $modifierArgument) {
+               $this->setExpectedException('InvalidArgumentException', 'The list "' . $currentValue . '" should be sorted numerically but contains a non-numeric value');
+               $this->typoScriptParser->_call('executeValueModifier', $modifierName, $modifierArgument, $currentValue);
+       }
+
        /**
         * @param string $typoScript
         * @param array $expected