Commit c1543fc2 authored by Helmut Hummel's avatar Helmut Hummel Committed by Jigal van Hemert
Browse files

[BUGFIX] Fix keepItems and removeItems handling with 0 values

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: default avatarTYPO3com <no-reply@typo3.com>
Reviewed-by: default avatarJoerg Boesche <typo3@joergboesche.de>
Reviewed-by: Henning Liebe's avatarHenning Liebe <h.liebe@neusta.de>
Reviewed-by: default avatarStefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: default avatarStefan Neufeind <typo3.neufeind@speedpartner.de>
Reviewed-by: default avatarJan Stockfisch <jan.stockfisch@googlemail.com>
Tested-by: default avatarJan Stockfisch <jan.stockfisch@googlemail.com>
Reviewed-by: Jigal van Hemert's avatarJigal van Hemert <jigal.van.hemert@typo3.org>
Tested-by: Jigal van Hemert's avatarJigal van Hemert <jigal.van.hemert@typo3.org>
parent bb9c7729
......@@ -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;
......
......@@ -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]);
}
}
......
......@@ -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));
}
......
......@@ -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]);
}
}
......
......@@ -1964,6 +1964,7 @@ class ArrayUtilityTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
public function keepItemsInArrayWorksWithOneArgumentDataProvider()
{
$array = [
0 => 0,
'one' => 'one',
'two' => 'two',
'three' => 'three'
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment