From ccd6bbc0a1124960b9cf3f3d26e663d063895859 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 19 Apr 2012 10:37:16 +0200 Subject: [PATCH] [Form] Removed extra CSRF field on collection prototype --- .../Csrf/Type/FormTypeCsrfExtension.php | 4 +- .../Form/Tests/AbstractDivLayoutTest.php | 64 ++++++++-------- .../Form/Tests/AbstractLayoutTest.php | 16 ++-- .../Form/Tests/AbstractTableLayoutTest.php | 74 +++++++++---------- .../Csrf/Type/FormTypeCsrfExtensionTest.php | 35 +++++++++ 5 files changed, 114 insertions(+), 79 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php index c8687d4af4748..3870a072db2c5 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php +++ b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php @@ -63,9 +63,9 @@ public function buildForm(FormBuilder $builder, array $options) * @param FormView $view The form view * @param FormInterface $form The form */ - public function buildView(FormView $view, FormInterface $form) + public function buildViewBottomUp(FormView $view, FormInterface $form) { - if ($form->isRoot() && $form->hasChildren() && $form->hasAttribute('csrf_field_name')) { + if (!$view->hasParent() && $view->hasChildren() && $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 7d1512f0037bd..07813061886ba 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -96,10 +96,7 @@ public function testRest() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/input - [@type="hidden"] - [@id="name__token"] -/following-sibling::div +'/div [ ./label[@for="name_field1"] /following-sibling::input[@type="text"][@id="name_field1"] @@ -112,6 +109,9 @@ public function testRest() [count(../div)=2] [count(..//label)=2] [count(..//input)=3] +/following-sibling::input + [@type="hidden"] + [@id="name__token"] ' ); } @@ -144,8 +144,7 @@ public function testRestWithChildrenForms() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/input[@type="hidden"][@id="parent__token"] -/following-sibling::div +'/div [ ./label[not(@for)] /following-sibling::div[@id="parent_child1"] @@ -172,6 +171,7 @@ public function testRestWithChildrenForms() ] [count(//label)=4] [count(//input[@type="text"])=2] +/following-sibling::input[@type="hidden"][@id="parent__token"] ' ); } @@ -189,15 +189,15 @@ public function testRestAndRepeatedWithRow() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/input - [@type="hidden"] - [@id="name__token"] -/following-sibling::div +'/div [ ./label[@for="name_first"] /following-sibling::input[@type="text"][@id="name_first"] ] [count(.//input)=1] +/following-sibling::input + [@type="hidden"] + [@id="name__token"] ' ); } @@ -216,16 +216,16 @@ public function testRestAndRepeatedWithRowPerChild() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/input - [@type="hidden"] - [@id="name__token"] -/following-sibling::div +'/div [ ./label[@for="name_first"] /following-sibling::input[@type="text"][@id="name_first"] ] [count(.//input)=1] [count(.//label)=1] +/following-sibling::input + [@type="hidden"] + [@id="name__token"] ' ); } @@ -246,16 +246,16 @@ public function testRestAndRepeatedWithWidgetPerChild() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/input - [@type="hidden"] - [@id="name__token"] -/following-sibling::div +'/div [ ./label[@for="name_first"] /following-sibling::input[@type="text"][@id="name_first"] ] [count(//input)=2] [count(//label)=1] +/following-sibling::input + [@type="hidden"] + [@id="name__token"] ' ); } @@ -293,8 +293,7 @@ public function testCollectionRow() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="form__token"] - /following-sibling::div + ./div [ ./label[not(@for)] /following-sibling::div @@ -311,6 +310,7 @@ public function testCollectionRow() ] ] ] + /following-sibling::input[@type="hidden"][@id="form__token"] ] [count(.//input)=3] ' @@ -327,8 +327,7 @@ public function testForm() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::div + ./div [ ./label[@for="name_firstName"] /following-sibling::input[@type="text"][@id="name_firstName"] @@ -338,6 +337,7 @@ public function testForm() ./label[@for="name_lastName"] /following-sibling::input[@type="text"][@id="name_lastName"] ] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(.//input)=3] ' @@ -383,8 +383,8 @@ public function testCsrf() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"][@value="foo&bar"] - /following-sibling::div + ./div + /following-sibling::input[@type="hidden"][@id="name__token"][@value="foo&bar"] ] [count(.//input[@type="hidden"])=1] ' @@ -400,8 +400,7 @@ public function testRepeated() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::div + ./div [ ./label[@for="name_first"] /following-sibling::input[@type="text"][@id="name_first"] @@ -411,6 +410,7 @@ public function testRepeated() ./label[@for="name_second"] /following-sibling::input[@type="text"][@id="name_second"] ] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(.//input)=3] ' @@ -428,8 +428,7 @@ public function testRepeatedWithCustomOptions() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::div + ./div [ ./label[@for="name_first"][.="[trans]Test[/trans]"] /following-sibling::input[@type="text"][@id="name_first"][@required="required"] @@ -439,6 +438,7 @@ public function testRepeatedWithCustomOptions() ./label[@for="name_second"][.="[trans]Test2[/trans]"] /following-sibling::input[@type="text"][@id="name_second"][@required="required"] ] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(.//input)=3] ' @@ -454,12 +454,12 @@ public function testSearchInputName() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="full__token"] - /following-sibling::div + ./div [ ./label[@for="full_name"] /following-sibling::input[@type="search"][@id="full_name"][@name="full[name]"] ] + /following-sibling::input[@type="hidden"][@id="full__token"] ] [count(//input)=2] ' @@ -521,8 +521,7 @@ public function testThemeInheritance($parentTheme, $childTheme) $this->assertWidgetMatchesXpath($view, array(), '/div [ - ./input[@type="hidden"] - /following-sibling::div + ./div [ ./label[.="parent"] /following-sibling::input[@type="text"] @@ -539,6 +538,7 @@ public function testThemeInheritance($parentTheme, $childTheme) ] ] ] + /following-sibling::input[@type="hidden"] ] ' ); diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 5d48b9921cffb..c75ba7c7a4ff3 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -646,11 +646,11 @@ public function testSingleChoiceExpanded() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@value="&a"][@checked] + ./input[@type="radio"][@name="name"][@id="name_0"][@value="&a"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][@value="&b"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(./input)=3] ' @@ -669,11 +669,11 @@ public function testSingleChoiceExpandedSkipEmptyValue() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@checked] + ./input[@type="radio"][@name="name"][@id="name_0"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(./input)=3] ' @@ -691,11 +691,11 @@ public function testSingleChoiceExpandedWithBooleanValue() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@checked] + ./input[@type="radio"][@name="name"][@id="name_0"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(./input)=3] ' @@ -714,13 +714,13 @@ public function testMultipleChoiceExpanded() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="hidden"][@id="name__token"] - /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_0"][@checked][not(@required)] + ./input[@type="checkbox"][@name="name[]"][@id="name_0"][@checked][not(@required)] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_1"][not(@checked)][not(@required)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_2"][@checked][not(@required)] /following-sibling::label[@for="name_2"][.="[trans]Choice&C[/trans]"] + /following-sibling::input[@type="hidden"][@id="name__token"] ] [count(./input)=4] ' diff --git a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php index 0943b06c4162d..f6edbce25a440 100644 --- a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php @@ -45,12 +45,7 @@ public function testRepeatedRow() $html = $this->renderRow($form->createView()); $this->assertMatchesXpath($html, -'/tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] -/following-sibling::tr +'/tr [ ./td [./label[@for="name_first"]] @@ -65,6 +60,11 @@ public function testRepeatedRow() [./input[@id="name_second"]] ] [count(../tr)=3] +/following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ' ); } @@ -81,11 +81,6 @@ public function testRepeatedRowWithErrors() [./td[@colspan="2"]/ul [./li[.="[trans]Error![/trans]"]] ] -/following-sibling::tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] /following-sibling::tr [ ./td @@ -101,6 +96,11 @@ public function testRepeatedRowWithErrors() [./input[@id="name_second"]] ] [count(../tr)=4] +/following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ' ); } @@ -126,12 +126,7 @@ public function testRest() $html = $this->renderRest($view); $this->assertMatchesXpath($html, -'/tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] -/following-sibling::tr +'/tr [ ./td [./label[@for="name_field1"]] @@ -148,6 +143,11 @@ public function testRest() [count(../tr)=3] [count(..//label)=2] [count(..//input)=3] +/following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ' ); } @@ -161,9 +161,9 @@ public function testCollection() $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr[@style="display: none"][./td[@colspan="2"]/input[@type="hidden"][@id="name__token"]] - /following-sibling::tr[./td/input[@type="text"][@value="a"]] + ./tr[./td/input[@type="text"][@value="a"]] /following-sibling::tr[./td/input[@type="text"][@value="b"]] + /following-sibling::tr[@style="display: none"][./td[@colspan="2"]/input[@type="hidden"][@id="name__token"]] ] [count(./tr[./td/input])=3] ' @@ -181,12 +181,7 @@ public function testForm() $this->assertWidgetMatchesXpath($view, array(), '/table [ - ./tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] - /following-sibling::tr + ./tr [ ./td [./label[@for="name_firstName"]] @@ -200,6 +195,11 @@ public function testForm() /following-sibling::td [./input[@id="name_lastName"]] ] + /following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ] [count(.//input)=3] ' @@ -267,12 +267,7 @@ public function testRepeated() $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] - /following-sibling::tr + ./tr [ ./td [./label[@for="name_first"]] @@ -286,6 +281,11 @@ public function testRepeated() /following-sibling::td [./input[@type="text"][@id="name_second"]] ] + /following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ] [count(.//input)=3] ' @@ -303,12 +303,7 @@ public function testRepeatedWithCustomOptions() $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr[@style="display: none"] - [./td[@colspan="2"]/input - [@type="hidden"] - [@id="name__token"] - ] - /following-sibling::tr + ./tr [ ./td [./label[@for="name_first"][.="[trans]Test[/trans]"]] @@ -322,6 +317,11 @@ public function testRepeatedWithCustomOptions() /following-sibling::td [./input[@type="password"][@id="name_second"][@required="required"]] ] + /following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] ] [count(.//input)=3] ' 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 9024002b5223b..4424f81e14140 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php @@ -11,9 +11,26 @@ namespace Symfony\Component\Form\Tests\Extension\Csrf\Type; +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\FormBuilder; use Symfony\Component\Form\Extension\Csrf\CsrfExtension; use Symfony\Component\Form\Tests\Extension\Core\Type\TypeTestCase; +class FormTypeCsrfExtensionTest_ChildType extends AbstractType +{ + public function buildForm(FormBuilder $builder, array $options) + { + // The form needs a child in order to trigger CSRF protection by + // default + $builder->add('name', 'text'); + } + + public function getName() + { + return 'csrf_collection_test'; + } +} + class FormTypeCsrfExtensionTest extends TypeTestCase { protected $csrfProvider; @@ -195,4 +212,22 @@ public function testDontValidateTokenIfRootButNoChildren() 'csrf' => 'token', )); } + + public function testNoCsrfProtectionOnPrototype() + { + $prototypeView = $this->factory + ->create('collection', null, array( + 'type' => new FormTypeCsrfExtensionTest_ChildType(), + 'options' => array( + 'csrf_field_name' => 'csrf', + ), + 'prototype' => true, + 'allow_add' => true, + )) + ->createView() + ->get('prototype'); + + $this->assertFalse($prototypeView->hasChild('csrf')); + $this->assertCount(1, $prototypeView); + } }