bug #25471 [HttpFoundation] we should not pass size on FileBag removing the contruct parameter (Simperfit, xabbuh)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[HttpFoundation] we should not pass size on FileBag removing the contruct parameter

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25466 <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | none

We may have forgotten this one, thanks to @craue for seeing it !

Commits
-------

0db65b5 fix tests
820186f [HttpFoundation] we should not pass size on FileBag
This commit is contained in:
Nicolas Grekas 2018-02-04 16:57:00 +01:00
commit d9e9b261a4
11 changed files with 63 additions and 52 deletions

View File

@ -597,7 +597,7 @@ class CompoundFormTest extends AbstractFormTest
{
$path = tempnam(sys_get_temp_dir(), 'sf2');
touch($path);
file_put_contents($path, 'zaza');
$values = array(
'author' => array(
'name' => 'Bernhard',
@ -630,7 +630,7 @@ class CompoundFormTest extends AbstractFormTest
$form->handleRequest($request);
$file = new UploadedFile($path, 'upload.png', 'image/png', null, UPLOAD_ERR_OK);
$file = new UploadedFile($path, 'upload.png', 'image/png', UPLOAD_ERR_OK);
$this->assertEquals('Bernhard', $form['name']->getData());
$this->assertEquals($file, $form['image']->getData());
@ -645,6 +645,7 @@ class CompoundFormTest extends AbstractFormTest
{
$path = tempnam(sys_get_temp_dir(), 'sf2');
touch($path);
file_put_contents($path, 'zaza');
$values = array(
'name' => 'Bernhard',
@ -676,7 +677,7 @@ class CompoundFormTest extends AbstractFormTest
$form->handleRequest($request);
$file = new UploadedFile($path, 'upload.png', 'image/png', null, UPLOAD_ERR_OK);
$file = new UploadedFile($path, 'upload.png', 'image/png', UPLOAD_ERR_OK);
$this->assertEquals('Bernhard', $form['name']->getData());
$this->assertEquals($file, $form['image']->getData());
@ -692,6 +693,7 @@ class CompoundFormTest extends AbstractFormTest
{
$path = tempnam(sys_get_temp_dir(), 'sf2');
touch($path);
file_put_contents($path, 'zaza');
$files = array(
'image' => array(
@ -714,7 +716,7 @@ class CompoundFormTest extends AbstractFormTest
$form->handleRequest($request);
$file = new UploadedFile($path, 'upload.png', 'image/png', null, UPLOAD_ERR_OK);
$file = new UploadedFile($path, 'upload.png', 'image/png', UPLOAD_ERR_OK);
$this->assertEquals($file, $form->getData());
@ -728,6 +730,7 @@ class CompoundFormTest extends AbstractFormTest
{
$path = tempnam(sys_get_temp_dir(), 'sf2');
touch($path);
file_put_contents($path, 'zaza');
$values = array(
'name' => 'Bernhard',

View File

@ -188,7 +188,7 @@ class FileTypeTest extends BaseTypeTest
private function createUploadedFileMock(RequestHandlerInterface $requestHandler, $path, $originalName)
{
if ($requestHandler instanceof HttpFoundationRequestHandler) {
return new UploadedFile($path, $originalName, null, null, null, true);
return new UploadedFile($path, $originalName, null, null, true);
}
return array(

View File

@ -27,7 +27,6 @@ class UploadedFile extends File
private $test = false;
private $originalName;
private $mimeType;
private $size;
private $error;
/**
@ -47,7 +46,6 @@ class UploadedFile extends File
* @param string $path The full temporary path to the file
* @param string $originalName The original file name of the uploaded file
* @param string|null $mimeType The type of the file as provided by PHP; null defaults to application/octet-stream
* @param int|null $size The file size provided by the uploader
* @param int|null $error The error constant of the upload (one of PHP's UPLOAD_ERR_XXX constants); null defaults to UPLOAD_ERR_OK
* @param bool $test Whether the test mode is active
* Local files are used in test mode hence the code should not enforce HTTP uploads
@ -55,14 +53,17 @@ class UploadedFile extends File
* @throws FileException If file_uploads is disabled
* @throws FileNotFoundException If the file does not exist
*/
public function __construct(string $path, string $originalName, string $mimeType = null, int $size = null, int $error = null, bool $test = false)
public function __construct(string $path, string $originalName, string $mimeType = null, int $error = null, $test = false)
{
$this->originalName = $this->getName($originalName);
$this->mimeType = $mimeType ?: 'application/octet-stream';
$this->size = $size;
if (null !== $size) {
@trigger_error('Passing a size in the constructor is deprecated since Symfony 4.1 and will be removed in 5.0. Use getSize() instead.', E_USER_DEPRECATED);
if (4 < func_num_args() ? !is_bool($test) : null !== $error && @filesize($path) === $error) {
@trigger_error(sprintf('Passing a size as 4th argument to the constructor of "%s" is deprecated since Symfony 4.1 and will be unsupported in 5.0.', __CLASS__), E_USER_DEPRECATED);
$error = $test;
$test = 5 < func_num_args() ? func_get_arg(5) : false;
}
$this->error = $error ?: UPLOAD_ERR_OK;
$this->test = $test;
@ -152,7 +153,7 @@ class UploadedFile extends File
{
@trigger_error(sprintf('"%s" is deprecated since Symfony 4.1 and will be removed in 5.0. Use getSize() instead.', __METHOD__), E_USER_DEPRECATED);
return $this->size;
return $this->getSize();
}
/**

View File

@ -84,7 +84,7 @@ class FileBag extends ParameterBag
if (UPLOAD_ERR_NO_FILE == $file['error']) {
$file = null;
} else {
$file = new UploadedFile($file['tmp_name'], $file['name'], $file['type'], $file['size'], $file['error']);
$file = new UploadedFile($file['tmp_name'], $file['name'], $file['type'], $file['error']);
}
} else {
$file = array_map(array($this, 'convertFileInformation'), $file);

View File

@ -40,7 +40,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
null,
null,
UPLOAD_ERR_OK
);
@ -57,7 +56,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/.unknownextension',
'original.gif',
null,
null,
UPLOAD_ERR_OK
);
@ -70,7 +68,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
null
);
@ -83,7 +80,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/jpeg',
null,
null
);
@ -96,7 +92,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
null
);
@ -109,7 +104,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
null
);
@ -122,7 +116,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
null
);
@ -138,7 +131,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
UPLOAD_ERR_OK
);
@ -158,7 +150,6 @@ class UploadedFileTest extends TestCase
$path,
'original.gif',
'image/gif',
null,
UPLOAD_ERR_OK,
true
);
@ -177,9 +168,7 @@ class UploadedFileTest extends TestCase
$file = new UploadedFile(
__DIR__.'/Fixtures/test.gif',
'../../original.gif',
'image/gif',
null,
null
'image/gif'
);
$this->assertEquals('original.gif', $file->getClientOriginalName());
@ -190,9 +179,7 @@ class UploadedFileTest extends TestCase
$file = new UploadedFile(
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
null,
null
'image/gif'
);
$this->assertEquals(filesize(__DIR__.'/Fixtures/test.gif'), $file->getSize());
@ -208,7 +195,7 @@ class UploadedFileTest extends TestCase
/**
* @group legacy
* @expectedDeprecation Passing a size in the constructor is deprecated since Symfony 4.1 and will be removed in 5.0. Use getSize() instead.
* @expectedDeprecation Passing a size as 4th argument to the constructor of "Symfony\Component\HttpFoundation\File\UploadedFile" is deprecated since Symfony 4.1 and will be unsupported in 5.0.
*/
public function testConstructDeprecatedSize()
{
@ -217,7 +204,24 @@ class UploadedFileTest extends TestCase
'original.gif',
'image/gif',
filesize(__DIR__.'/Fixtures/test.gif'),
null
UPLOAD_ERR_OK,
false
);
$this->assertEquals(filesize(__DIR__.'/Fixtures/test.gif'), $file->getSize());
}
/**
* @group legacy
* @expectedDeprecation Passing a size as 4th argument to the constructor of "Symfony\Component\HttpFoundation\File\UploadedFile" is deprecated since Symfony 4.1 and will be unsupported in 5.0.
*/
public function testConstructDeprecatedSizeWhenPassingOnlyThe4Needed()
{
$file = new UploadedFile(
__DIR__.'/Fixtures/test.gif',
'original.gif',
'image/gif',
filesize(__DIR__.'/Fixtures/test.gif')
);
$this->assertEquals(filesize(__DIR__.'/Fixtures/test.gif'), $file->getSize());
@ -227,8 +231,7 @@ class UploadedFileTest extends TestCase
{
$file = new UploadedFile(
__DIR__.'/Fixtures/test.gif',
'original.gif',
null
'original.gif'
);
$this->assertEquals('gif', $file->getExtension());
@ -240,7 +243,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
null,
null,
UPLOAD_ERR_OK,
true
);
@ -257,7 +259,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
null,
null,
$error
);
@ -281,7 +282,6 @@ class UploadedFileTest extends TestCase
__DIR__.'/Fixtures/test.gif',
'original.gif',
null,
null,
UPLOAD_ERR_OK
);

View File

@ -34,7 +34,7 @@ class FileBagTest extends TestCase
public function testShouldConvertsUploadedFiles()
{
$tmpFile = $this->createTempFile();
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0);
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
$bag = new FileBag(array('file' => array(
'name' => basename($tmpFile),
@ -89,7 +89,7 @@ class FileBagTest extends TestCase
public function testShouldConvertUploadedFilesWithPhpBug()
{
$tmpFile = $this->createTempFile();
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0);
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
$bag = new FileBag(array(
'child' => array(
@ -118,7 +118,7 @@ class FileBagTest extends TestCase
public function testShouldConvertNestedUploadedFilesWithPhpBug()
{
$tmpFile = $this->createTempFile();
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0);
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
$bag = new FileBag(array(
'child' => array(
@ -147,7 +147,7 @@ class FileBagTest extends TestCase
public function testShouldNotConvertNestedUploadedFiles()
{
$tmpFile = $this->createTempFile();
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0);
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
$bag = new FileBag(array('image' => array('file' => $file)));
$files = $bag->all();
@ -156,7 +156,10 @@ class FileBagTest extends TestCase
protected function createTempFile()
{
return tempnam(sys_get_temp_dir().'/form_test', 'FormTest');
$tempFile = tempnam(sys_get_temp_dir().'/form_test', 'FormTest');
file_put_contents($tempFile, '1');
return $tempFile;
}
protected function setUp()

View File

@ -168,7 +168,6 @@ EOF;
'',
$value->getClientOriginalName(),
$value->getClientMimeType(),
null,
UPLOAD_ERR_INI_SIZE,
true
);
@ -177,7 +176,6 @@ EOF;
$value->getPathname(),
$value->getClientOriginalName(),
$value->getClientMimeType(),
null,
$value->getError(),
true
);

View File

@ -97,6 +97,7 @@ class ClientTest extends TestCase
public function testUploadedFile()
{
$source = tempnam(sys_get_temp_dir(), 'source');
file_put_contents($source, '1');
$target = sys_get_temp_dir().'/sf.moved.file';
@unlink($target);
@ -105,7 +106,7 @@ class ClientTest extends TestCase
$files = array(
array('tmp_name' => $source, 'name' => 'original', 'type' => 'mime/original', 'size' => null, 'error' => UPLOAD_ERR_OK),
new UploadedFile($source, 'original', 'mime/original', null, UPLOAD_ERR_OK, true),
new UploadedFile($source, 'original', 'mime/original', UPLOAD_ERR_OK, true),
);
$file = null;
@ -120,7 +121,7 @@ class ClientTest extends TestCase
$this->assertEquals('original', $file->getClientOriginalName());
$this->assertEquals('mime/original', $file->getClientMimeType());
$this->assertTrue($file->isValid());
$this->assertEquals(1, $file->getSize());
}
$file->move(dirname($target), basename($target));
@ -153,15 +154,19 @@ class ClientTest extends TestCase
$file = $this
->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')
->setConstructorArgs(array($source, 'original', 'mime/original', null, UPLOAD_ERR_OK, true))
->setMethods(array('getSize'))
->setConstructorArgs(array($source, 'original', 'mime/original', UPLOAD_ERR_OK, true))
->setMethods(array('getSize', 'getClientSize'))
->getMock()
;
$file->expects($this->once())
/* should be modified when the getClientSize will be removed */
$file->expects($this->any())
->method('getSize')
->will($this->returnValue(INF))
;
$file->expects($this->any())
->method('getClientSize')
->will($this->returnValue(INF))
;
$client->request('POST', '/', array(), array($file));

View File

@ -18,7 +18,7 @@
"require": {
"php": "^7.1.3",
"symfony/event-dispatcher": "~3.4|~4.0",
"symfony/http-foundation": "~3.4.4|~4.0.4",
"symfony/http-foundation": "~4.1",
"symfony/debug": "~3.4|~4.0",
"psr/log": "~1.0"
},

View File

@ -83,7 +83,8 @@ abstract class FileValidatorTest extends ConstraintValidatorTestCase
public function testValidUploadedfile()
{
$file = new UploadedFile($this->path, 'originalName', null, null, null, true);
file_put_contents($this->path, '1');
$file = new UploadedFile($this->path, 'originalName', null, null, true);
$this->validator->validate($file, new File());
$this->assertNoViolation();
@ -411,7 +412,7 @@ abstract class FileValidatorTest extends ConstraintValidatorTestCase
*/
public function testUploadedFileError($error, $message, array $params = array(), $maxSize = null)
{
$file = new UploadedFile('/path/to/file', 'originalName', 'mime', null, $error);
$file = new UploadedFile(tempnam(sys_get_temp_dir(), 'file-validator-test-'), 'originalName', 'mime', $error);
$constraint = new File(array(
$message => 'myMessage',

View File

@ -21,7 +21,7 @@
"symfony/translation": "~3.4|~4.0"
},
"require-dev": {
"symfony/http-foundation": "~3.4|~4.0",
"symfony/http-foundation": "~4.1",
"symfony/http-kernel": "~3.4|~4.0",
"symfony/var-dumper": "~3.4|~4.0",
"symfony/intl": "~3.4|~4.0",