Skip to content

Ensure that submitted data are uploaded files #24993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions UPGRADE-2.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ Router
Form
----

* the `isFileUpload()` method was added to the `RequestHandlerInterface`

* In form types and extension overriding the "setDefaultOptions" of the
AbstractType or AbstractTypeExtension has been deprecated in favor of
overriding the new "configureOptions" method.
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

2.7.38
------

* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`

2.7.0
-----

Expand Down
31 changes: 23 additions & 8 deletions src/Symfony/Component/Form/Extension/Core/Type/FileType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -28,9 +29,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
{
private $serverParams;

/**
* {@inheritdoc}
*/
public function __construct(ServerParams $serverParams = null)
{
$this->serverParams = $serverParams ?: new ServerParams();
Expand Down Expand Up @@ -109,4 +107,9 @@ public function handleRequest(FormInterface $form, $request = null)

$form->submit($data, 'PATCH' !== $method);
}

public function isFileUpload($data)
{
return $data instanceof File;
}
}
21 changes: 13 additions & 8 deletions src/Symfony/Component/Form/NativeRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -42,6 +34,11 @@ public function __construct(ServerParams $params = null)
'type',
);

public function __construct(ServerParams $params = null)
{
$this->serverParams = $params ?: new ServerParams();
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -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.
*
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Form/RequestHandlerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
12 changes: 12 additions & 0 deletions src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
121 changes: 84 additions & 37 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 'file';
Expand All @@ -28,40 +33,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);
Expand All @@ -72,11 +86,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']);
Expand Down Expand Up @@ -108,29 +125,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,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,9 @@ protected function getMockFile($suffix = '')
{
return new UploadedFile(__DIR__.'/../../Fixtures/foo'.$suffix, 'foo'.$suffix);
}

protected function getInvalidFile()
{
return 'file:///etc/passwd';
}
}
11 changes: 11 additions & 0 deletions src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}