[BUGFIX] Fix keepItems and removeItems handling with 0 values 26/54626/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:24:25 +0000 (21:24 +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/54626
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Joerg Boesche <typo3@joergboesche.de>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
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 08203cd..a0604de 100644 (file)
@@ -489,22 +489,22 @@ class NewContentElementController extends AbstractModule
                         $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 8bfc949..4016b9d 100644 (file)
@@ -551,8 +551,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 [];
         }
 
@@ -578,19 +577,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 8724693..009de51 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();
     }
 
@@ -2125,6 +2121,10 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                                     0 => 'removeMe',
                                     1 => 'remove',
                                 ],
+                                2 => [
+                                    0 => 'removeMe',
+                                    1 => 0,
+                                ],
                             ],
                             'maxitems' => 99999,
                         ],
@@ -2149,7 +2149,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));
     }
@@ -2316,6 +2319,12 @@ class TcaSelectItemsTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
                                     0 => 'removeMe',
                                     1 => 'remove',
                                 ],
+                                2 => [
+                                    0 => 'keep me',
+                                    1 => 0,
+                                    null,
+                                    null,
+                                ],
                             ],
                             'maxitems' => 99999,
                         ],
@@ -2341,7 +2350,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 74abcf7..932a992 100644 (file)
@@ -719,10 +719,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 1b28a30..3f0d45d 100644 (file)
@@ -1994,6 +1994,7 @@ class ArrayUtilityTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     public function keepItemsInArrayWorksWithOneArgumentDataProvider()
     {
         $array = [
+            0 => 0,
             'one' => 'one',
             'two' => 'two',
             'three' => 'three'