Skip to content

Commit cda414b

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.3' into 3.4
* 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
2 parents db5f51d + 8559613 commit cda414b

File tree

9 files changed

+166
-56
lines changed

9 files changed

+166
-56
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ CHANGELOG
5757
* moved data trimming logic of TrimListener into StringUtil
5858
* [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type.
5959

60+
2.7.38
61+
------
62+
63+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
64+
6065
2.7.0
6166
-----
6267

‎Extension/Core/Type/FileType.php

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,35 @@ class FileType extends AbstractType
2727
*/
2828
public function buildForm(FormBuilderInterface $builder, array $options)
2929
{
30-
if ($options['multiple']) {
31-
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
32-
$form = $event->getForm();
33-
$data = $event->getData();
30+
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
31+
$form = $event->getForm();
32+
$requestHandler = $form->getConfig()->getRequestHandler();
33+
$data = null;
34+
35+
if ($options['multiple']) {
36+
$data = array();
37+
38+
foreach ($event->getData() as $file) {
39+
if ($requestHandler->isFileUpload($file)) {
40+
$data[] = $file;
41+
}
42+
}
3443

3544
// submitted data for an input file (not required) without choosing any file
36-
if (array(null) === $data) {
45+
if (array(null) === $data || array() === $data) {
3746
$emptyData = $form->getConfig()->getEmptyData();
3847

3948
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
40-
$event->setData($data);
4149
}
42-
});
43-
}
50+
51+
$event->setData($data);
52+
} elseif (!$requestHandler->isFileUpload($event->getData())) {
53+
$emptyData = $form->getConfig()->getEmptyData();
54+
55+
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
56+
$event->setData($data);
57+
}
58+
});
4459
}
4560

4661
/**

‎Extension/HttpFoundation/HttpFoundationRequestHandler.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Form\FormInterface;
1717
use Symfony\Component\Form\RequestHandlerInterface;
1818
use Symfony\Component\Form\Util\ServerParams;
19+
use Symfony\Component\HttpFoundation\File\File;
1920
use Symfony\Component\HttpFoundation\Request;
2021

2122
/**
@@ -28,9 +29,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
2829
{
2930
private $serverParams;
3031

31-
/**
32-
* {@inheritdoc}
33-
*/
3432
public function __construct(ServerParams $serverParams = null)
3533
{
3634
$this->serverParams = $serverParams ?: new ServerParams();
@@ -109,4 +107,9 @@ public function handleRequest(FormInterface $form, $request = null)
109107

110108
$form->submit($data, 'PATCH' !== $method);
111109
}
110+
111+
public function isFileUpload($data)
112+
{
113+
return $data instanceof File;
114+
}
112115
}

‎NativeRequestHandler.php

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ class NativeRequestHandler implements RequestHandlerInterface
2323
{
2424
private $serverParams;
2525

26-
/**
27-
* {@inheritdoc}
28-
*/
29-
public function __construct(ServerParams $params = null)
30-
{
31-
$this->serverParams = $params ?: new ServerParams();
32-
}
33-
3426
/**
3527
* The allowed keys of the $_FILES array.
3628
*/
@@ -42,6 +34,11 @@ public function __construct(ServerParams $params = null)
4234
'type',
4335
);
4436

37+
public function __construct(ServerParams $params = null)
38+
{
39+
$this->serverParams = $params ?: new ServerParams();
40+
}
41+
4542
/**
4643
* {@inheritdoc}
4744
*/
@@ -121,6 +118,14 @@ public function handleRequest(FormInterface $form, $request = null)
121118
$form->submit($data, 'PATCH' !== $method);
122119
}
123120

121+
public function isFileUpload($data)
122+
{
123+
// POST data will always be strings or arrays of strings. Thus, we can be sure
124+
// that the submitted data is a file upload if the "error" value is an integer
125+
// (this value must have been injected by PHP itself).
126+
return is_array($data) && isset($data['error']) && is_int($data['error']);
127+
}
128+
124129
/**
125130
* Returns the method used to submit the request to the server.
126131
*

‎RequestHandlerInterface.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,11 @@ interface RequestHandlerInterface
2525
* @param mixed $request The current request
2626
*/
2727
public function handleRequest(FormInterface $form, $request = null);
28+
29+
/**
30+
* @param mixed $data
31+
*
32+
* @return bool
33+
*/
34+
public function isFileUpload($data);
2835
}

‎Tests/AbstractRequestHandlerTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,12 +353,24 @@ public function getPostMaxSizeFixtures()
353353
);
354354
}
355355

356+
public function testUploadedFilesAreAccepted()
357+
{
358+
$this->assertTrue($this->requestHandler->isFileUpload($this->getMockFile()));
359+
}
360+
361+
public function testInvalidFilesAreRejected()
362+
{
363+
$this->assertFalse($this->requestHandler->isFileUpload($this->getInvalidFile()));
364+
}
365+
356366
abstract protected function setRequestData($method, $data, $files = array());
357367

358368
abstract protected function getRequestHandler();
359369

360370
abstract protected function getMockFile($suffix = '');
361371

372+
abstract protected function getInvalidFile();
373+
362374
protected function getMockForm($name, $method = null, $compound = true)
363375
{
364376
$config = $this->getMockBuilder('Symfony\Component\Form\FormConfigInterface')->getMock();

‎Tests/Extension/Core/Type/FileTypeTest.php

Lines changed: 84 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111

1212
namespace Symfony\Component\Form\Tests\Extension\Core\Type;
1313

14+
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
15+
use Symfony\Component\Form\NativeRequestHandler;
16+
use Symfony\Component\Form\RequestHandlerInterface;
17+
use Symfony\Component\HttpFoundation\File\UploadedFile;
18+
1419
class FileTypeTest extends BaseTypeTest
1520
{
1621
const TESTED_TYPE = 'Symfony\Component\Form\Extension\Core\Type\FileType';
@@ -29,40 +34,49 @@ public function testSetData()
2934
$this->assertSame($data, $form->getData());
3035
}
3136

32-
public function testSubmit()
37+
/**
38+
* @dataProvider requestHandlerProvider
39+
*/
40+
public function testSubmit(RequestHandlerInterface $requestHandler)
3341
{
34-
$form = $this->factory->createBuilder(static::TESTED_TYPE)->getForm();
35-
$data = $this->createUploadedFileMock('abcdef', 'original.jpg', true);
42+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
43+
$data = $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg');
3644

3745
$form->submit($data);
3846

3947
$this->assertSame($data, $form->getData());
4048
}
4149

42-
public function testSetDataMultiple()
50+
/**
51+
* @dataProvider requestHandlerProvider
52+
*/
53+
public function testSetDataMultiple(RequestHandlerInterface $requestHandler)
4354
{
4455
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
4556
'multiple' => true,
46-
))->getForm();
57+
))->setRequestHandler($requestHandler)->getForm();
4758

4859
$data = array(
49-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
50-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
60+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
61+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
5162
);
5263

5364
$form->setData($data);
5465
$this->assertSame($data, $form->getData());
5566
}
5667

57-
public function testSubmitMultiple()
68+
/**
69+
* @dataProvider requestHandlerProvider
70+
*/
71+
public function testSubmitMultiple(RequestHandlerInterface $requestHandler)
5872
{
5973
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
6074
'multiple' => true,
61-
))->getForm();
75+
))->setRequestHandler($requestHandler)->getForm();
6276

6377
$data = array(
64-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
65-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
78+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
79+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
6680
);
6781

6882
$form->submit($data);
@@ -73,11 +87,14 @@ public function testSubmitMultiple()
7387
$this->assertArrayHasKey('multiple', $view->vars['attr']);
7488
}
7589

76-
public function testDontPassValueToView()
90+
/**
91+
* @dataProvider requestHandlerProvider
92+
*/
93+
public function testDontPassValueToView(RequestHandlerInterface $requestHandler)
7794
{
78-
$form = $this->factory->create(static::TESTED_TYPE);
95+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
7996
$form->submit(array(
80-
'file' => $this->createUploadedFileMock('abcdef', 'original.jpg', true),
97+
'file' => $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
8198
));
8299

83100
$this->assertEquals('', $form->createView()->vars['value']);
@@ -109,29 +126,59 @@ public function testSubmitNullWhenMultiple()
109126
$this->assertSame(array(), $form->getViewData());
110127
}
111128

112-
private function createUploadedFileMock($name, $originalName, $valid)
129+
/**
130+
* @dataProvider requestHandlerProvider
131+
*/
132+
public function testSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler)
113133
{
114-
$file = $this
115-
->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')
116-
->setConstructorArgs(array(__DIR__.'/../../../Fixtures/foo', 'foo'))
117-
->getMock()
118-
;
119-
$file
120-
->expects($this->any())
121-
->method('getBasename')
122-
->will($this->returnValue($name))
123-
;
124-
$file
125-
->expects($this->any())
126-
->method('getClientOriginalName')
127-
->will($this->returnValue($originalName))
128-
;
129-
$file
130-
->expects($this->any())
131-
->method('isValid')
132-
->will($this->returnValue($valid))
133-
;
134-
135-
return $file;
134+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
135+
$form->submit('file:///etc/passwd');
136+
137+
$this->assertNull($form->getData());
138+
$this->assertNull($form->getNormData());
139+
$this->assertSame('', $form->getViewData());
140+
}
141+
142+
/**
143+
* @dataProvider requestHandlerProvider
144+
*/
145+
public function testMultipleSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler)
146+
{
147+
$form = $this->factory
148+
->createBuilder(static::TESTED_TYPE, null, array(
149+
'multiple' => true,
150+
))
151+
->setRequestHandler($requestHandler)
152+
->getForm();
153+
$form->submit(array(
154+
'file:///etc/passwd',
155+
$this->createUploadedFileMock(new HttpFoundationRequestHandler(), __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
156+
$this->createUploadedFileMock(new NativeRequestHandler(), __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
157+
));
158+
159+
$this->assertCount(1, $form->getData());
160+
}
161+
162+
public function requestHandlerProvider()
163+
{
164+
return array(
165+
array(new HttpFoundationRequestHandler()),
166+
array(new NativeRequestHandler()),
167+
);
168+
}
169+
170+
private function createUploadedFileMock(RequestHandlerInterface $requestHandler, $path, $originalName)
171+
{
172+
if ($requestHandler instanceof HttpFoundationRequestHandler) {
173+
return new UploadedFile($path, $originalName, null, 10, null, true);
174+
}
175+
176+
return array(
177+
'name' => $originalName,
178+
'error' => 0,
179+
'type' => 'text/plain',
180+
'tmp_name' => $path,
181+
'size' => 10,
182+
);
136183
}
137184
}

‎Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,9 @@ protected function getMockFile($suffix = '')
5151
{
5252
return new UploadedFile(__DIR__.'/../../Fixtures/foo'.$suffix, 'foo'.$suffix);
5353
}
54+
55+
protected function getInvalidFile()
56+
{
57+
return 'file:///etc/passwd';
58+
}
5459
}

‎Tests/NativeRequestHandlerTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,4 +216,15 @@ protected function getMockFile($suffix = '')
216216
'size' => 100,
217217
);
218218
}
219+
220+
protected function getInvalidFile()
221+
{
222+
return array(
223+
'name' => 'upload.txt',
224+
'type' => 'text/plain',
225+
'tmp_name' => 'owfdskjasdfsa',
226+
'error' => '0',
227+
'size' => '100',
228+
);
229+
}
219230
}

0 commit comments

Comments
 (0)