Skip to content

[HttpFoundation] Fix file upload multiple with no files #24198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 30, 2017
Merged

[HttpFoundation] Fix file upload multiple with no files #24198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpFoundation/FileBag.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function add(array $files = array())
*
* @param array|UploadedFile $file A (multi-dimensional) array of uploaded file information
*
* @return UploadedFile|UploadedFile[] A (multi-dimensional) array of UploadedFile instances
* @return UploadedFile[]|UploadedFile|null A (multi-dimensional) array of UploadedFile instances
*/
protected function convertFileInformation($file)
{
Expand All @@ -89,7 +89,7 @@ protected function convertFileInformation($file)
$file = new UploadedFile($file['tmp_name'], $file['name'], $file['type'], $file['size'], $file['error']);
}
} else {
$file = array_map(array($this, 'convertFileInformation'), $file);
$file = array_filter(array_map(array($this, 'convertFileInformation'), $file));
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ public function testShouldSetEmptyUploadedFilesToNull()
$this->assertNull($bag->get('file'));
}

public function testShouldRemoveEmptyUploadedFilesForMultiUpload()
{
$bag = new FileBag(array('file' => array(
'name' => array(''),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the structure that is used when multiple files are submitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh Yes.

Copy link
Contributor Author

@enumag enumag Sep 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more precise it's the structure used for <input type="file" name="file[]"> (note the []), regardless of how many files are actually sent (including 0) and regardless of the multiple attribute.

You can try yourself with the example script I posted above. Just add var_export($_FILES); somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the records, that's also explicitly described here: http://php.net/manual/en/features.file-upload.multiple.php

Copy link
Member

@yceruto yceruto Sep 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce the issue, this FileBag with those values returns:

file-bag

according to:

symfony/src/Symfony/Component/HttpFoundation/FileBag.php

Lines 86 to 87 in 12f1239

if (UPLOAD_ERR_NO_FILE == $file['error']) {
$file = null;

the rest is handled on PRE_SUBMIT event inside FileType if multiple = true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See test case also:

symfony/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php

Lines 99 to 110 in 77653d1

public function testSubmitNullWhenMultiple()
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
'multiple' => true,
));
// submitted data when an input file is uploaded without choosing any file
$form->submit(array(null));
$this->assertSame(array(), $form->getData());
$this->assertSame(array(), $form->getNormData());
$this->assertSame(array(), $form->getViewData());
}

'type' => array(''),
'tmp_name' => array(''),
'error' => array(UPLOAD_ERR_NO_FILE),
'size' => array(0),
)));

$this->assertSame(array(), $bag->get('file'));
}

public function testShouldConvertUploadedFilesWithPhpBug()
{
$tmpFile = $this->createTempFile();
Expand Down