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

InvalidPropertyPathException: Could not parse property path #37027

Closed
flack opened this issue May 31, 2020 · 16 comments
Closed

InvalidPropertyPathException: Could not parse property path #37027

flack opened this issue May 31, 2020 · 16 comments

Comments

@flack
Copy link
Contributor

flack commented May 31, 2020

Symfony version(s) affected: 4.4.9, 5.0.9, 5.1.0

Description

Since the last point release (on 4.4, 5.0 and 5.1 branches), I get the following exception when calling submit on a Symfony Form where the validation for some field in a subform failed:

Symfony\Component\PropertyAccess\Exception\InvalidPropertyPathException: Could not parse property path "children[[salutation]].data". Unexpected token "]" at position 21.

vendor/symfony/property-access/PropertyPath.php:111
vendor/symfony/form/Extension/Validator/ViolationMapper/ViolationPath.php:55
vendor/symfony/form/Extension/Validator/ViolationMapper/ViolationMapper.php:52
vendor/symfony/form/Extension/Validator/EventListener/ValidationListener.php:55
vendor/symfony/event-dispatcher/EventDispatcher.php:264
vendor/symfony/event-dispatcher/EventDispatcher.php:239
vendor/symfony/event-dispatcher/EventDispatcher.php:73
vendor/symfony/event-dispatcher/ImmutableEventDispatcher.php:44
vendor/symfony/form/Form.php:671

I am not sure if the bug is on my end or in Symfony, all I can say is that it used to work fine up until 4.4.8, in 4.4.9 it broke, so I'm guessing it might be related to #36865 ?

How to reproduce
It's kind of a custom setup, so making a reproducable sample could take a while. But like I said, I'm not even sure if it's a bug on my end or in Symfony, so maybe someone has an idea off the top of their heads?

@matthieu-viel
Copy link

Similar issue for me:
Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException: PropertyAccessor requires a graph of objects or arrays to operate on, but it found type "NULL" while trying to traverse path "parent.all[myfield].data" at property "all". in .../vendor/symfony/property-access/PropertyAccessor.php:327
Working fine after downgrade 4.4.9 to 4.4.7

@silverbackdan
Copy link

silverbackdan commented Jun 1, 2020

Hi, this has become an issue for me as well.

I'll give you my reproduction as it is failing in a test.

NestedType.php

<?php

