[BUGFIX] Fix keepItems and removeItems handling with 0 values 23/54623/2
authorHelmut Hummel <typo3@helhum.io>
Sun, 12 Nov 2017 18:27:49 +0000 (19:27 +0100)
committerJigal van Hemert <jigal.van.hemert@typo3.org>
Thu, 16 Nov 2017 20:22:37 +0000 (21:22 +0100)
TSConfig properties keepItems and removeItems are checked
with a weak in_array() test (third argument not set to true),
which leads to the situation, that arbitrary string values
are treated equal to integer 0

This is now fixed by flipping the array and check for the
array index. This works nicely because PHP silently
converts strings which look like integer to integer for the keys.

Tests are added which cover these cases and demonstrate
the failure before the change.

Resolves: #82980
Releases: 8.7, master
Change-Id: I544a221674fa89d302cb6c1bcca506847c6f7f0f
Reviewed-on: https://review.typo3.org/54623
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Joerg Boesche <typo3@joergboesche.de>
Reviewed-by: Henning Liebe <h.liebe@neusta.de>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Reviewed-by: Jan Stockfisch <jan.stockfisch@googlemail.com>
Tested-by: Jan Stockfisch <jan.stockfisch@googlemail.com>
Reviewed-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
Tested-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
typo3/sysext/backend/Classes/Controller/ContentElement/NewContentElementController.php
typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php
typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php
typo3/sysext/core/Classes/Utility/ArrayUtility.php
typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php

index c15b79e..c80ca0c 100644 (file)
@@ -496,22 +496,22 @@ class NewContentElementController
                         $authModeDeny = $config['type'] === 'select' && $config['authMode']
                             && !$backendUser->checkAuthMode('tt_content', $fN, $fV, $config['authMode']);
                         // explode TSconfig keys only as needed
