From 649752c947d17b46ecbede055c959a09aeb99693 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 20 Apr 2012 18:58:52 +0200 Subject: [PATCH] [Form] Fixed: CSRF token was not displayed on empty complex forms --- .../EventListener/CsrfValidationListener.php | 2 +- .../Csrf/Type/FormTypeCsrfExtension.php | 2 +- .../Form/Tests/AbstractDivLayoutTest.php | 1 + .../Form/Tests/AbstractTableLayoutTest.php | 3 +- .../Csrf/Type/FormTypeCsrfExtensionTest.php | 71 +++++++++---------- 5 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php index 1c7a1edfaa..7b05185878 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php +++ b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php @@ -63,7 +63,7 @@ class CsrfValidationListener implements EventSubscriberInterface $form = $event->getForm(); $data = $event->getData(); - if ($form->isRoot() && $form->hasChildren()) { + if ($form->isRoot() && !$form->getAttribute('primitive')) { if (!isset($data[$this->fieldName]) || !$this->csrfProvider->isCsrfTokenValid($this->intention, $data[$this->fieldName])) { $form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form')); } diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php index 8145e8e575..9bff7cc128 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php +++ b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php @@ -64,7 +64,7 @@ class FormTypeCsrfExtension extends AbstractTypeExtension */ public function buildViewBottomUp(FormView $view, FormInterface $form) { - if (!$view->hasParent() && $view->hasChildren() && $form->hasAttribute('csrf_field_name')) { + if (!$view->hasParent() && !$form->getAttribute('primitive') && $form->hasAttribute('csrf_field_name')) { $name = $form->getAttribute('csrf_field_name'); $csrfProvider = $form->getAttribute('csrf_provider'); $intention = $form->getAttribute('csrf_intention'); diff --git a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php index 19c9b60bc6..6f0675cba5 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -285,6 +285,7 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/div + [./input[@type="hidden"][@id="name__token"]] [count(./div)=0] ' ); diff --git a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php index d786cdec01..9bf77d9c59 100644 --- a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php @@ -178,7 +178,8 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/table - [count(./tr[./td/input])=0] + [./tr[@style="display: none"][./td[@colspan="2"]/input[@type="hidden"][@id="name__token"]]] + [count(./tr[./td/input])=1] ' ); } diff --git a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php index 0d269d0267..bb03c601b4 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php @@ -56,43 +56,42 @@ class FormTypeCsrfExtensionTest extends TypeTestCase )); } - public function testCsrfProtectionByDefaultIfRootAndChildren() + public function testCsrfProtectionByDefaultIfRootAndNotPrimitive() { $view = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', + 'primitive' => false, )) - ->add($this->factory->createNamedBuilder('form', 'child')) - ->getForm() ->createView(); $this->assertTrue($view->hasChild('csrf')); } - public function testNoCsrfProtectionByDefaultIfChildrenButNotRoot() + public function testNoCsrfProtectionByDefaultIfNotPrimitiveButNotRoot() { $view = $this->factory ->createNamedBuilder('form', 'root') ->add($this->factory ->createNamedBuilder('form', 'form', null, array( 'csrf_field_name' => 'csrf', + 'primitive' => false, )) - ->add($this->factory->createNamedBuilder('form', 'child')) ) ->getForm() - ->get('form') - ->createView(); + ->createView() + ->getChild('form'); $this->assertFalse($view->hasChild('csrf')); } - public function testNoCsrfProtectionByDefaultIfRootButNoChildren() + public function testNoCsrfProtectionByDefaultIfRootButPrimitive() { $view = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', + 'primitive' => true, )) - ->getForm() ->createView(); $this->assertFalse($view->hasChild('csrf')); @@ -101,12 +100,11 @@ class FormTypeCsrfExtensionTest extends TypeTestCase public function testCsrfProtectionCanBeDisabled() { $view = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', 'csrf_protection' => false, + 'primitive' => false, )) - ->add($this->factory->createNamedBuilder('form', 'child')) - ->getForm() ->createView(); $this->assertFalse($view->hasChild('csrf')); @@ -120,13 +118,12 @@ class FormTypeCsrfExtensionTest extends TypeTestCase ->will($this->returnValue('token')); $view = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', 'csrf_provider' => $this->csrfProvider, - 'intention' => '%INTENTION%' + 'intention' => '%INTENTION%', + 'primitive' => false, )) - ->add($this->factory->createNamedBuilder('form', 'child')) - ->getForm() ->createView(); $this->assertEquals('token', $view->getChild('csrf')->get('value')); @@ -143,7 +140,7 @@ class FormTypeCsrfExtensionTest extends TypeTestCase /** * @dataProvider provideBoolean */ - public function testValidateTokenOnBindIfRootAndChildren($valid) + public function testValidateTokenOnBindIfRootAndNotPrimitive($valid) { $this->csrfProvider->expects($this->once()) ->method('isCsrfTokenValid') @@ -151,13 +148,12 @@ class FormTypeCsrfExtensionTest extends TypeTestCase ->will($this->returnValue($valid)); $form = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', 'csrf_provider' => $this->csrfProvider, - 'intention' => '%INTENTION%' - )) - ->add($this->factory->createNamedBuilder('form', 'child')) - ->getForm(); + 'intention' => '%INTENTION%', + 'primitive' => false, + )); $form->bind(array( 'child' => 'foobar', @@ -171,19 +167,18 @@ class FormTypeCsrfExtensionTest extends TypeTestCase $this->assertSame($valid, $form->isValid()); } - public function testFailIfRootAndChildrenAndTokenMissing() + public function testFailIfRootAndNotPrimitiveAndTokenMissing() { $this->csrfProvider->expects($this->never()) ->method('isCsrfTokenValid'); $form = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', 'csrf_provider' => $this->csrfProvider, - 'intention' => '%INTENTION%' - )) - ->add($this->factory->createNamedBuilder('form', 'child')) - ->getForm(); + 'intention' => '%INTENTION%', + 'primitive' => false, + )); $form->bind(array( 'child' => 'foobar', @@ -197,7 +192,7 @@ class FormTypeCsrfExtensionTest extends TypeTestCase $this->assertFalse($form->isValid()); } - public function testDontValidateTokenIfChildrenButNoRoot() + public function testDontValidateTokenIfNotPrimitiveButNoRoot() { $this->csrfProvider->expects($this->never()) ->method('isCsrfTokenValid'); @@ -208,9 +203,9 @@ class FormTypeCsrfExtensionTest extends TypeTestCase ->createNamedBuilder('form', 'form', null, array( 'csrf_field_name' => 'csrf', 'csrf_provider' => $this->csrfProvider, - 'intention' => '%INTENTION%' + 'intention' => '%INTENTION%', + 'primitive' => false, )) - ->add($this->factory->createNamedBuilder('form', 'child')) ) ->getForm() ->get('form'); @@ -221,18 +216,18 @@ class FormTypeCsrfExtensionTest extends TypeTestCase )); } - public function testDontValidateTokenIfRootButNoChildren() + public function testDontValidateTokenIfRootButPrimitive() { $this->csrfProvider->expects($this->never()) ->method('isCsrfTokenValid'); $form = $this->factory - ->createBuilder('form', null, array( + ->create('form', null, array( 'csrf_field_name' => 'csrf', 'csrf_provider' => $this->csrfProvider, - 'intention' => '%INTENTION%' - )) - ->getForm(); + 'intention' => '%INTENTION%', + 'primitive' => true, + )); $form->bind(array( 'csrf' => 'token',