You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When using the $clearMissing option set to false (when submitting a form with $form->submit()), validation constraints on missing fields are ignored.
Since the fields are in the form, I think they should be validated too even if they are missing on submit.
I reported this issue several years ago but it seems to be misunderstand: #9998
The example provided on the original issue is still valid to reproduce the issue.
@Seb33300 You can just create a small example project that allows to reproduce your issue if you struggle to come up with a real test case. We can then look into it and see if there's actually any work to do.
This is a really annoying issue that is most likely the reason of a lot of posts around the net if you search for this problem topic.
If you need more input please let me know. This bug is really clear to me and shouldn't be too hard to reproduce/locate.
This was reported many times in the past, and is not a bug. Changing this behavior is a big BC break.
This could only be done as an opt-in feature.
When patching an object it should not be in an invalide state in the first place, so only changed values are validated.
if a field modifies another one, the workaround is to add a callback to validate them as expected (business rule, Symfony cannot handle this).
I understand the technical difficulties and also to break a previous (bad) behavior. But putting this to side and focus on the expectations of a user on this functionality I can't share the position to call this not a bug.
If I have a form and I add NotNull/NotBlank validators the expectations to get an error in any missing case can't be wrong.
Also from a security point this is sensitive. A HTML-Form that per default sends an empty input field going through all tests. A simple "hack" by removing this empty field from the post would break the validation of this application with unknown consequences.
Hello @simonberger, sorry I don't follow you, especially on the security part.
Let's put things in context :).
What does $clearMissing mean?
true => a missing field will be submitted as null false => a missing field will be ignored
What do we expect from Form::submit()?
To bind incoming data to the initial data.
When can we use $clearMissing as true?
All the time, this is the default behavior. we update the whole initial data with the submitted one.
In such case, it makes sense to validate the whole data.
When as false?
Symfony does it internally to handle PATCH requests, because for this verb, only submitted data should be bound. This verb "should" be used to "update" an existing resource, meaning the initial integrity of this resource is not the responsibility of the framework at this point.
In such case, it makes senses to validate only submitted fields.
From here, I ask you guys. What concrete use case does not fit with that? Then let's try to fix it.
Thank you for your answer @HeahDude
I'll try to describe my problem with the current behavior and what in my opinion leads to the confusion regarding the parameter $clearMissing and the possible resulting misuse through us users.
What the different impact of the parameter values is:
$clearMissing === true
Adds missing request parameters set as null. ("Clear missing" is misleading in this context too)
$clearMissing === false
Ignores validation of required parameters
In my project we wanted to archive the following:
Ignore optional parameter. Those gets set by patching elsewhere -> tried setting $clearMissing to false
Validate required parameters in this creation step -> needs `$clearMissing' been set to false
We can just have one of those above requirements.
So my point is $clearMissing does two jobs in one parameter, the real intended mechanism to support patching can't be read from inline documentation
Note:
This documentation site shows all information and warnings perfectly https://symfony.com/doc/current/form/direct_submit.html
But just having the online documentation while the parameter name has near zero information content often isn't enough.
I know we can't do much here without breaking functionality. But for example the parameter name can be changed which would add clarity. I favor long parameter names for such a complex impact ;)
The conflict could be solved with another parameter but I fear that would be an ugly workaround while the clearMissing parameter has to keep his functionality.
Thanks @simonberger, now that I understand the use cases, I can tell you, indeed, this $clearMissing parameter is not intended to solve them.
Doing such things like "Ignore optional parameter" or "Validate required parameters in this creation step" should involve validation groups.
The validation is decoupled from the forms. One can validate his/her data anytime using its dedicated service, the form is just a kind of "data handler" which delegates that part (too much IMO, but that's another issue and I would like to change that in master in the future), and integrates it "seamlessly".
Hi,
As I understood from this $clearMissing param, if I use:
$form->submit($request->request->get($form->getName()); //default is true
it behaves the same as: $form->handleRequest($request);
which means that missing entity fields that are not submitted in the request, their values are set to null, which is not what I want when I am using an Edit Form, because normally I have many optional fields and some and not even rendered in the form because they are no intended to be modified.
So it seems that they are useful for Creation Forms. Because if I am editing a entity using forms, I should use: $form->submit($request->request->get($form->getName(), false);
to avoid problems.
I'm I right?
The validation part of the entity is other related thing, that can be achieved manually by calling the validator on the entity, or using $form->isValid(). $clearMissing set to false does not fire validation on entity fields that are no submitted, they have to be manually validated, for example, right?
As I understood from this $clearMissing param, if I use:
$form->submit($request->request->get($form->getName()); //default is true
it behaves the same as:
$form->handleRequest($request);
true
which means that missing entity fields that are not submitted in the request, their values are set to null, which is not what I want when I am using an Edit Form, because normally I have many optional fields and some and not even rendered in the form because they are no intended to be modified.
In such case (clear missing is true), when editing, optional fields can mean two things:
the field value can remain empty or the field name does not have to be part of submitted data, and the form will set the property to null
the field is removed from the tree, then it won't be rendered and the form will not try to map null on the underlying property
As I understood from this $clearMissing param, if I use:
$form->submit($request->request->get($form->getName()); //default is true
it behaves the same as:
$form->handleRequest($request);
the request handler also prevents submitting if the post max size is reached, and merges $request->files if needed. All that only when methods configured on the form and read from the request match.
Re-reading the issue I am going to close here. As @HeahDude explained this is the expected behaviour and we are not going to change that. Though, if someone likes to open a pull request to improve the documentation, we will happily review and merge that.
Activity
Simperfit commentedon Dec 6, 2017
Could you please add a test case failling so we can look at it and find a fix ?
Seb33300 commentedon Dec 8, 2017
I am trying to create a test but I am not able to make it working.
In
Symfony\Component\Form\Tests\Extension\Validator\Constraints\FormValidatorTest
I would like to create a form with a single field and submit it a blank value (in order to check if the
NotBlank
constraint is properly working)Then we should have to change the submit by
$form->submit(array(), false);
to reproduce the issue.xabbuh commentedon Dec 14, 2017
@Seb33300 You can just create a small example project that allows to reproduce your issue if you struggle to come up with a real test case. We can then look into it and see if there's actually any work to do.
xabbuh commentedon Dec 22, 2017
@Seb33300 I am closing for now, but please leave a comment when you have a reproducer and we can reopen.
Seb33300 commentedon Dec 22, 2017
The project I created on my original issue is still valid to reproduce the issue: #9998 (comment)
simonberger commentedon Feb 2, 2018
This is a really annoying issue that is most likely the reason of a lot of posts around the net if you search for this problem topic.
If you need more input please let me know. This bug is really clear to me and shouldn't be too hard to reproduce/locate.
1 remaining item
Simperfit commentedon Feb 2, 2018
@HeahDude Could you please look at this one ? it seems to be a really annoying bug.
HeahDude commentedon Feb 2, 2018
This was reported many times in the past, and is not a bug. Changing this behavior is a big BC break.
This could only be done as an opt-in feature.
When patching an object it should not be in an invalide state in the first place, so only changed values are validated.
if a field modifies another one, the workaround is to add a callback to validate them as expected (business rule, Symfony cannot handle this).
simonberger commentedon Feb 2, 2018
I understand the technical difficulties and also to break a previous (bad) behavior. But putting this to side and focus on the expectations of a user on this functionality I can't share the position to call this not a bug.
If I have a form and I add NotNull/NotBlank validators the expectations to get an error in any missing case can't be wrong.
Also from a security point this is sensitive. A HTML-Form that per default sends an empty input field going through all tests. A simple "hack" by removing this empty field from the post would break the validation of this application with unknown consequences.
HeahDude commentedon Feb 2, 2018
Hello @simonberger, sorry I don't follow you, especially on the security part.
Let's put things in context :).
What does
$clearMissing
mean?true
=> a missing field will be submitted asnull
false
=> a missing field will be ignoredWhat do we expect from
Form::submit()
?To bind incoming data to the initial data.
When can we use
$clearMissing
astrue
?All the time, this is the default behavior. we update the whole initial data with the submitted one.
In such case, it makes sense to validate the whole data.
When as
false
?Symfony does it internally to handle PATCH requests, because for this verb, only submitted data should be bound. This verb "should" be used to "update" an existing resource, meaning the initial integrity of this resource is not the responsibility of the framework at this point.
In such case, it makes senses to validate only submitted fields.
From here, I ask you guys. What concrete use case does not fit with that? Then let's try to fix it.
simonberger commentedon Feb 6, 2018
Thank you for your answer @HeahDude
I'll try to describe my problem with the current behavior and what in my opinion leads to the confusion regarding the parameter
$clearMissing
and the possible resulting misuse through us users.What the different impact of the parameter values is:
$clearMissing === true
Adds missing request parameters set as null. ("Clear missing" is misleading in this context too)
$clearMissing === false
Ignores validation of required parameters
In my project we wanted to archive the following:
$clearMissing
to falseWe can just have one of those above requirements.
So my point is
$clearMissing
does two jobs in one parameter, the real intended mechanism to support patching can't be read from inline documentationNote:
This documentation site shows all information and warnings perfectly https://symfony.com/doc/current/form/direct_submit.html
But just having the online documentation while the parameter name has near zero information content often isn't enough.
I know we can't do much here without breaking functionality. But for example the parameter name can be changed which would add clarity. I favor long parameter names for such a complex impact ;)
The conflict could be solved with another parameter but I fear that would be an ugly workaround while the clearMissing parameter has to keep his functionality.
HeahDude commentedon Feb 6, 2018
Thanks @simonberger, now that I understand the use cases, I can tell you, indeed, this
$clearMissing
parameter is not intended to solve them.Doing such things like "Ignore optional parameter" or "Validate required parameters in this creation step" should involve validation groups.
The validation is decoupled from the forms. One can validate his/her data anytime using its dedicated service, the form is just a kind of "data handler" which delegates that part (too much IMO, but that's another issue and I would like to change that in master in the future), and integrates it "seamlessly".
pribeirojtm commentedon Jun 22, 2018
Hi,
As I understood from this
$clearMissing
param, if I use:$form->submit($request->request->get($form->getName()); //default is true
it behaves the same as:
$form->handleRequest($request);
which means that missing entity fields that are not submitted in the request, their values are set to null, which is not what I want when I am using an Edit Form, because normally I have many optional fields and some and not even rendered in the form because they are no intended to be modified.
So it seems that they are useful for Creation Forms. Because if I am editing a entity using forms, I should use:
$form->submit($request->request->get($form->getName(), false);
to avoid problems.
I'm I right?
The validation part of the entity is other related thing, that can be achieved manually by calling the validator on the entity, or using
$form->isValid()
.$clearMissing
set to false does not fire validation on entity fields that are no submitted, they have to be manually validated, for example, right?Thanks.
HeahDude commentedon Jun 22, 2018
true
In such case (clear missing is true), when editing, optional fields can mean two things:
HeahDude commentedon Jun 22, 2018
I may need to precise about:
the request handler also prevents submitting if the post max size is reached, and merges
$request->files
if needed. All that only when methods configured on the form and read from the request match.xabbuh commentedon Sep 23, 2018
Re-reading the issue I am going to close here. As @HeahDude explained this is the expected behaviour and we are not going to change that. Though, if someone likes to open a pull request to improve the documentation, we will happily review and merge that.
Added example for direct form submit with validation
Added example for direct form submit with validation
minor #11905 Added example for direct form submit with validation (pa…