-                        if (!isset($removeItems[$fN])) {
-                            $removeItems[$fN] = GeneralUtility::trimExplode(
+                        if (!isset($removeItems[$fN]) && isset($TCEFORM_TSconfig[$fN]['removeItems']) && $TCEFORM_TSconfig[$fN]['removeItems'] !== '') {
+                            $removeItems[$fN] = array_flip(GeneralUtility::trimExplode(
                                 ',',
                                 $TCEFORM_TSconfig[$fN]['removeItems'],
                                 true
-                            );
+                            ));
                         }
-                        if (!isset($keepItems[$fN])) {
-                            $keepItems[$fN] = GeneralUtility::trimExplode(
+                        if (!isset($keepItems[$fN]) && isset($TCEFORM_TSconfig[$fN]['keepItems']) && $TCEFORM_TSconfig[$fN]['keepItems'] !== '') {
+                            $keepItems[$fN] = array_flip(GeneralUtility::trimExplode(
                                 ',',
                                 $TCEFORM_TSconfig[$fN]['keepItems'],
                                 true
-                            );
+                            ));
                         }
-                        $isNotInKeepItems = !empty($keepItems[$fN]) && !in_array($fV, $keepItems[$fN]);
-                        if ($authModeDeny || $fN === 'CType' && (in_array($fV, $removeItems[$fN]) || $isNotInKeepItems)) {
+                        $isNotInKeepItems = !empty($keepItems[$fN]) && !isset($keepItems[$fN][$fV]);
+                        if ($authModeDeny || ($fN === 'CType' && (isset($removeItems[$fN][$fV]) || $isNotInKeepItems))) {
                             // Remove element all together:
                             unset($wizardItems[$key]);
                             break;
index ad41104..cce602d 100644 (file)
@@ -533,8 +533,7 @@ abstract class AbstractItemProvider
         }
 
         // If keepItems is set but is an empty list all current items get removed
-        if (empty($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'])
-            && $result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'] !== '0') {
+        if ($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'] === '') {
             return [];
         }
 
@@ -560,19 +559,20 @@ abstract class AbstractItemProvider
     protected function removeItemsByRemoveItemsPageTsConfig(array $result, $fieldName, array $items)
     {
         $table = $result['tableName'];
-        if (empty($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['removeItems'])
+        if (!isset($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['removeItems'])
             || !is_string($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['removeItems'])
+            || $result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['removeItems'] === ''
         ) {
             return $items;
         }
 
-        $removeItems = GeneralUtility::trimExplode(
+        $removeItems = array_flip(GeneralUtility::trimExplode(
             ',',
             $result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['removeItems'],
             true
-        );
+        ));
         foreach ($items as $key => $itemValues) {
-            if (in_array($itemValues[1], $removeItems)) {
+            if (isset($removeItems[$itemValues[1]])) {
                 unset($items[$key]);
             }
         }
index feba451..7eee75e 100644 (file)
@@ -52,10 +52,6 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     protected function setUp()
     {
         $this->singletonInstances = GeneralUtility::getSingletonInstances();
-        $this->subject = $this->getMockBuilder(TcaSelectItems::class)
-            ->setMethods(['getDatabaseRow'])
-            ->getMock();
-
         $this->subject = new TcaSelectItems();
     }
 
@@ -2080,6 +2076,10 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                                     0 => 'removeMe',
                                     1 => 'remove',
                                 ],
+                                2 => [
+                                    0 => 'removeMe',
+                                    1 => 0,
+                                ],
                             ],
                             'maxitems' => 99999,
                         ],
@@ -2104,7 +2104,10 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
 
         $expected = $input;
         $expected['databaseRow']['aField'] = [];
-        unset($expected['processedTca']['columns']['aField']['config']['items'][1]);
+        unset(
+            $expected['processedTca']['columns']['aField']['config']['items'][1],
+            $expected['processedTca']['columns']['aField']['config']['items'][2]
+        );
 
         $this->assertEquals($expected, $this->subject->addData($input));
     }
@@ -2271,6 +2274,12 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                                     0 => 'removeMe',
                                     1 => 'remove',
                                 ],
+                                2 => [
+                                    0 => 'keep me',
+                                    1 => 0,
+                                    null,
+                                    null,
+                                ],
                             ],
                             'maxitems' => 99999,
                         ],
@@ -2296,7 +2305,68 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
         $expected = $input;
         $expected['databaseRow']['aField'] = [];
         unset($expected['processedTca']['columns']['aField']['config']['items'][1]);
+        $expected['processedTca']['columns']['aField']['config']['items'] = array_values($expected['processedTca']['columns']['aField']['config']['items']);
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
+     * @test
+     */
+    public function addDataRemovesItemsByZeroValueRemoveItemsPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => ''
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => 'keep',
+                                    null,
+                                    null,
+                                ],
+                                1 => [
+                                    0 => 'keepMe',
+                                    1 => 'keepMe2',
+                                    null,
+                                    null,
+                                ],
+                                2 => [
+                                    0 => 'remove me',
+                                    1 => 0,
+                                ],
+                            ],
+                            'maxitems' => 99999,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'removeItems' => '0',
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
 
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        unset($expected['processedTca']['columns']['aField']['config']['items'][2]);
         $this->assertEquals($expected, $this->subject->addData($input));
     }
 
index 7610f30..106025a 100644 (file)
@@ -685,10 +685,11 @@ class ArrayUtility
             }
             // Do the filtering:
             if (is_array($keepItems) && !empty($keepItems)) {
+                $keepItems = array_flip($keepItems);
                 foreach ($array as $key => $value) {
                     // Get the value to compare by using the callback function:
                     $keepValue = isset($getValueFunc) ? call_user_func($getValueFunc, $value) : $value;
-                    if (!in_array($keepValue, $keepItems)) {
+                    if (!isset($keepItems[$keepValue])) {
                         unset($array[$key]);
                     }
                 }
index 2c2771e..0e4740b 100644 (file)
@@ -1964,6 +1964,7 @@ class ArrayUtilityTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     public function keepItemsInArrayWorksWithOneArgumentDataProvider()
     {
         $array = [
+            0 => 0,
             'one' => 'one',
             'two' => 'two',
             'three' => 'three'