Skip to content

[Form] Fix/csrf token added to prototypes, fixes #3987. #3990

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

Conversation

pjedrzejewski
Copy link

Bug fix: yes.
Feature addition: no.
Backwards compatibility break: no.
Symfony2 tests pass: no, but it's not related in any way to forms.
Fixes the following tickets: #3987.

After recent refactoring of csrf protection, the token was added to all prototypes.

@fabpot
Copy link
Member

fabpot commented Apr 19, 2012

ping @bschussek

@vicb
Copy link
Contributor

vicb commented Apr 19, 2012

What about

<?php
// CollectionType.php
        if ($options['allow_add'] && $options['prototype']) {
            $prototype = $builder->create($options['prototype_name'], $options['type'], array_replace(
                array('label' => $options['prototype_name'] . 'label__'),
                $options['options'],
                array('csrf_protection' => false)
            ));
            $builder->setAttribute('prototype', $prototype->getForm());
        }

@pjedrzejewski
Copy link
Author

Yes, that was first what I thought about, but the form extensions should be decoupled I think, Core doesn't know about Csrf.
If someone doesn't use the Csrf extension, this could trigger an exception about non existing option, not checked that though.

@vicb
Copy link
Contributor

vicb commented Apr 19, 2012

It could be moved to the default options of a collection type extension. Not sure what is the best way to fix this.

@pjedrzejewski
Copy link
Author

This is basically the implementation that was used before csrf refactoring, it is a bit tricky, because we're adding it and then removing. I think we could somehow identify whether the field is prototype or not, but current approach is more universal.
It will work also with dynamically added forms on form data events.

@webmozart
Copy link
Contributor

I fixed this in a separate PR. The fix was a bit different.

@webmozart webmozart closed this Apr 19, 2012
fabpot added a commit that referenced this pull request Apr 19, 2012
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: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3990)

---------------------------------------------------------------------------

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants