Skip to content

Commit cde34fd

Browse files
webmozartwebmozart
committed
[Form] Throwing an AlreadyBoundException in add, remove, setParent, bind and setData if called on a bound form
1 parent 92cb685 commit cde34fd

File tree

6 files changed

+150
-12
lines changed

6 files changed

+150
-12
lines changed

‎CHANGELOG-2.1.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
210210
* added constant Guess::VERY_HIGH_CONFIDENCE
211211
* FormType::getDefaultOptions() now sees default options defined by parent types
212212
* [BC BREAK] FormType::getParent() does not see default options anymore
213+
* [BC BREAK] The methods `add`, `remove`, `setParent`, `bind` and `setData`
214+
in class Form now throw an exception if the form is already bound
213215

214216
### HttpFoundation
215217

‎UPGRADE-2.1.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,3 +248,11 @@ UPGRADE FROM 2.0 to 2.1
248248
{
249249
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'text' : 'choice';
250250
}
251+
252+
* The methods `add`, `remove`, `setParent`, `bind` and `setData` in class Form
253+
now throw an exception if the form is already bound
254+
255+
If you used these methods on bound forms, you should consider moving your
256+
logic to an event listener listening to either of the events
257+
FormEvents::PRE_BIND, FormEvents::BIND_CLIENT_DATA or
258+
FormEvents::BIND_NORM_DATA instead.

‎src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Form\Extension\Csrf\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\Form\FormEvents;
16+
use Symfony\Component\Form\FormError;
17+
use Symfony\Component\Form\Event\DataEvent;
18+
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
19+
20+
/**
21+
* @author Bernhard Schussek <bschussek@gmail.com>
22+
*/
23+
class CsrfValidationListener implements EventSubscriberInterface
24+
{
25+
/**
26+
* The provider for generating and validating CSRF tokens
27+
* @var CsrfProviderInterface
28+
*/
29+
private $csrfProvider;
30+
31+
/**
32+
* A text mentioning the intention of the CSRF token
33+
*
34+
* Validation of the token will only succeed if it was generated in the
35+
* same session and with the same intention.
36+
*
37+
* @var string
38+
*/
39+
private $intention;
40+
41+
static public function getSubscribedEvents()
42+
{
43+
return array(
44+
FormEvents::BIND_CLIENT_DATA => 'onBindClientData',
45+
);
46+
}
47+
48+
public function __construct(CsrfProviderInterface $csrfProvider, $intention)
49+
{
50+
$this->csrfProvider = $csrfProvider;
51+
$this->intention = $intention;
52+
}
53+
54+
public function onBindClientData(DataEvent $event)
55+
{
56+
$form = $event->getForm();
57+
$data = $event->getData();
58+
59+
if ((!$form->hasParent() || $form->getParent()->isRoot())
60+
&& !$this->csrfProvider->isCsrfTokenValid($this->intention, $data)) {
61+
$form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form'));
62+
63+
// If the session timed out, the token is invalid now.
64+
// Regenerate the token so that a resubmission is possible.
65+
$event->setData($this->csrfProvider->generateCsrfToken($this->intention));
66+
}
67+
}
68+
}

‎src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111

1212
namespace Symfony\Component\Form\Extension\Csrf\Type;
1313

14+
1415
use Symfony\Component\Form\AbstractType;
1516
use Symfony\Component\Form\FormInterface;
1617
use Symfony\Component\Form\FormBuilder;
1718
use Symfony\Component\Form\FormError;
19+
use Symfony\Component\Form\Extension\Csrf\EventListener\CsrfValidationListener;
1820
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
1921
use Symfony\Component\Form\CallbackValidator;
2022

@@ -46,18 +48,9 @@ public function buildForm(FormBuilder $builder, array $options)
4648
$csrfProvider = $options['csrf_provider'];
4749
$intention = $options['intention'];
4850

49-
$validator = function (FormInterface $form) use ($csrfProvider, $intention)
50-
{
51-
if ((!$form->hasParent() || $form->getParent()->isRoot())
52-
&& !$csrfProvider->isCsrfTokenValid($intention, $form->getData())) {
53-
$form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form'));
54-
$form->setData($csrfProvider->generateCsrfToken($intention));
55-
}
56-
};
57-
5851
$builder
5952
->setData($csrfProvider->generateCsrfToken($intention))
60-
->addValidator(new CallbackValidator($validator))
53+
->addEventSubscriber(new CsrfValidationListener($csrfProvider, $intention))
6154
;
6255
}
6356

