Skip to content

Commit 4ad1176

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.7' into 2.8
* 2.7: 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.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 6b6e1e3 + ece23c6 commit 4ad1176

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

15+
2.7.38
16+
------
17+
18+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
19+
1520
2.7.0
1621
-----
1722

‎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';
@@ -39,40 +44,49 @@ public function testSetData()
3944
$this->assertSame($data, $form->getData());
4045
}
4146

42-
public function testSubmit()
47+
/**
48+
* @dataProvider requestHandlerProvider
49+
*/
50+
public function testSubmit(RequestHandlerInterface $requestHandler)
4351
{
44-
$form = $this->factory->createBuilder(static::TESTED_TYPE)->getForm();
45-
$data = $this->createUploadedFileMock('abcdef', 'original.jpg', true);
52+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
53+
$data = $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg');
4654

4755
$form->submit($data);
4856

4957
$this->assertSame($data, $form->getData());
5058
}
5159

52-
public function testSetDataMultiple()
60+
/**
61+
* @dataProvider requestHandlerProvider
62+
*/
63+
public function testSetDataMultiple(RequestHandlerInterface $requestHandler)
5364
{
5465
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
5566
'multiple' => true,
56-
))->getForm();
67+
))->setRequestHandler($requestHandler)->getForm();
5768

5869
$data = array(
59-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
60-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
70+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
71+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
6172
);
6273

6374
$form->setData($data);
6475
$this->assertSame($data, $form->getData());
6576
}
6677

67-
public function testSubmitMultiple()
78+
/**
79+
* @dataProvider requestHandlerProvider
80+
*/
81+
public function testSubmitMultiple(RequestHandlerInterface $requestHandler)
6882
{
6983
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
7084
'multiple' => true,
71-
))->getForm();
85+
))->setRequestHandler($requestHandler)->getForm();
7286

7387
$data = array(
74-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
75-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
88+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
89+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
7690
);
7791

7892
$form->submit($data);
@@ -83,11 +97,14 @@ public function testSubmitMultiple()
8397
$this->assertArrayHasKey('multiple', $view->vars['attr']);
8498
}
8599

86-
public function testDontPassValueToView()
100+
/**
101+
* @dataProvider requestHandlerProvider
102+
*/
103+
public function testDontPassValueToView(RequestHandlerInterface $requestHandler)
87104
{
88-
$form = $this->factory->create(static::TESTED_TYPE);
105+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
89106
$form->submit(array(
90-
'file' => $this->createUploadedFileMock('abcdef', 'original.jpg', true),
107+
'file' => $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
91108
));
92109

93110
$this->assertEquals('', $form->createView()->vars['value']);
@@ -119,29 +136,59 @@ public function testSubmitNullWhenMultiple()
119136
$this->assertSame(array(), $form->getViewData());
120137
}
121138

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

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