From 3399cb6a3a49f361901f13f8d73d34c68bc1bd00 Mon Sep 17 00:00:00 2001
From: Marco Pfeiffer <noreply@example.com>
Date: Wed, 26 Feb 2020 10:21:01 +0000
Subject: [PATCH] [TASK] use strict type comparison for isValueSelected
\TYPO3\CMS\Fluid\ViewHelpers\Form\SelectViewHelper::isSelected
uses a strict type comparison but the OptionViewHelper does not.
This resulted in funny results when the values were convertable to int
like this:
var_dump(in_array(1, ['1-2'])); // prints bool(true)
The value is now converted to a string, just like in the
SelectViewHelper, and then strictly compared.
Releases: master
Resolves: #90540
Change-Id: I6c28316ca306f5188141a0222b6f09a1d1fc108e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63439
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog <look@susi.dev>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Susanne Moog <look@susi.dev>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
---
.../Form/Select/OptionViewHelper.php | 16 ++--
.../Form/Select/OptionViewHelperTest.php | 89 +++++++++++++++++--
2 files changed, 92 insertions(+), 13 deletions(-)
diff --git a/typo3/sysext/fluid/Classes/ViewHelpers/Form/Select/OptionViewHelper.php b/typo3/sysext/fluid/Classes/ViewHelpers/Form/Select/OptionViewHelper.php
index e6c1791ab182..79e6fa59a972 100644
--- a/typo3/sysext/fluid/Classes/ViewHelpers/Form/Select/OptionViewHelper.php
+++ b/typo3/sysext/fluid/Classes/ViewHelpers/Form/Select/OptionViewHelper.php
@@ -42,12 +42,12 @@ class OptionViewHelper extends \TYPO3\CMS\Fluid\ViewHelpers\Form\AbstractFormFie
*/
public function render()
{
- if ($this->arguments['selected'] ?? $this->isValueSelected($this->arguments['value'])) {
- $this->tag->addAttribute('selected', 'selected');
- }
$childContent = $this->renderChildren();
$this->tag->setContent($childContent);
$value = $this->arguments['value'] ?? $childContent;
+ if ($this->arguments['selected'] ?? $this->isValueSelected((string)$value)) {
+ $this->tag->addAttribute('selected', 'selected');
+ }
$this->tag->addAttribute('value', $value);
$parentRequestedFormTokenFieldName = $this->renderingContext->getViewHelperVariableContainer()->get(
SelectViewHelper::class,
@@ -63,18 +63,18 @@ class OptionViewHelper extends \TYPO3\CMS\Fluid\ViewHelpers\Form\AbstractFormFie
}
/**
- * @param mixed $value
+ * @param string $value
* @return bool
*/
- protected function isValueSelected($value)
+ protected function isValueSelected(string $value): bool
{
$selectedValue = $this->renderingContext->getViewHelperVariableContainer()->get(SelectViewHelper::class, 'selectedValue');
if (is_array($selectedValue)) {
- return in_array($value, $selectedValue);
+ return in_array($value, $selectedValue, true);
}
if ($selectedValue instanceof \Iterator) {
- return in_array($value, iterator_to_array($selectedValue));
+ return in_array($value, iterator_to_array($selectedValue), true);
}
- return $value == $selectedValue;
+ return $value === $selectedValue;
}
}
diff --git a/typo3/sysext/fluid/Tests/Unit/ViewHelpers/Form/Select/OptionViewHelperTest.php b/typo3/sysext/fluid/Tests/Unit/ViewHelpers/Form/Select/OptionViewHelperTest.php
index 5ad7219bb280..21bc26c7aa0e 100644
--- a/typo3/sysext/fluid/Tests/Unit/ViewHelpers/Form/Select/OptionViewHelperTest.php
+++ b/typo3/sysext/fluid/Tests/Unit/ViewHelpers/Form/Select/OptionViewHelperTest.php
@@ -17,6 +17,7 @@ namespace TYPO3\CMS\Fluid\Tests\Unit\ViewHelpers\Form\Select;
*/
use TYPO3\CMS\Fluid\ViewHelpers\Form\Select\OptionViewHelper;
+use TYPO3\CMS\Fluid\ViewHelpers\Form\SelectViewHelper;
use TYPO3\TestingFramework\Fluid\Unit\ViewHelpers\ViewHelperBaseTestcase;
use TYPO3Fluid\Fluid\Core\ViewHelper\TagBuilder;
@@ -34,8 +35,9 @@ class OptionViewHelperTest extends ViewHelperBaseTestcase
{
parent::setUp();
$this->viewHelper = $this->getMockBuilder(OptionViewHelper::class)
- ->setMethods(['isValueSelected', 'registerFieldNameForFormTokenGeneration', 'renderChildren'])
+ ->onlyMethods(['registerFieldNameForFormTokenGeneration', 'renderChildren'])
->getMock();
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'registerFieldNameForFormTokenGeneration')->willReturn('');
$this->arguments['selected'] = null;
$this->arguments['value'] = null;
$this->tagBuilder = new TagBuilder();
@@ -49,6 +51,7 @@ class OptionViewHelperTest extends ViewHelperBaseTestcase
public function optionTagNameIsSet(): void
{
$tagBuilder = $this->createMock(TagBuilder::class);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn('');
$tagBuilder->expects(self::atLeastOnce())->method('setTagName')->with('option');
$this->viewHelper->setTagBuilder($tagBuilder);
$this->viewHelper->initializeArgumentsAndRender();
@@ -60,6 +63,7 @@ class OptionViewHelperTest extends ViewHelperBaseTestcase
public function childrenContentIsUsedAsValueAndLabelByDefault(): void
{
$this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn('');
$expected = '<option value="Option Label">Option Label</option>';
self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
}
@@ -71,6 +75,7 @@ class OptionViewHelperTest extends ViewHelperBaseTestcase
{
$this->arguments['value'] = 'value';
$this->viewHelper->setArguments($this->arguments);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn('');
$this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
$expected = '<option value="value">Option Label</option>';
self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
@@ -83,29 +88,103 @@ class OptionViewHelperTest extends ViewHelperBaseTestcase
{
$this->arguments['selected'] = null;
$this->viewHelper->setArguments($this->arguments);
-
- $this->viewHelper->expects(self::once())->method('isValueSelected')->willReturn(true);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn('Option Label');
$this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
$expected = '<option selected="selected" value="Option Label">Option Label</option>';
self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
}
+ public function selectedIsAddedToSelectedOptionForProvidedValueDataProvider(): array
+ {
+ return [
+ 'string value, string selection' => [
+ 'val1', 'val1'
+ ],
+ 'string value, array selection' => [
+ 'val1', ['val1']
+ ],
+ 'string value, iterable selection' => [
+ 'val1', (new \ArrayObject(['val1']))->getIterator()
+ ],
+ 'int value, array selection' => [
+ 1, ['1']
+ ]
+ ];
+ }
+
/**
* @test
+ * @dataProvider selectedIsAddedToSelectedOptionForProvidedValueDataProvider
+ * @param string $value
+ * @param mixed $selected
*/
- public function selectedIsNotAddedToUnselectedOptionForNoSelectionOverride(): void
+ public function selectedIsAddedToSelectedOptionForProvidedValue($value, $selected): void
{
$this->arguments['selected'] = null;
+ $this->arguments['value'] = $value;
+ $this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
$this->viewHelper->setArguments($this->arguments);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn($selected);
- $this->viewHelper->expects(self::once())->method('isValueSelected')->willReturn(false);
+ $expected = '<option value="' . $value . '" selected="selected">Option Label</option>';
+ self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
+ }
+
+ /**
+ * @test
+ */
+ public function selectedIsNotAddedToUnselectedOptionForNoSelectionOverride(): void
+ {
+ $this->arguments['selected'] = null;
+ $this->viewHelper->setArguments($this->arguments);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn('');
$this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
$expected = '<option value="Option Label">Option Label</option>';
self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
}
+ public function selectedIsNotAddedToSelectedOptionForProvidedValueDataProvider(): array
+ {
+ return [
+ 'string value, string selection' => [
+ '1',
+ '1-2'
+ ],
+ 'string value, array selection' => [
+ 'val1',
+ ['val3']
+ ],
+ 'string value, iterable selection' => [
+ 'val1',
+ (new \ArrayObject(['val3']))->getIterator()
+ ],
+ 'int value, array selection' => [
+ 1,
+ ['1-2']
+ ]
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider selectedIsNotAddedToSelectedOptionForProvidedValueDataProvider
+ * @param string $value
+ * @param mixed $selected
+ */
+ public function selectedIsNotAddedToSelectedOptionForProvidedValue($value, $selected): void
+ {
+ $this->arguments['selected'] = null;
+ $this->arguments['value'] = $value;
+ $this->viewHelper->expects(self::once())->method('renderChildren')->willReturn('Option Label');
+ $this->viewHelper->setArguments($this->arguments);
+ $this->viewHelperVariableContainer->get(SelectViewHelper::class, 'selectedValue')->willReturn($selected);
+
+ $expected = '<option value="' . $value . '">Option Label</option>';
+ self::assertEquals($expected, $this->viewHelper->initializeArgumentsAndRender());
+ }
+
/**
* @test
*/
--
GitLab