From 4fcb98547ce20f2e1e3bfe5be52af29355974e51 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Jan 2011 23:09:22 +0100 Subject: [PATCH] [Form] Simplified Form::bind(), added convenience methods Form::bindRequest() and Form::bindGlobals() --- src/Symfony/Component/Form/Form.php | 71 +++++++++--------- .../Symfony/Tests/Component/Form/FormTest.php | 75 ++++++++++++++++--- 2 files changed, 99 insertions(+), 47 deletions(-) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 1db76caa59..6c16b7fab7 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Form; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\FileBag; use Symfony\Component\Validator\ValidatorInterface; use Symfony\Component\Form\Exception\FormException; use Symfony\Component\Form\CsrfProvider\CsrfProviderInterface; @@ -20,11 +22,11 @@ use Symfony\Component\Form\CsrfProvider\CsrfProviderInterface; * * A form is composed of a validator schema and a widget form schema. * - * Form also takes care of Csrf protection by default. + * Form also takes care of CSRF protection by default. * - * A Csrf secret can be any random string. If set to false, it disables the - * Csrf protection, and if set to null, it forces the form to use the global - * Csrf secret. If the global Csrf secret is also null, then a random one + * A CSRF secret can be any random string. If set to false, it disables the + * CSRF protection, and if set to null, it forces the form to use the global + * CSRF secret. If the global CSRF secret is also null, then a random one * is generated on the fly. * * @author Fabien Potencier @@ -141,32 +143,42 @@ class Form extends FieldGroup } /** - * Binds the form with values and files. + * Binds the form with submitted data from a Request object * - * This method is final because it is very easy to break a form when - * overriding this method and adding logic that depends on $taintedFiles. - * You should override doBind() instead where the uploaded files are - * already merged into the data array. - * - * @param array $taintedValues The form data of the $_POST array - * @param array $taintedFiles An array of uploaded files - * @return Boolean Whether the form is valid + * @param Request $request The request object + * @see bind() */ - final public function bind($taintedValues, array $taintedFiles = null) + public function bindRequest(Request $request) { - if (null === $taintedFiles) { - if ($this->isMultipart() && $this->getParent() === null) { - throw new \InvalidArgumentException('You must provide a files array for multipart forms'); - } + $values = $request->request->get($this->getName()); + $files = $request->files->get($this->getName()); - $taintedFiles = array(); - } + $this->bind(self::deepArrayUnion($values, $files)); + } - if (null === $taintedValues) { - $taintedValues = array(); - } + /** + * Binds the form with submitted data from the PHP globals $_POST and + * $_FILES + * + * @see bind() + */ + public function bindGlobals() + { + $values = $_POST[$this->getName()]; - $this->doBind(self::deepArrayUnion($taintedValues, $taintedFiles)); + // fix files array and convert to UploadedFile instances + $fileBag = new FileBag($_FILES); + $files = $fileBag->get($this->getName()); + + $this->bind(self::deepArrayUnion($values, $files)); + } + + /** + * {@inheritDoc} + */ + public function bind($values) + { + parent::bind($values); if ($this->getParent() === null) { if ($this->validator === null) { @@ -192,17 +204,6 @@ class Form extends FieldGroup } } - /** - * Binds the form with the given data. - * - * @param array $taintedData The data to bind to the form - * @return Boolean Whether the form is valid - */ - protected function doBind(array $taintedData) - { - parent::bind($taintedData); - } - /** * @return true if this form is CSRF protected */ diff --git a/tests/Symfony/Tests/Component/Form/FormTest.php b/tests/Symfony/Tests/Component/Form/FormTest.php index 1a50350dbf..9548e95393 100644 --- a/tests/Symfony/Tests/Component/Form/FormTest.php +++ b/tests/Symfony/Tests/Component/Form/FormTest.php @@ -20,6 +20,8 @@ use Symfony\Component\Form\Field; use Symfony\Component\Form\HiddenField; use Symfony\Component\Form\FieldGroup; use Symfony\Component\Form\PropertyPath; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\Validator\ConstraintViolation; use Symfony\Component\Validator\ConstraintViolationList; use Symfony\Tests\Component\Form\Fixtures\Author; @@ -222,26 +224,75 @@ class FormTest extends \PHPUnit_Framework_TestCase $form->bind(array()); // irrelevant } - public function testMultipartFormsWithoutParentsRequireFiles() + public function testBindRequest() { - $form = new Form('author', new Author(), $this->validator); - $form->add($this->createMultipartMockField('file')); + $values = array( + 'author' => array( + 'name' => 'Bernhard', + 'image' => array('filename' => 'foobar.png'), + ), + ); + $files = array( + 'author' => array( + 'error' => array('image' => array('file' => UPLOAD_ERR_OK)), + 'name' => array('image' => array('file' => 'upload.png')), + 'size' => array('image' => array('file' => 123)), + 'tmp_name' => array('image' => array('file' => 'abcdef.png')), + 'type' => array('image' => array('file' => 'image/png')), + ), + ); - $this->setExpectedException('InvalidArgumentException'); + $request = new Request(array(), $values, array(), array(), $files); - // should be given in second argument - $form->bind(array('file' => 'test.txt')); + $form = new Form('author', null, $this->createMockValidator()); + $form->add(new TestField('name')); + $imageForm = new FieldGroup('image'); + $imageForm->add(new TestField('file')); + $imageForm->add(new TestField('filename')); + $form->add($imageForm); + + $form->bindRequest($request); + + $file = new UploadedFile('abcdef.png', 'upload.png', 'image/png', 123, UPLOAD_ERR_OK); + + $this->assertEquals('Bernhard', $form['name']->getData()); + $this->assertEquals('foobar.png', $form['image']['filename']->getData()); + $this->assertEquals($file, $form['image']['file']->getData()); } - public function testMultipartFormsWithParentsRequireNoFiles() + public function testBindGlobals() { - $form = new Form('author', new Author(), $this->validator); - $form->add($this->createMultipartMockField('file')); + $_POST = array( + 'author' => array( + 'name' => 'Bernhard', + 'image' => array('filename' => 'foobar.png'), + ), + ); + $_FILES = array( + 'author' => array( + 'error' => array('image' => array('file' => UPLOAD_ERR_OK)), + 'name' => array('image' => array('file' => 'upload.png')), + 'size' => array('image' => array('file' => 123)), + 'tmp_name' => array('image' => array('file' => 'abcdef.png')), + 'type' => array('image' => array('file' => 'image/png')), + ), + ); - $form->setParent($this->createMockField('group')); - // files are expected to be converted by the parent - $form->bind(array('file' => 'test.txt')); + $form = new Form('author', null, $this->createMockValidator()); + $form->add(new TestField('name')); + $imageForm = new FieldGroup('image'); + $imageForm->add(new TestField('file')); + $imageForm->add(new TestField('filename')); + $form->add($imageForm); + + $form->bindGlobals(); + + $file = new UploadedFile('abcdef.png', 'upload.png', 'image/png', 123, UPLOAD_ERR_OK); + + $this->assertEquals('Bernhard', $form['name']->getData()); + $this->assertEquals('foobar.png', $form['image']['filename']->getData()); + $this->assertEquals($file, $form['image']['file']->getData()); } public function testUpdateFromPropertyIsIgnoredIfFormHasObject()