Skip to content

Commit 8559613

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.8' into 3.3
* 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
2 parents 04a06fe + 4ad1176 commit 8559613

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
@@ -48,6 +48,11 @@ CHANGELOG
4848
* moved data trimming logic of TrimListener into StringUtil
4949
* [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type.
5050

51+
2.7.38
52+
------
53+
54+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
55+
5156
2.7.0
5257
-----
5358

‎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)