From cd7d2a4805f4fb8c72390d09608687ca5a5b0a08 Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 13 Jan 2017 14:08:30 +0900 Subject: [PATCH 1/7] [Form] Simplify FQCN in ChoiceTypeTest --- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index b675d5d6eee08..0e02389f6bebc 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -14,9 +14,10 @@ use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView; use Symfony\Component\Form\ChoiceList\View\ChoiceView; use Symfony\Component\Form\Extension\Core\ChoiceList\ObjectChoiceList; +use Symfony\Component\Form\Test\TypeTestCase; use Symfony\Component\Form\Tests\Fixtures\ChoiceSubType; -class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase +class ChoiceTypeTest extends TypeTestCase { private $choices = array( 'Bernhard' => 'a', From fb33ad25514aecf0b173f8909e06bc921ebacbfc Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 13 Jan 2017 15:07:56 +0900 Subject: [PATCH 2/7] [Form] Add failure test case to ChoiceTypeTest for invalid nested value --- .../Extension/Core/Type/ChoiceTypeTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 0e02389f6bebc..5d83a3d44ef38 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -2284,4 +2284,30 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() // In this case the 'choice_label' closure returns null and not the closure from the first choice type. $this->assertNull($form->get('subChoice')->getConfig()->getOption('choice_label')); } + + public function invalidNestedValueTestMatrix() + { + return array( + 'non-multiple, non-expanded' => array(false, false, array(array())), + 'non-multiple, expanded' => array(false, true, array(array())), + 'multiple, non-expanded' => array(true, false, array(array())), + 'multiple, expanded' => array(true, true, array(array())), + ); + } + + /** + * @dataProvider invalidNestedValueTestMatrix + */ + public function testSubmitInvalidNestedValue($multiple, $expanded, $submissionData) + { + $form = $this->factory->create('choice', null, array( + 'choices' => $this->choices, + 'multiple' => $multiple, + 'expanded' => $expanded, + )); + + $form->submit($submissionData); + $this->assertFalse($form->isSynchronized()); + $this->assertEquals('All elements of submitted array must not be an array.', $form->getTransformationFailure()->getMessage()); + } } From 98195dd06d85203e9a3e6cd1c7bafe9c6e30901e Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 13 Jan 2017 15:09:07 +0900 Subject: [PATCH 3/7] [Form] Fix ChoiceType to ensure submitted data is not nested unnecessarily --- .../Form/Extension/Core/Type/ChoiceType.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index d376109828718..334f8cc66e892 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -65,6 +65,22 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null */ public function buildForm(FormBuilderInterface $builder, array $options) { + // To avoid some problem against treating of array (e.g. Array to string conversion), + // we have to first ensure the all elements of submitted data ain't an array. + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { + $data = $event->getData(); + + if (!is_array($data)) { + return; + } + + foreach ($data as $v) { + if (is_array($v)) { + throw new TransformationFailedException('All elements of submitted array must not be an array.'); + } + } + }, 256); + if ($options['expanded']) { $builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper()); From 9d406db0803ae8f056f05be40079bfed2e082c91 Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 13 Jan 2017 15:46:53 +0900 Subject: [PATCH 4/7] Fix CS --- By applying http://fabbot.io/patch/symfony/symfony/21267/466bebe26e8697d9c32cea0044b25f580c04a3bf/cs.diff --- .../Component/Form/Extension/Core/Type/ChoiceType.php | 4 ++-- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 334f8cc66e892..a22b7a1fdc8f3 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -521,8 +521,8 @@ private function createChoiceListView(ChoiceListInterface $choiceList, array $op * "choice_label" closure by default. * * @param array|\Traversable $choices The choice labels indexed by choices - * @param object $choiceLabels The object that receives the choice labels - * indexed by generated keys. + * @param object $choiceLabels the object that receives the choice labels + * indexed by generated keys * @param int $nextKey The next generated key * * @return array The choices in a normalized array with labels replaced by generated keys diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 5d83a3d44ef38..4f29e585f104d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -2289,9 +2289,9 @@ public function invalidNestedValueTestMatrix() { return array( 'non-multiple, non-expanded' => array(false, false, array(array())), - 'non-multiple, expanded' => array(false, true, array(array())), - 'multiple, non-expanded' => array(true, false, array(array())), - 'multiple, expanded' => array(true, true, array(array())), + 'non-multiple, expanded' => array(false, true, array(array())), + 'multiple, non-expanded' => array(true, false, array(array())), + 'multiple, expanded' => array(true, true, array(array())), ); } @@ -2301,7 +2301,7 @@ public function invalidNestedValueTestMatrix() public function testSubmitInvalidNestedValue($multiple, $expanded, $submissionData) { $form = $this->factory->create('choice', null, array( - 'choices' => $this->choices, + 'choices' => $this->choices, 'multiple' => $multiple, 'expanded' => $expanded, )); From 61c80c2a1d4e9176143792df5ef6ffbd60ca2a1e Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Thu, 19 Jan 2017 20:25:20 +0900 Subject: [PATCH 5/7] [Form] Fix validation-listener logic and move block to the end-of-method --- .../Form/Extension/Core/Type/ChoiceType.php | 32 +++++++++---------- .../Extension/Core/Type/ChoiceTypeTest.php | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index a22b7a1fdc8f3..e7c2f2aee521a 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -65,22 +65,6 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null */ public function buildForm(FormBuilderInterface $builder, array $options) { - // To avoid some problem against treating of array (e.g. Array to string conversion), - // we have to first ensure the all elements of submitted data ain't an array. - $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { - $data = $event->getData(); - - if (!is_array($data)) { - return; - } - - foreach ($data as $v) { - if (is_array($v)) { - throw new TransformationFailedException('All elements of submitted array must not be an array.'); - } - } - }, 256); - if ($options['expanded']) { $builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper()); @@ -176,6 +160,22 @@ public function buildForm(FormBuilderInterface $builder, array $options) // transformation is merged back into the original collection $builder->addEventSubscriber(new MergeCollectionListener(true, true)); } + + // To avoid some problem against treating of array (e.g. Array to string conversion), + // we have to first ensure the all elements of submitted data ain't an array. + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { + $data = $event->getData(); + + if (!is_array($data)) { + return; + } + + foreach ($data as $v) { + if (null !== $v && !is_string($v)) { + throw new TransformationFailedException('All choices submitted must be NULL or strings.'); + } + } + }, 256); } /** diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 4f29e585f104d..439cccaad55e2 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -2308,6 +2308,6 @@ public function testSubmitInvalidNestedValue($multiple, $expanded, $submissionDa $form->submit($submissionData); $this->assertFalse($form->isSynchronized()); - $this->assertEquals('All elements of submitted array must not be an array.', $form->getTransformationFailure()->getMessage()); + $this->assertEquals('All choices submitted must be NULL or strings.', $form->getTransformationFailure()->getMessage()); } } From eb4d85a0ed0dc52c389f733f6eda367e05a13850 Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 10 Feb 2017 00:45:57 +0900 Subject: [PATCH 6/7] Update comment --- src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index e7c2f2aee521a..489d5d77d72e4 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -161,8 +161,8 @@ public function buildForm(FormBuilderInterface $builder, array $options) $builder->addEventSubscriber(new MergeCollectionListener(true, true)); } - // To avoid some problem against treating of array (e.g. Array to string conversion), - // we have to first ensure the all elements of submitted data ain't an array. + // To avoid issues when the submitted choices are arrays (i.e. array to string conversions), + // we have to ensure that all elements of the submitted choice data are strings or null. $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { $data = $event->getData(); From d6d2db52038efbf56ccd38d28728dfaf5021e7fd Mon Sep 17 00:00:00 2001 From: "Issei.M" Date: Fri, 10 Feb 2017 12:47:31 +0900 Subject: [PATCH 7/7] Move test data provider after the test method --- .../Extension/Core/Type/ChoiceTypeTest.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 439cccaad55e2..10adffbecc26d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -2285,16 +2285,6 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() $this->assertNull($form->get('subChoice')->getConfig()->getOption('choice_label')); } - public function invalidNestedValueTestMatrix() - { - return array( - 'non-multiple, non-expanded' => array(false, false, array(array())), - 'non-multiple, expanded' => array(false, true, array(array())), - 'multiple, non-expanded' => array(true, false, array(array())), - 'multiple, expanded' => array(true, true, array(array())), - ); - } - /** * @dataProvider invalidNestedValueTestMatrix */ @@ -2310,4 +2300,14 @@ public function testSubmitInvalidNestedValue($multiple, $expanded, $submissionDa $this->assertFalse($form->isSynchronized()); $this->assertEquals('All choices submitted must be NULL or strings.', $form->getTransformationFailure()->getMessage()); } + + public function invalidNestedValueTestMatrix() + { + return array( + 'non-multiple, non-expanded' => array(false, false, array(array())), + 'non-multiple, expanded' => array(false, true, array(array())), + 'multiple, non-expanded' => array(true, false, array(array())), + 'multiple, expanded' => array(true, true, array(array())), + ); + } }