diff --git a/CHANGELOG.md b/CHANGELOG.md index 19105b9e4d..123ec4fbe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,11 @@ CHANGELOG * moved data trimming logic of TrimListener into StringUtil * [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type. +2.7.38 +------ + + * [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface` + 2.7.0 ----- diff --git a/Extension/Core/Type/FileType.php b/Extension/Core/Type/FileType.php index c882c8eda3..b801101cba 100644 --- a/Extension/Core/Type/FileType.php +++ b/Extension/Core/Type/FileType.php @@ -27,20 +27,35 @@ class FileType extends AbstractType */ public function buildForm(FormBuilderInterface $builder, array $options) { - if ($options['multiple']) { - $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { - $form = $event->getForm(); - $data = $event->getData(); + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) { + $form = $event->getForm(); + $requestHandler = $form->getConfig()->getRequestHandler(); + $data = null; + + if ($options['multiple']) { + $data = array(); + + foreach ($event->getData() as $file) { + if ($requestHandler->isFileUpload($file)) { + $data[] = $file; + } + } // submitted data for an input file (not required) without choosing any file - if (array(null) === $data) { + if (array(null) === $data || array() === $data) { $emptyData = $form->getConfig()->getEmptyData(); $data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData; - $event->setData($data); } - }); - } + + $event->setData($data); + } elseif (!$requestHandler->isFileUpload($event->getData())) { + $emptyData = $form->getConfig()->getEmptyData(); + + $data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData; + $event->setData($data); + } + }); } /** diff --git a/Extension/HttpFoundation/HttpFoundationRequestHandler.php b/Extension/HttpFoundation/HttpFoundationRequestHandler.php index f65ee33f87..368a5a80a1 100644 --- a/Extension/HttpFoundation/HttpFoundationRequestHandler.php +++ b/Extension/HttpFoundation/HttpFoundationRequestHandler.php @@ -16,6 +16,7 @@ use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\RequestHandlerInterface; use Symfony\Component\Form\Util\ServerParams; +use Symfony\Component\HttpFoundation\File\File; use Symfony\Component\HttpFoundation\Request; /** @@ -28,9 +29,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface { private $serverParams; - /** - * {@inheritdoc} - */ public function __construct(ServerParams $serverParams = null) { $this->serverParams = $serverParams ?: new ServerParams(); @@ -109,4 +107,9 @@ public function handleRequest(FormInterface $form, $request = null) $form->submit($data, 'PATCH' !== $method); } + + public function isFileUpload($data) + { + return $data instanceof File; + } } diff --git a/NativeRequestHandler.php b/NativeRequestHandler.php index e28c2b4b10..f68efe25cf 100644 --- a/NativeRequestHandler.php +++ b/NativeRequestHandler.php @@ -23,14 +23,6 @@ class NativeRequestHandler implements RequestHandlerInterface { private $serverParams; - /** - * {@inheritdoc} - */ - public function __construct(ServerParams $params = null) - { - $this->serverParams = $params ?: new ServerParams(); - } - /** * The allowed keys of the $_FILES array. */ @@ -42,6 +34,11 @@ public function __construct(ServerParams $params = null) 'type', ); + public function __construct(ServerParams $params = null) + { + $this->serverParams = $params ?: new ServerParams(); + } + /** * {@inheritdoc} */ @@ -121,6 +118,14 @@ public function handleRequest(FormInterface $form, $request = null) $form->submit($data, 'PATCH' !== $method); } + public function isFileUpload($data) + { + // POST data will always be strings or arrays of strings. Thus, we can be sure + // that the submitted data is a file upload if the "error" value is an integer + // (this value must have been injected by PHP itself). + return is_array($data) && isset($data['error']) && is_int($data['error']); + } + /** * Returns the method used to submit the request to the server. * diff --git a/RequestHandlerInterface.php b/RequestHandlerInterface.php index 598a7bb174..e6360e4498 100644 --- a/RequestHandlerInterface.php +++ b/RequestHandlerInterface.php @@ -25,4 +25,11 @@ interface RequestHandlerInterface * @param mixed $request The current request */ public function handleRequest(FormInterface $form, $request = null); + + /** + * @param mixed $data + * + * @return bool + */ + public function isFileUpload($data); } diff --git a/Tests/AbstractRequestHandlerTest.php b/Tests/AbstractRequestHandlerTest.php index f9aa7a5341..7bc1a65219 100644 --- a/Tests/AbstractRequestHandlerTest.php +++ b/Tests/AbstractRequestHandlerTest.php @@ -353,12 +353,24 @@ public function getPostMaxSizeFixtures() ); } + public function testUploadedFilesAreAccepted() + { + $this->assertTrue($this->requestHandler->isFileUpload($this->getMockFile())); + } + + public function testInvalidFilesAreRejected() + { + $this->assertFalse($this->requestHandler->isFileUpload($this->getInvalidFile())); + } + abstract protected function setRequestData($method, $data, $files = array()); abstract protected function getRequestHandler(); abstract protected function getMockFile($suffix = ''); + abstract protected function getInvalidFile(); + protected function getMockForm($name, $method = null, $compound = true) { $config = $this->getMockBuilder('Symfony\Component\Form\FormConfigInterface')->getMock(); diff --git a/Tests/Extension/Core/Type/FileTypeTest.php b/Tests/Extension/Core/Type/FileTypeTest.php index 1545e8ae9d..8ac62c8dfb 100644 --- a/Tests/Extension/Core/Type/FileTypeTest.php +++ b/Tests/Extension/Core/Type/FileTypeTest.php @@ -11,6 +11,11 @@ namespace Symfony\Component\Form\Tests\Extension\Core\Type; +use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler; +use Symfony\Component\Form\NativeRequestHandler; +use Symfony\Component\Form\RequestHandlerInterface; +use Symfony\Component\HttpFoundation\File\UploadedFile; + class FileTypeTest extends BaseTypeTest { const TESTED_TYPE = 'Symfony\Component\Form\Extension\Core\Type\FileType'; @@ -29,40 +34,49 @@ public function testSetData() $this->assertSame($data, $form->getData()); } - public function testSubmit() + /** + * @dataProvider requestHandlerProvider + */ + public function testSubmit(RequestHandlerInterface $requestHandler) { - $form = $this->factory->createBuilder(static::TESTED_TYPE)->getForm(); - $data = $this->createUploadedFileMock('abcdef', 'original.jpg', true); + $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm(); + $data = $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'); $form->submit($data); $this->assertSame($data, $form->getData()); } - public function testSetDataMultiple() + /** + * @dataProvider requestHandlerProvider + */ + public function testSetDataMultiple(RequestHandlerInterface $requestHandler) { $form = $this->factory->createBuilder(static::TESTED_TYPE, null, array( 'multiple' => true, - ))->getForm(); + ))->setRequestHandler($requestHandler)->getForm(); $data = array( - $this->createUploadedFileMock('abcdef', 'first.jpg', true), - $this->createUploadedFileMock('zyxwvu', 'second.jpg', true), + $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'), + $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'), ); $form->setData($data); $this->assertSame($data, $form->getData()); } - public function testSubmitMultiple() + /** + * @dataProvider requestHandlerProvider + */ + public function testSubmitMultiple(RequestHandlerInterface $requestHandler) { $form = $this->factory->createBuilder(static::TESTED_TYPE, null, array( 'multiple' => true, - ))->getForm(); + ))->setRequestHandler($requestHandler)->getForm(); $data = array( - $this->createUploadedFileMock('abcdef', 'first.jpg', true), - $this->createUploadedFileMock('zyxwvu', 'second.jpg', true), + $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'), + $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'), ); $form->submit($data); @@ -73,11 +87,14 @@ public function testSubmitMultiple() $this->assertArrayHasKey('multiple', $view->vars['attr']); } - public function testDontPassValueToView() + /** + * @dataProvider requestHandlerProvider + */ + public function testDontPassValueToView(RequestHandlerInterface $requestHandler) { - $form = $this->factory->create(static::TESTED_TYPE); + $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm(); $form->submit(array( - 'file' => $this->createUploadedFileMock('abcdef', 'original.jpg', true), + 'file' => $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'), )); $this->assertEquals('', $form->createView()->vars['value']); @@ -109,29 +126,59 @@ public function testSubmitNullWhenMultiple() $this->assertSame(array(), $form->getViewData()); } - private function createUploadedFileMock($name, $originalName, $valid) + /** + * @dataProvider requestHandlerProvider + */ + public function testSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler) { - $file = $this - ->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile') - ->setConstructorArgs(array(__DIR__.'/../../../Fixtures/foo', 'foo')) - ->getMock() - ; - $file - ->expects($this->any()) - ->method('getBasename') - ->will($this->returnValue($name)) - ; - $file - ->expects($this->any()) - ->method('getClientOriginalName') - ->will($this->returnValue($originalName)) - ; - $file - ->expects($this->any()) - ->method('isValid') - ->will($this->returnValue($valid)) - ; - - return $file; + $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm(); + $form->submit('file:///etc/passwd'); + + $this->assertNull($form->getData()); + $this->assertNull($form->getNormData()); + $this->assertSame('', $form->getViewData()); + } + + /** + * @dataProvider requestHandlerProvider + */ + public function testMultipleSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler) + { + $form = $this->factory + ->createBuilder(static::TESTED_TYPE, null, array( + 'multiple' => true, + )) + ->setRequestHandler($requestHandler) + ->getForm(); + $form->submit(array( + 'file:///etc/passwd', + $this->createUploadedFileMock(new HttpFoundationRequestHandler(), __DIR__.'/../../../Fixtures/foo', 'foo.jpg'), + $this->createUploadedFileMock(new NativeRequestHandler(), __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'), + )); + + $this->assertCount(1, $form->getData()); + } + + public function requestHandlerProvider() + { + return array( + array(new HttpFoundationRequestHandler()), + array(new NativeRequestHandler()), + ); + } + + private function createUploadedFileMock(RequestHandlerInterface $requestHandler, $path, $originalName) + { + if ($requestHandler instanceof HttpFoundationRequestHandler) { + return new UploadedFile($path, $originalName, null, 10, null, true); + } + + return array( + 'name' => $originalName, + 'error' => 0, + 'type' => 'text/plain', + 'tmp_name' => $path, + 'size' => 10, + ); } } diff --git a/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php b/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php index e71a1fdfd8..9fbe122583 100644 --- a/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php +++ b/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php @@ -51,4 +51,9 @@ protected function getMockFile($suffix = '') { return new UploadedFile(__DIR__.'/../../Fixtures/foo'.$suffix, 'foo'.$suffix); } + + protected function getInvalidFile() + { + return 'file:///etc/passwd'; + } } diff --git a/Tests/NativeRequestHandlerTest.php b/Tests/NativeRequestHandlerTest.php index 38580063e7..aba417288a 100644 --- a/Tests/NativeRequestHandlerTest.php +++ b/Tests/NativeRequestHandlerTest.php @@ -216,4 +216,15 @@ protected function getMockFile($suffix = '') 'size' => 100, ); } + + protected function getInvalidFile() + { + return array( + 'name' => 'upload.txt', + 'type' => 'text/plain', + 'tmp_name' => 'owfdskjasdfsa', + 'error' => '0', + 'size' => '100', + ); + } }