/*
 * This file is part of the Silverback API Components Bundle Project
 *
 * (c) Daniel West <daniel@silverback.is>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

declare(strict_types=1);

namespace Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Form;

use Doctrine\Common\Collections\ArrayCollection;
use Silverback\ApiComponentsBundle\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\CollectionType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Validator\Constraints\All;
use Symfony\Component\Validator\Constraints\Count;
use Symfony\Component\Validator\Constraints\Length;

class NestedType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults(
            [
                'csrf_protection' => false,
                'attr' => [
                    'novalidate' => 'novalidate',
                ],
                'post_app_proxy' => '/proxy',
            ]
        );
    }

    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add(
                'children',
                CollectionType::class,
                [
                    'entry_type' => ChildType::class,
                    'label' => 'Children',
                    'allow_add' => true,
                    'allow_delete' => true,
                    'by_reference' => false,
                    'required' => true,
                    'error_bubbling' => false,
                    'empty_data' => new ArrayCollection([new ChildType()]),
                    'constraints' => [
                        new Count(
                            [
                                'min' => 1,
                                'minMessage' => 'At least one child is required with a name',
                            ]
                        ),
                    ],
                ]
            )
            ->add(
                'text_children',
                CollectionType::class,
                [
                    'entry_type' => TextType::class,
                    'label' => 'Text Children',
                    'allow_add' => true,
                    'allow_delete' => true,
                    'by_reference' => false,
                    'required' => false,
                    'error_bubbling' => false,
                    'empty_data' => [],
                    'constraints' => [
                        new All(
                            [
                                new Length(
                                    [
                                        'min' => 2,
                                        'minMessage' => 'Must be at least 2 characters',
                                    ]
                                ),
                            ]
                        ),
                    ],
                ]
            );
    }
}

ChildType.php

<?php

/*
 * This file is part of the Silverback API Components Bundle Project
 *
 * (c) Daniel West <daniel@silverback.is>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

declare(strict_types=1);

namespace Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Form;

use Silverback\ApiComponentsBundle\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Validator\Constraints\NotBlank;

class ChildType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add(
                'name',
                TextType::class,
                [
                    'constraints' => [
                        new NotBlank(
                            [
                                'message' => 'Please provide your name',
                            ]
                        ),
                    ],
                ]
            );
    }
}

Path failed to parse: children[children].children[[1]].children[name].data

$formData:

array:1 [
  "children" => array:2 [
    0 => []
    1 => array:1 [
      "name" => ""
    ]
  ]
]

Triggered by:

$formData = $this->getRootData($symfonyForm, $data);

There are some additional unrelated parts in this code, such as the second CollectionType in the root form type. Nor does it matter if the validation fails on just 1 child and that is the first defined in the $formData.

I hope this is clear enough. Sorry I'm rushing writing this a little so please come back to me if something isn't clear or I can do adjustments in my test to double check things quickly for you.

The violation message that is being triggered (correctly) is Please provide your name from the ChildType class

This looks to have been a change made in the most recent patch of version 5.0 as well as forcing the 5.0.9 release also fails. 5.0.8 form component is working with 5.1 on other components.

As you'd expect the double square brackets in the property path is the issue and should be children[children].children[1].children[name].data in my instance.

@silverbackdan
Copy link

silverbackdan commented Jun 1, 2020

$fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s';
This check is performed twice on different lines.
https://github.com/xabbuh/symfony/blob/b819d94d1413e426237f35e6b6eddbc380dbccf9/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L75
https://github.com/xabbuh/symfony/blob/b819d94d1413e426237f35e6b6eddbc380dbccf9/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L103

In my case, the object is a doctrine ArrayCollection which is why I think the $fieldPropertyPath property is including the square brackets when it shouldn't...

Or $field->getPropertyPath() should return the path without additional square brackets.

@flack
Copy link
Contributor Author

flack commented Jun 1, 2020

to confirm @silverbackdan's theory, I change https://github.com/xabbuh/symfony/blob/b819d94d1413e426237f35e6b6eddbc380dbccf9/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L103 to look like this:

$fieldPropertyPath = \is_object($data) && !$data instanceof \Iterator ? 'children[%s]' : 'children%s';

That seems to fix the problem for me. It's probably not the right way to fix this, but at least it looks like that line is somehow problematic

lchrusciel added a commit to Sylius/Sylius that referenced this issue Jun 2, 2020
…/form (GSadee)

This PR was merged into the 1.7 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.7
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | associated with symfony/symfony#37027
| License         | MIT


Commits
-------

a38c470 [Maintenance] Add conflict to the 4.4.9 version of symfony/form
@Amunak
Copy link
Contributor

Amunak commented Jun 2, 2020

Can confirm, similar issue here: I have a PriceType inside of a form, and it consists of a NumberType and a ChoiceType. When I put an invalid value for the ChoiceType, I get this exception when I submit the form: Could not parse property path "children[price].children[[currency]]". Unexpected token "]" at position 35.

flack added a commit to flack/openpsa that referenced this issue Jun 2, 2020
@flack
Copy link
Contributor Author

flack commented Jun 2, 2020

@xabbuh I was looking at

https://github.com/xabbuh/symfony/blob/b819d94d1413e426237f35e6b6eddbc380dbccf9/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L103

a bit longer, and I don't understand how that is_object() part can work reliably. If you look at

$parent = $this->parent;
while ($parent && $parent->getConfig()->getInheritData()) {
$parent = $parent->getParent();
}
if ($parent && null === $parent->getConfig()->getDataClass()) {
$this->propertyPath = new PropertyPath('['.$this->name.']');
} else {
$this->propertyPath = new PropertyPath($this->name);
}

$parent isn't necessarily the same as the $form where $data came from. So at least if getInheritData is true for $parent, you could get double brackets I think

@Xen3r0
Copy link

Xen3r0 commented Jun 3, 2020

Hello, i have same problem with Symfony 3.4.41 :

Could not parse property path "children[controles].children[[0]]". Unexpected token "]" at position 32.

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2020

I am currently looking into #37025. The fix for that issue might this one too. But in the meantime it would be nice if anyone of you could create a small example application allowing to reproduce this specific error so that we can then check if both issues will be fixed.

@matthieu-viel
Copy link

in my case it is just a date field validation depending on another one:

$form->add(
                        'startDate',
                        DateType::class,
                        [
                            'widget' => 'single_text',
                            'required' => false
                        ]
                    )
                    ->add(
                        'endDate',
                        DateType::class,
                        [
                            'widget' => 'single_text',
                            'required' => false,
                            'constraints' => [
                                new GreaterThan([
                                    'propertyPath' => 'parent.all[startDate].data',
                                    'message' => 'startDate.greaterThan.endDate'
                                ])
                            ]
                        ]
                    );

@kl3sk
Copy link

kl3sk commented Jun 3, 2020

I can confirm this error on Symfony 5.0.9:

Could not parse property path "children[contractChildren].children[[0]].children[engagementLetterFile].data". Unexpected token "]" at position 39.

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2020

Could you all please check if #37085 does fix this issue by chance too?

@flack
Copy link
Contributor Author

flack commented Jun 3, 2020

@xabbuh I tried it and it does fix the original error. However, I now get duplicate validation error messages:

Screenshot_20200603_154216

in 5.0.8 and earlier this worked as expected (i.e. just one error message, not one for every child)

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2020

I am afraid we will need an example application to be able to reproduce this.

@flack
Copy link
Contributor Author

flack commented Jun 3, 2020

alright, I'll try to put something together later today

@kl3sk
Copy link

kl3sk commented Jun 3, 2020

@xabbuh for me your fix doesn't raise the error anymore.

Everything works again thanks.

Hope for a quick merge, to be able to update my staging app.

@flack
Copy link
Contributor Author

flack commented Jun 3, 2020

@xabbuh it turns out that the duplicate errors happened because I didn't apply the patch correctly apparently, so all is well in the end, sorry for the noise!

At least I got a reproducer for the original issue out of it:

https://github.com/flack/sf37027

fabpot added a commit that referenced this issue Jun 4, 2020
This PR was merged into the 3.4 branch.

Discussion
----------

[Form] properly cascade validation to child forms

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37025, Fix #37027
| License       | MIT
| Doc PR        |

TODO:

- [x] improve test coverage

Commits
-------

7df5298 properly cascade validation to child forms
@fabpot fabpot closed this as completed Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants