-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
ConstraintValidator allows extra fields if all children are optional #33986
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
Comments
Another workaround is to add an additional field to the constraint and make it check on NULL. Ex: $toValidate['testestest'] = null;
$constraints = new Assert\Collection([
'testestest' => new Assert\IsNull(),
'firstName' => new Assert\Optional(),
'lastName' => new Assert\Optional(),
'username' => new Assert\Optional(),
]); But this is a very hackish solution. |
this looks like an issue related to the handling of the implicit group for this case. Putting an explicit |
It looks like the merging of groups for the Composite constraint does not work well when all nested constraints are composite ones (and |
@stof You are correct, if I explicitly state groups and fields on the Collection then it does work aswell. $constraints = new Assert\Collection([
'groups' => 'Default',
'fields' => [
'firstName' => new Assert\Optional(),
'lastName' => new Assert\Optional(),
'username' => new Assert\Optional(),
]
]); Is this intended behavior? |
and this also works: $toValidate = ['a' => 'b'];
$constraints = new Assert\Collection([
'firstName' => new Assert\Optional([new Assert\Type('string')]),
'lastName' => new Assert\Optional(),
'username' => new Assert\Optional(),
]);
$validator = \Symfony\Component\Validator\Validation::createValidator();
$violations = $validator->validate($toValidate, array($constraints));
dump($violations); so this is indeed about composite constraint with no nested constraints (or with only such broken composite constraints as nested constraints) |
I could not reproduce the issue. Has it been fixed in the meantime? Does it happen with the form integration? @stof @jannes-io could you please confirm this is still an issue in latest releases? |
@HeahDude I just tested on 4.3 and 4.4 and if all fields are optional it does not return the "UnexpectedField" violation, we've fixed it by always supplying an
This is the exact
|
@jannes-io thank you for the quick feedback! I've been able to add some tests this time at the root of the issue. In fact this also fails with required keys: $constraints = new Assert\Collection([
'fields' => [
'firstName' => new Assert\Required(),
],
]); as long as we don't define explicitly the See #36365 for a fix. Could you confirm it works for you? Thanks! |
Hi @HeahDude , Thanks for the quick fix! Unfortunately I don't have a "symfony playground" set up at the moment to juggle between versions without bricking the project but from reading the tests in the commit it does align with the behavior that we expect from Good for me 👍 Just wondering, when it is merged in 3.4, will that cascade to the next releases of 4.3/4.4 and 5.0 too? Additionally I assume that this will require a minor version bump, seeing that this could be a breaking change for some edge cases? |
Yes, we merge fixes in the lowest maintained branch possible (currently 3.4, 4.4 or 5.0), then merge branches up to master. |
…raints (HeahDude) This PR was merged into the 3.4 branch. Discussion ---------- [Validator] Fixed default group for nested composite constraints | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #33986 | License | MIT | Doc PR | ~ Take a breath: when composite constraints are nested in a parent composite constraint without having non composite nested constraints (i.e empty), then the default group is not added, making the validator failing to validate in any group (including default), because there is no group at all, which should never happen. Commits ------- 117ee34 [Validator] Fixed default group for nested composite constraints
Symfony version(s) affected: 4.3.3
Description
When running a validator on a collection that only has optional fields it does not give violations on extra fields. It does work however if I make one of the fields required.
How to reproduce
If I change firstName to
new Assert\Positive()
as example, then it does work.The text was updated successfully, but these errors were encountered: