Skip to content

Commit

Permalink
Merge branch '3.3' into 3.4
Browse files Browse the repository at this point in the history
* 3.3:
  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 3.3.13
  updated VERSION for 3.3.12
  updated CHANGELOG for 3.3.12
  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 db5f51d + 8559613 commit cda414b
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 @@ -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
-----

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 cda414b

Please sign in to comment.