Skip to content

Commit

Permalink
Merge branch '2.8' into 3.3
Browse files Browse the repository at this point in the history
* 2.8:
  fixed CS
  fixed CS
  [Security] Namespace generated CSRF tokens depending of the current scheme
  ensure that submitted data are uploaded files
  [Console] remove dead code
  bumped Symfony version to 2.8.31
  updated VERSION for 2.8.30
  updated CHANGELOG for 2.8.30
  bumped Symfony version to 2.7.38
  updated VERSION for 2.7.37
  updated CHANGELOG for 2.7.37
  [Security] Validate redirect targets using the session cookie domain
  prevent bundle readers from breaking out of paths
  • Loading branch information
nicolas-grekas committed Nov 16, 2017
2 parents 04a06fe + 4ad1176 commit 8559613
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 56 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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
-----

Expand Down
31 changes: 23 additions & 8 deletions 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
9 changes: 6 additions & 3 deletions Extension/HttpFoundation/HttpFoundationRequestHandler.php
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 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 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 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 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 = 'Symfony\Component\Form\Extension\Core\Type\FileType';
Expand All @@ -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);
Expand All @@ -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']);
Expand Down Expand Up @@ -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,
);
}
}
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 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',
);
}
}

0 comments on commit 8559613

Please sign in to comment.