Skip to content

Commit f9e210c

Browse files
committed
ensure that submitted data are uploaded files
1 parent 85f7cc8 commit f9e210c

File tree

10 files changed

+168
-56
lines changed

10 files changed

+168
-56
lines changed

UPGRADE-2.7.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ Router
3939
Form
4040
----
4141

42+
* the `isFileUpload()` method was added to the `RequestHandlerInterface`
43+
4244
* In form types and extension overriding the "setDefaultOptions" of the
4345
AbstractType or AbstractTypeExtension has been deprecated in favor of
4446
overriding the new "configureOptions" method.

src/Symfony/Component/Form/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
2.7.38
5+
------
6+
7+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
8+
49
2.7.0
510
-----
611

src/Symfony/Component/Form/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
/**

src/Symfony/Component/Form/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
}

src/Symfony/Component/Form/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
*

src/Symfony/Component/Form/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
}

src/Symfony/Component/Form/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();

src/Symfony/Component/Form/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 = 'file';
@@ -28,40 +33,49 @@ public function testSetData()
2833
$this->assertSame($data, $form->getData());
2934
}
3035

31-
public function testSubmit()
36+
/**
37+
* @dataProvider requestHandlerProvider
38+
*/
39+
public function testSubmit(RequestHandlerInterface $requestHandler)
3240
{
33-
$form = $this->factory->createBuilder(static::TESTED_TYPE)->getForm();
34-
$data = $this->createUploadedFileMock('abcdef', 'original.jpg', true);
41+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
42+
$data = $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg');
3543

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

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

41-
public function testSetDataMultiple()
49+
/**
50+
* @dataProvider requestHandlerProvider
51+
*/
52+
public function testSetDataMultiple(RequestHandlerInterface $requestHandler)
4253
{
4354
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
4455
'multiple' => true,
45-
))->getForm();
56+
))->setRequestHandler($requestHandler)->getForm();
4657

4758
$data = array(
48-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
49-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
59+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
60+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
5061
);
5162

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

56-
public function testSubmitMultiple()
67+
/**
68+
* @dataProvider requestHandlerProvider
69+
*/
70+
public function testSubmitMultiple(RequestHandlerInterface $requestHandler)
5771
{
5872
$form = $this->factory->createBuilder(static::TESTED_TYPE, null, array(
5973
'multiple' => true,
60-
))->getForm();
74+
))->setRequestHandler($requestHandler)->getForm();
6175

6276
$data = array(
63-
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
64-
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
77+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
78+
$this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
6579
);
6680

6781
$form->submit($data);
@@ -72,11 +86,14 @@ public function testSubmitMultiple()
7286
$this->assertArrayHasKey('multiple', $view->vars['attr']);
7387
}
7488

75-
public function testDontPassValueToView()
89+
/**
90+
* @dataProvider requestHandlerProvider
91+
*/
92+
public function testDontPassValueToView(RequestHandlerInterface $requestHandler)
7693
{
77-
$form = $this->factory->create(static::TESTED_TYPE);
94+
$form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
7895
$form->submit(array(
79-
'file' => $this->createUploadedFileMock('abcdef', 'original.jpg', true),
96+
'file' => $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
8097
));
8198

8299
$this->assertEquals('', $form->createView()->vars['value']);
@@ -108,29 +125,59 @@ public function testSubmitNullWhenMultiple()
108125
$this->assertSame(array(), $form->getViewData());
109126
}
110127

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

src/Symfony/Component/Form/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
}

src/Symfony/Component/Form/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)