‎src/Symfony/Component/Form/Form.php

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Form\Event\DataEvent;
1515
use Symfony\Component\Form\Event\FilterDataEvent;
1616
use Symfony\Component\Form\Exception\FormException;
17+
use Symfony\Component\Form\Exception\AlreadyBoundException;
1718
use Symfony\Component\Form\Exception\UnexpectedTypeException;
1819
use Symfony\Component\Form\Exception\TransformationFailedException;
1920
use Symfony\Component\HttpFoundation\Request;
@@ -297,6 +298,10 @@ public function isDisabled()
297298
*/
298299
public function setParent(FormInterface $parent = null)
299300
{
301+
if ($this->bound) {
302+
throw new AlreadyBoundException('You cannot set the parent of a bound form');
303+
}
304+
300305
if ('' === $this->getName()) {
301306
throw new FormException('Form with empty name can not have parent form.');
302307
}
@@ -377,6 +382,10 @@ public function getAttribute($name)
377382
*/
378383
public function setData($appData)
379384
{
385+
if ($this->bound) {
386+
throw new AlreadyBoundException('You cannot change the data of a bound form');
387+
}
388+
380389
$event = new DataEvent($this, $appData);
381390
$this->dispatcher->dispatch(FormEvents::PRE_SET_DATA, $event);
382391

@@ -451,6 +460,10 @@ public function getExtraData()
451460
*/
452461
public function bind($clientData)
453462
{
463+
if ($this->bound) {
464+
throw new AlreadyBoundException('A form can only be bound once');
465+
}
466+
454467
if ($this->isDisabled()) {
455468
$this->bound = true;
456469

@@ -689,7 +702,7 @@ public function isEmpty()
689702
*/
690703
public function isValid()
691704
{
692-
if (!$this->isBound()) {
705+
if (!$this->bound) {
693706
throw new \LogicException('You cannot call isValid() on a form that is not bound.');
694707
}
695708

@@ -821,6 +834,10 @@ public function hasChildren()
821834
*/
822835
public function add(FormInterface $child)
823836
{
837+
if ($this->bound) {
838+
throw new AlreadyBoundException('You cannot add children to a bound form');
839+
}
840+
824841
$this->children[$child->getName()] = $child;
825842

826843
$child->setParent($this);
@@ -841,6 +858,10 @@ public function add(FormInterface $child)
841858
*/
842859
public function remove($name)
843860
{
861+
if ($this->bound) {
862+
throw new AlreadyBoundException('You cannot remove children from a bound form');
863+
}
864+
844865
if (isset($this->children[$name])) {
845866
$this->children[$name]->setParent(null);
846867

‎tests/Symfony/Tests/Component/Form/FormTest.php

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ public function testBind()
199199
$this->assertEquals(array('firstName' => 'Bernhard'), $this->form->getData());
200200
}
201201

202+
/**
203+
* @expectedException Symfony\Component\Form\Exception\AlreadyBoundException
204+
*/
205+
public function testBindThrowsExceptionIfAlreadyBound()
206+
{
207+
$this->form->bind(array());
208+
$this->form->bind(array());
209+
}
210+
202211
public function testBindForwardsNullIfValueIsMissing()
203212
{
204213
$child = $this->getMockForm('firstName');
@@ -408,8 +417,8 @@ public function testNotValidIfChildNotValid()
408417
->method('isValid')
409418
->will($this->returnValue(false));
410419

411-
$this->form->bind('foobar');
412420
$this->form->add($child);
421+
$this->form->bind(array());
413422

414423
$this->assertFalse($this->form->isValid());
415424
}
@@ -438,6 +447,15 @@ public function testHasNoChildren()
438447
$this->assertFalse($this->form->hasChildren());
439448
}
440449

450+
/**
451+
* @expectedException Symfony\Component\Form\Exception\AlreadyBoundException
452+
*/
453+
public function testSetParentThrowsExceptionIfAlreadyBound()
454+
{
455+
$this->form->bind(array());
456+
$this->form->setParent($this->getBuilder('parent')->getForm());
457+
}
458+
441459
public function testAdd()
442460
{
443461
$child = $this->getBuilder('foo')->getForm();
@@ -447,6 +465,15 @@ public function testAdd()
447465
$this->assertSame(array('foo' => $child), $this->form->getChildren());
448466
}
449467

468+
/**
469+
* @expectedException Symfony\Component\Form\Exception\AlreadyBoundException
470+
*/
471+
public function testAddThrowsExceptionIfAlreadyBound()
472+
{
473+
$this->form->bind(array());
474+
$this->form->add($this->getBuilder('foo')->getForm());
475+
}
476+
450477
public function testRemove()
451478
{
452479
$child = $this->getBuilder('foo')->getForm();
@@ -457,6 +484,16 @@ public function testRemove()
457484
$this->assertFalse($this->form->hasChildren());
458485
}
459486

487+
/**
488+
* @expectedException Symfony\Component\Form\Exception\AlreadyBoundException
489+
*/
490+
public function testRemoveThrowsExceptionIfAlreadyBound()
491+
{
492+
$this->form->add($this->getBuilder('foo')->getForm());
493+
$this->form->bind(array('foo' => 'bar'));
494+
$this->form->remove('foo');
495+
}
496+
460497
public function testRemoveIgnoresUnknownName()
461498
{
462499
$this->form->remove('notexisting');
@@ -504,6 +541,15 @@ public function testNotBound()
504541
$this->assertFalse($this->form->isBound());
505542
}
506543

544+
/**
545+
* @expectedException Symfony\Component\Form\Exception\AlreadyBoundException
546+
*/
547+
public function testSetDataThrowsExceptionIfAlreadyBound()
548+
{
549+
$this->form->bind(array());
550+
$this->form->setData(null);
551+
}
552+
507553
public function testSetDataExecutesTransformationChain()
508554
{
509555
// use real event dispatcher now

0 commit comments

Comments
 (0)