diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 02ff0a5301..5468406214 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -210,6 +210,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * added constant Guess::VERY_HIGH_CONFIDENCE * FormType::getDefaultOptions() now sees default options defined by parent types * [BC BREAK] FormType::getParent() does not see default options anymore + * [BC BREAK] The methods `add`, `remove`, `setParent`, `bind` and `setData` + in class Form now throw an exception if the form is already bound ### HttpFoundation diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md index b43f1564cd..4f14ed03da 100644 --- a/UPGRADE-2.1.md +++ b/UPGRADE-2.1.md @@ -291,3 +291,11 @@ UPGRADE FROM 2.0 to 2.1 Any session storage driver that wants to use custom save handlers should implement `Symfony\Component\HttpFoundation\Session\Storage\SaveHandlerInterface` + +* The methods `add`, `remove`, `setParent`, `bind` and `setData` in class Form + now throw an exception if the form is already bound + + If you used these methods on bound forms, you should consider moving your + logic to an event listener listening to either of the events + FormEvents::PRE_BIND, FormEvents::BIND_CLIENT_DATA or + FormEvents::BIND_NORM_DATA instead. diff --git a/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php new file mode 100644 index 0000000000..3de52ab867 --- /dev/null +++ b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php @@ -0,0 +1,68 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Extension\Csrf\EventListener; + +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Form\FormEvents; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\Event\DataEvent; +use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; + +/** + * @author Bernhard Schussek + */ +class CsrfValidationListener implements EventSubscriberInterface +{ + /** + * The provider for generating and validating CSRF tokens + * @var CsrfProviderInterface + */ + private $csrfProvider; + + /** + * A text mentioning the intention of the CSRF token + * + * Validation of the token will only succeed if it was generated in the + * same session and with the same intention. + * + * @var string + */ + private $intention; + + static public function getSubscribedEvents() + { + return array( + FormEvents::BIND_CLIENT_DATA => 'onBindClientData', + ); + } + + public function __construct(CsrfProviderInterface $csrfProvider, $intention) + { + $this->csrfProvider = $csrfProvider; + $this->intention = $intention; + } + + public function onBindClientData(DataEvent $event) + { + $form = $event->getForm(); + $data = $event->getData(); + + if ((!$form->hasParent() || $form->getParent()->isRoot()) + && !$this->csrfProvider->isCsrfTokenValid($this->intention, $data)) { + $form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form')); + + // If the session timed out, the token is invalid now. + // Regenerate the token so that a resubmission is possible. + $event->setData($this->csrfProvider->generateCsrfToken($this->intention)); + } + } +} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php b/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php index 3e7973240f..0484b8feac 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php +++ b/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php @@ -11,10 +11,12 @@ namespace Symfony\Component\Form\Extension\Csrf\Type; + use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormBuilder; use Symfony\Component\Form\FormError; +use Symfony\Component\Form\Extension\Csrf\EventListener\CsrfValidationListener; use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; use Symfony\Component\Form\CallbackValidator; @@ -46,18 +48,9 @@ class CsrfType extends AbstractType $csrfProvider = $options['csrf_provider']; $intention = $options['intention']; - $validator = function (FormInterface $form) use ($csrfProvider, $intention) - { - if ((!$form->hasParent() || $form->getParent()->isRoot()) - && !$csrfProvider->isCsrfTokenValid($intention, $form->getData())) { - $form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form')); - $form->setData($csrfProvider->generateCsrfToken($intention)); - } - }; - $builder ->setData($csrfProvider->generateCsrfToken($intention)) - ->addValidator(new CallbackValidator($validator)) + ->addEventSubscriber(new CsrfValidationListener($csrfProvider, $intention)) ; } diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 292be24650..f1151b2540 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Form; use Symfony\Component\Form\Event\DataEvent; use Symfony\Component\Form\Event\FilterDataEvent; use Symfony\Component\Form\Exception\FormException; +use Symfony\Component\Form\Exception\AlreadyBoundException; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Exception\TransformationFailedException; use Symfony\Component\HttpFoundation\Request; @@ -297,6 +298,10 @@ class Form implements \IteratorAggregate, FormInterface */ public function setParent(FormInterface $parent = null) { + if ($this->bound) { + throw new AlreadyBoundException('You cannot set the parent of a bound form'); + } + if ('' === $this->getName()) { throw new FormException('Form with empty name can not have parent form.'); } @@ -377,6 +382,10 @@ class Form implements \IteratorAggregate, FormInterface */ public function setData($appData) { + if ($this->bound) { + throw new AlreadyBoundException('You cannot change the data of a bound form'); + } + $event = new DataEvent($this, $appData); $this->dispatcher->dispatch(FormEvents::PRE_SET_DATA, $event); @@ -451,6 +460,10 @@ class Form implements \IteratorAggregate, FormInterface */ public function bind($clientData) { + if ($this->bound) { + throw new AlreadyBoundException('A form can only be bound once'); + } + if ($this->isDisabled()) { $this->bound = true; @@ -689,7 +702,7 @@ class Form implements \IteratorAggregate, FormInterface */ public function isValid() { - if (!$this->isBound()) { + if (!$this->bound) { throw new \LogicException('You cannot call isValid() on a form that is not bound.'); } @@ -821,6 +834,10 @@ class Form implements \IteratorAggregate, FormInterface */ public function add(FormInterface $child) { + if ($this->bound) { + throw new AlreadyBoundException('You cannot add children to a bound form'); + } + $this->children[$child->getName()] = $child; $child->setParent($this); @@ -841,6 +858,10 @@ class Form implements \IteratorAggregate, FormInterface */ public function remove($name) { + if ($this->bound) { + throw new AlreadyBoundException('You cannot remove children from a bound form'); + } + if (isset($this->children[$name])) { $this->children[$name]->setParent(null); diff --git a/tests/Symfony/Tests/Component/Form/FormTest.php b/tests/Symfony/Tests/Component/Form/FormTest.php index 17cfbeaa46..aeb2e73f85 100644 --- a/tests/Symfony/Tests/Component/Form/FormTest.php +++ b/tests/Symfony/Tests/Component/Form/FormTest.php @@ -199,6 +199,15 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array('firstName' => 'Bernhard'), $this->form->getData()); } + /** + * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + */ + public function testBindThrowsExceptionIfAlreadyBound() + { + $this->form->bind(array()); + $this->form->bind(array()); + } + public function testBindForwardsNullIfValueIsMissing() { $child = $this->getMockForm('firstName'); @@ -408,8 +417,8 @@ class FormTest extends \PHPUnit_Framework_TestCase ->method('isValid') ->will($this->returnValue(false)); - $this->form->bind('foobar'); $this->form->add($child); + $this->form->bind(array()); $this->assertFalse($this->form->isValid()); } @@ -438,6 +447,15 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertFalse($this->form->hasChildren()); } + /** + * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + */ + public function testSetParentThrowsExceptionIfAlreadyBound() + { + $this->form->bind(array()); + $this->form->setParent($this->getBuilder('parent')->getForm()); + } + public function testAdd() { $child = $this->getBuilder('foo')->getForm(); @@ -447,6 +465,15 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertSame(array('foo' => $child), $this->form->getChildren()); } + /** + * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + */ + public function testAddThrowsExceptionIfAlreadyBound() + { + $this->form->bind(array()); + $this->form->add($this->getBuilder('foo')->getForm()); + } + public function testRemove() { $child = $this->getBuilder('foo')->getForm(); @@ -457,6 +484,16 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertFalse($this->form->hasChildren()); } + /** + * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + */ + public function testRemoveThrowsExceptionIfAlreadyBound() + { + $this->form->add($this->getBuilder('foo')->getForm()); + $this->form->bind(array('foo' => 'bar')); + $this->form->remove('foo'); + } + public function testRemoveIgnoresUnknownName() { $this->form->remove('notexisting'); @@ -504,6 +541,15 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertFalse($this->form->isBound()); } + /** + * @expectedException Symfony\Component\Form\Exception\AlreadyBoundException + */ + public function testSetDataThrowsExceptionIfAlreadyBound() + { + $this->form->bind(array()); + $this->form->setData(null); + } + public function testSetDataExecutesTransformationChain() { // use real event dispatcher now