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

Do not validate child constraints if form has no validation groups #30595

Merged
merged 1 commit into from Mar 20, 2019
Merged

Do not validate child constraints if form has no validation groups #30595

merged 1 commit into from Mar 20, 2019

Conversation

maryo
Copy link
Contributor

@maryo maryo commented Mar 18, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

If a form has Valid constraint and validation_groups set to an empty array (to disable validation) then its children were still validated using default validation group because FormValidator validated the form data using the empty array validation group here
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L76

and then RecursiveContextualValidator treats the empty array as default validation group here.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php#L86

@maryo maryo changed the base branch from master to 3.4 March 18, 2019 20:45
@maryo maryo changed the title Empty validation groups Callable returning no validation groups Mar 18, 2019
@maryo maryo changed the title Callable returning no validation groups Do not validate child constraints if form has no validation groups Mar 19, 2019
@maryo
Copy link
Contributor Author

maryo commented Mar 19, 2019

I've updated it because it has nothing to do with validation_groups being set as a callback, just a simple empty array is enough to reproduce the bug.

@HeahDude
Copy link
Contributor

HeahDude commented Mar 19, 2019

Hello @maryo, thank you for opening this issue.

It seems to be a doc issue to me, to disable validation you should define the validation_groups option to false instead.

@maryo
Copy link
Contributor Author

maryo commented Mar 19, 2019

@HeahDude Yeah. But according to this code
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Type/BaseValidatorExtension.php#L35
false is just an alias for an empty array but not in case validation_groups is a callback. Then it is not normalized. Then it just returns the callable and it is then then resolved inside FormValidator here
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L183

If the callback returns false, then it actually validates it against group false. That's not such a problem but still a bug IMO.

And all the tests inside FormValidator also expect an empty array to disable validation. That's probably also because the normalization from false to [] inside BaseValidatorExtension is not triggered by calling FormValidator->validate(/*...*/) manually.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Mar 19, 2019
Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes when used with both the constraints option. Looks good. Thanks!

@fabpot
Copy link
Member

fabpot commented Mar 20, 2019

Thank you @maryo.

@fabpot fabpot merged commit f45f0d0 into symfony:3.4 Mar 20, 2019
fabpot added a commit that referenced this pull request Mar 20, 2019
…n groups (maryo)

This PR was merged into the 3.4 branch.

Discussion
----------

Do not validate child constraints if form has no validation groups

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

If a form has `Valid` constraint and `validation_groups` set to an empty array (to disable validation) then its children were still validated using default validation group because `FormValidator` validated the form data using the empty array validation group here
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L76

and then `RecursiveContextualValidator` treats the empty array as default validation group here.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php#L86

Commits
-------

f45f0d0 [Form] Preventing validation of children if parent with Valid constraint has no validation groups
This was referenced Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants