Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #21267 [Form] Fix ChoiceType to ensure submitted data is not nest…
…ed unnecessarily (issei-m)

This PR was squashed before being merged into the 2.7 branch (closes #21267).

Discussion
----------

[Form] Fix ChoiceType to ensure submitted data is not nested unnecessarily

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Fixed ChoiceType to protect against some problem caused by treating of array.

Let's say we have the choice-form like:

```php
$form = $factory->create(ChoiceType, null, [
    'choices' => [
        'A',
        'B',
        'C',
    ],
    'expanded' => true,
    'multiple' => true,
]);
```

Then, submit data like this:

```php
$form->submit([
    [], // unnecessality nested
]);
```

(Yes, I agree in most cases these situation doesn't happen, but can be)

Then, we get `array_flip(): Can only flip STRING and INTEGER values!` error at [here](https://github.com/symfony/symfony/blob/6babdb3296a5fbbd9c69d1e3410adbdf749959ef/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php#L114).
Even if form is not `multiple`, annoying `Array to string conversion` error occurs in [here](https://github.com/symfony/symfony/blob/6babdb3296a5fbbd9c69d1e3410adbdf749959ef/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php#L144) (via [ChoicesToValuesTransformer](https://github.com/symfony/symfony/blob/5129c4cf7e294b1a5ea30d6fec6e89b75396dcd2/src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoicesToValuesTransformer.php#L74)).
(As far as I know, non-multiple and non-expanded form has no problem, thanks to [ChoiceToValueTransformer](https://github.com/symfony/symfony/blob/6babdb3296a5fbbd9c69d1e3410adbdf749959ef/src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoiceToValueTransformer.php#L43))

To resolve these problems, I just added a simple-validation listener to choice type.

Commits
-------

64d7a82 [Form] Fix ChoiceType to ensure submitted data is not nested unnecessarily
  • Loading branch information
fabpot committed Mar 1, 2017
2 parents 0a15eea + 64d7a82 commit 8175248
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
20 changes: 18 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
Expand Up @@ -160,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 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();

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);
}

/**
Expand Down Expand Up @@ -505,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
Expand Down
Expand Up @@ -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',
Expand Down Expand Up @@ -2283,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'));
}

/**
* @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 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())),
);
}
}

0 comments on commit 8175248

Please sign in to comment.