Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeated field type required is a required option #11694

Closed
kix opened this issue Aug 18, 2014 · 8 comments
Closed

Repeated field type required is a required option #11694

kix opened this issue Aug 18, 2014 · 8 comments
Labels

Comments

@kix
Copy link
Contributor

kix commented Aug 18, 2014

The Symfony\Component\Form\Extension\Core\Type\RepeatedType throws Illegal string offset 'required' when a required option is not passed to the form builder. Could this be resolved via the OptionsResolverInterface::setRequired() method call in the RepeatedType::setDefaultOptions method?

@kix kix changed the title R Repeated field type required is a required option Aug 18, 2014
@kix
Copy link
Contributor Author

kix commented Aug 18, 2014

My exact usecase is this. The form type:

class UserType extends AbstractAPIType
{

    /**
     * @param FormBuilderInterface $builder
     * @param array                $options
     */
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('email', 'email', [])
            ->add('username', 'text', [])
            ->add('password', 'repeated', [
                'type' => 'password',
                'required' => true,
                'first_options' => 'Password',
                'second_options' => 'Password (again)',
            ])
        ;
    }
}

...extending this simple wrapper

class AbstractAPIType extends AbstractType
{

    /**
     * Returns the name of this type.
     *
     * @return string The name of this type
     */
    final public function getName()
    {
        return '';
    }

    /**
     * @param OptionsResolverInterface $resolver
     */
    public function setDefaultOptions(OptionsResolverInterface $resolver)
    {
        $resolver
            ->setDefaults(['csrf_protection' => false])
        ;
    }
}

...throws an error in a test:

class UserTypeTest extends TypeTestCase
{
    public function testSubmitValidData()
    {
        $formData = [
            'email' => 'email@mail.com',
            'name'  => 'test user',
            'password' => '12345',
        ];

        $type = new UserType();
        $form = $this->factory->create($type);

        $form->submit($formData);
        $this->assertInstanceOf(User::class, $form->getData());

        $view = $form->createView();
        $children = $view->children;

        foreach (array_keys($formData) as $key) {
            $this->assertArrayHasKey($key, $children);
        }
    }
}

The trace and the error:

Illegal string offset 'required'

/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php:27
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/ResolvedFormType.php:140
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/FormFactory.php:91
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/FormBuilder.php:106
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/FormBuilder.php:271
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/FormBuilder.php:219
/Volumes/Storage/Users/kix/Sites/myproj/vendor/symfony/symfony/src/Symfony/Component/Form/FormFactory.php:39
/Volumes/Storage/Users/kix/Sites/myproj/src/MyProj/APIBundle/Tests/Form/UserTypeTest.php:38
/Users/kix/.composer/vendor/phpunit/phpunit/src/TextUI/Command.php:179
/Users/kix/.composer/vendor/phpunit/phpunit/src/TextUI/Command.php:132

@stof
Copy link
Member

stof commented Aug 18, 2014

The required option has a default value for the repeated type, which is set by its parent type form. So this should not happen.
Are you registering a custom type in your application named form which would overwrite the core form type (which is the parent of all other types except buttons) ?

@kix
Copy link
Contributor Author

kix commented Aug 18, 2014

@stof, no, there isn't any form type that would do this; I've also tried commenting out all of the non-standard bundles, the error is still there.
I've changed my UserType to extend Symfony\Component\Form\AbstractType, and it still fails; I'll try this once again against a clean Symfony install now (currently I'm on 2.5)

@kix
Copy link
Contributor Author

kix commented Aug 18, 2014

@stof, well, it's actually a bit more funny: I've been passing the labels like this:

'first_options' => 'Password',

So the $options parameter passed into RepeatedType::buildForm is a string, and PHP treats it as an array while there's no way a string could have a string offset at this line.

Adding this into RepeatedType::setDefaultOptions resolves the issue:

$resolver->setAllowedTypes(array(
    'first_options' => 'array',
    'second_options' => 'array',
 ));

@kix
Copy link
Contributor Author

kix commented Aug 18, 2014

I'll try to cover this with tests and issue a PR now, I think the issue is pretty obvious now.

@stof
Copy link
Member

stof commented Aug 18, 2014

hmm, the right fix is indeed to add the allowed types for these 2 options to enforce passing an array

@kix
Copy link
Contributor Author

kix commented Aug 19, 2014

This is resolved, no need to widen the scope I think.

@kix kix closed this as completed Aug 19, 2014
@kix kix reopened this Aug 19, 2014
webmozart added a commit that referenced this issue Oct 16, 2014
…me form types (kix)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #11696).

Discussion
----------

[Form] Fix #11694 - Enforce options value type check in some form types

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

`Symfony\Component\Form\Extension\Core\Type\RepeatedType` allowed passing strings as options, which caused problems as in #11694.

Commits
-------

0af6467 [Form] Fix #11694 - Enforce options value type check in some form types
@webmozart
Copy link
Contributor

Fixed in #11696.

fabpot added a commit that referenced this issue Oct 24, 2014
* 2.3:
  enforce memcached version to be 2.1.0
  [FrameworkBundle] improve server:run feedback
  [Form] no need to add the url listener when it does not do anything
  [Form] Fix #11694 - Enforce options value type check in some form types
  Lithuanian security translations
  [Router] Cleanup
  [FrameworkBundle] Fixed ide links
  Add missing argument
  [TwigBundle] do not pass a template reference to twig
  [TwigBundle] show correct fallback exception template in debug mode
  [TwigBundle] remove unused email placeholder from error page
  use meta charset in layouts without legacy http-equiv

Conflicts:
	src/Symfony/Bundle/TwigBundle/Loader/FilesystemLoader.php
	src/Symfony/Bundle/TwigBundle/Resources/views/layout.html.twig
fabpot added a commit that referenced this issue Oct 24, 2014
* 2.5:
  enforce memcached version to be 2.1.0
  [PropertyAccess] Simplified code
  [FrameworkBundle] improve server:run feedback
  [Form] no need to add the url listener when it does not do anything
  [Form] Fix #11694 - Enforce options value type check in some form types
  Lithuanian security translations
  [SecurityBundle] Add trust_resolver variable into expression | Q             | A | ------------- | --- | Bug fix?      | [yes] | New feature?  | [no] | BC breaks?    | [no] | Deprecations? | [no] | Tests pass?   | [yes] | Fixed tickets | [#12224] | License       | MIT | Doc PR        | [-]
  [Router] Cleanup
  Fixed UPGRADE-3.0.md markup
  [FrameworkBundle] Fixed ide links
  Add missing argument
  [TwigBundle] do not pass a template reference to twig
  [TwigBundle] show correct fallback exception template in debug mode
  [TwigBundle] remove unused email placeholder from error page
  use meta charset in layouts without legacy http-equiv

Conflicts:
	src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants