[Form] Removed extra CSRF field on collection prototype #3996
Conversation
|
Wait a minute please, I oversaw some tests that have to be adapted. |
| { | ||
| if ($form->isRoot() && $form->hasChildren() && $form->hasAttribute('csrf_field_name')) { | ||
| if (!$view->hasParent() && $view->hasChildren() && $form->hasAttribute('csrf_field_name')) { |
There was a problem hiding this comment.
@bschussek I came to about the same conclusion: !$view->hasParent() && $form->hasChildren() && $form->hasAttribute('csrf_field_name'). I thought the children of the view might not be there yet ?
There was a problem hiding this comment.
That's why I changed it to buildViewBottomUp, where the children are already there.
There was a problem hiding this comment.
Do we need to check the children at all ? (what if the root form is an empty collection)
There was a problem hiding this comment.
Yes, otherwise a simple input field cannot be submitted. If an empty collection form has a CSRF field, it doesn't hurt anyone.
There was a problem hiding this comment.
empty collection -> hasChildren() === false -> no CSRF ?
There was a problem hiding this comment.
Ah, got me there :) An empty collection (or form in general for that sake) doesn't transmit data, so there's nothing to protect, right?
There was a problem hiding this comment.
- couldn't this be used to erase a collection (not protected) ?
- what if children are added dynamically ?
There was a problem hiding this comment.
I have created a new issue as this PR is closed
|
Fixed. ping @fabpot |
Commits ------- ccd6bbc [Form] Removed extra CSRF field on collection prototype Discussion ---------- [Form] Removed extra CSRF field on collection prototype Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #3987, #3990 Todo: -  --------------------------------------------------------------------------- by bschussek at 2012-04-19T08:43:32Z Wait a minute please, I oversaw some tests that have to be adapted. --------------------------------------------------------------------------- by bschussek at 2012-04-19T09:22:45Z Fixed. ping @fabpot
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3987, #3990
Todo: -