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

[Validator] Fix ValidValidator group cascading usage #33834

Merged
merged 1 commit into from Oct 7, 2019

Conversation

@fancyweb
Copy link
Contributor

commented Oct 3, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

$this->context->getGroup() returns string or null.

ContextualValidatorInterface::validate() 3rd argument accepts an array but it must not contain null (its contract doesn't allow it). If it does, it will fail in RecursiveContextualValidator::validateInGroup() for example because of the string scalar type on the $group argument. (on 4.4)

Note that in our "common" usage of the Valid constraint, the group in its validator will never be null because this constraint has a special treatment. However, if this validator is reused in a custom way, it can fail.

@xabbuh
xabbuh approved these changes Oct 7, 2019
@xabbuh

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

Thank you @fancyweb.

xabbuh added a commit that referenced this pull request Oct 7, 2019
…yweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Fix ValidValidator group cascading usage

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

`$this->context->getGroup()` returns `string` or `null`.

`ContextualValidatorInterface::validate()` 3rd argument accepts an array but it must not contain `null` (its contract doesn't allow it). If it does, it will fail in `RecursiveContextualValidator::validateInGroup()` for example because of the `string` scalar type on the `$group` argument. (on 4.4)

Note that in our "common" usage of the `Valid` constraint, the group in its validator will never be `null` because this constraint has a special treatment. However, if this validator is reused in a custom way, it can fail.

Commits
-------

72684b0 [Validator] Fix ValidValidator group cascading usage
@xabbuh xabbuh merged commit 72684b0 into symfony:3.4 Oct 7, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@fancyweb fancyweb deleted the fancyweb:validator-valid-validator-group branch Oct 7, 2019
This was referenced Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.