Skip to content
Browse files

merged branch bschussek/issue3990 (PR #3996)

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
  • Loading branch information...
2 parents fd52f93 + ccd6bbc commit aebaece46063e62124e0ce585c07fd3004943c67 @fabpot fabpot committed Apr 19, 2012
View
4 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');
View
64 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"]
]
'
);
View
16 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]
'
View
74 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]
'
View
35 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);
+ }
}

0 comments on commit aebaece

Please sign in to comment.
Something went wrong with that request. Please try again.