From 820186fcacb9c3366351caaaad148017e49cd340 Mon Sep 17 00:00:00 2001 From: Amrouche Hamza Date: Fri, 5 Jan 2018 15:40:54 +0100 Subject: [PATCH] [HttpFoundation] we should not pass size on FileBag --- .../Component/Form/Tests/CompoundFormTest.php | 11 +++-- .../Extension/Core/Type/FileTypeTest.php | 2 +- .../HttpFoundation/File/UploadedFile.php | 15 ++++--- .../Component/HttpFoundation/FileBag.php | 2 +- .../Tests/File/UploadedFileTest.php | 44 +++++++++---------- .../HttpFoundation/Tests/FileBagTest.php | 44 +++++++++++++++++-- src/Symfony/Component/HttpKernel/Client.php | 2 - .../Component/HttpKernel/Tests/ClientTest.php | 21 ++++++--- .../Tests/Constraints/FileValidatorTest.php | 6 ++- 9 files changed, 98 insertions(+), 49 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index 11244fee21..ef0e7d534f 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -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', diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php index e150213071..a7edef0144 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php @@ -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( diff --git a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php index 3f22123777..b311bf6b81 100644 --- a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php +++ b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php @@ -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(); } /** diff --git a/src/Symfony/Component/HttpFoundation/FileBag.php b/src/Symfony/Component/HttpFoundation/FileBag.php index 5edd0e6210..f0d97caca2 100644 --- a/src/Symfony/Component/HttpFoundation/FileBag.php +++ b/src/Symfony/Component/HttpFoundation/FileBag.php @@ -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); diff --git a/src/Symfony/Component/HttpFoundation/Tests/File/UploadedFileTest.php b/src/Symfony/Component/HttpFoundation/Tests/File/UploadedFileTest.php index 17455fe5c9..2c4f29c07a 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/File/UploadedFileTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/File/UploadedFileTest.php @@ -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 ); diff --git a/src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php b/src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php index 4e859a868c..efb4b64f7c 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php @@ -31,10 +31,14 @@ class FileBagTest extends TestCase new FileBag(array('file' => 'foo')); } + /** + * @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 testShouldConvertsUploadedFiles() { $tmpFile = $this->createTempFile(); - $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0); + $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', 0); $bag = new FileBag(array('file' => array( 'name' => basename($tmpFile), @@ -60,6 +64,26 @@ class FileBagTest extends TestCase $this->assertNull($bag->get('file')); } + /** + * @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 testShouldNotTriggerDeprecationWhenPassingSize() + { + $tmpFile = $this->createTempFile(); + $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', 0); + + $bag = new FileBag(array('file' => array( + 'name' => basename($tmpFile), + 'type' => 'text/plain', + 'tmp_name' => $tmpFile, + 'error' => 0, + 'size' => 123456, + ))); + + $this->assertEquals($file, $bag->get('file')); + } + public function testShouldRemoveEmptyUploadedFilesForMultiUpload() { $bag = new FileBag(array('files' => array( @@ -86,10 +110,14 @@ class FileBagTest extends TestCase $this->assertSame(array('file1' => null), $bag->get('files')); } + /** + * @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 testShouldConvertUploadedFilesWithPhpBug() { $tmpFile = $this->createTempFile(); - $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0); + $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', 0); $bag = new FileBag(array( 'child' => array( @@ -115,10 +143,14 @@ class FileBagTest extends TestCase $this->assertEquals($file, $files['child']['file']); } + /** + * @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 testShouldConvertNestedUploadedFilesWithPhpBug() { $tmpFile = $this->createTempFile(); - $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0); + $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', 0); $bag = new FileBag(array( 'child' => array( @@ -144,10 +176,14 @@ class FileBagTest extends TestCase $this->assertEquals($file, $files['child']['sub']['file']); } + /** + * @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 testShouldNotConvertNestedUploadedFiles() { $tmpFile = $this->createTempFile(); - $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', null, 0); + $file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain', 0); $bag = new FileBag(array('image' => array('file' => $file))); $files = $bag->all(); diff --git a/src/Symfony/Component/HttpKernel/Client.php b/src/Symfony/Component/HttpKernel/Client.php index abd84529c9..cc744b398c 100644 --- a/src/Symfony/Component/HttpKernel/Client.php +++ b/src/Symfony/Component/HttpKernel/Client.php @@ -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 ); diff --git a/src/Symfony/Component/HttpKernel/Tests/ClientTest.php b/src/Symfony/Component/HttpKernel/Tests/ClientTest.php index b612ce63c6..38666ddec3 100644 --- a/src/Symfony/Component/HttpKernel/Tests/ClientTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/ClientTest.php @@ -94,9 +94,14 @@ class ClientTest extends TestCase $this->assertEquals('foo', $domResponse->getContent()); } + /** + * @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 testUploadedFile() { $source = tempnam(sys_get_temp_dir(), 'source'); + file_put_contents($source, ''); $target = sys_get_temp_dir().'/sf.moved.file'; @unlink($target); @@ -105,7 +110,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', 0, UPLOAD_ERR_OK, true), ); $file = null; @@ -120,7 +125,7 @@ class ClientTest extends TestCase $this->assertEquals('original', $file->getClientOriginalName()); $this->assertEquals('mime/original', $file->getClientMimeType()); - $this->assertTrue($file->isValid()); + $this->assertEquals($file->getSize(), 0); } $file->move(dirname($target), basename($target)); @@ -153,15 +158,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)); diff --git a/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php index c9574fbcc8..52a207aecd 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php @@ -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,8 @@ 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); + touch('/tmp/file'); + $file = new UploadedFile('/tmp/file', 'originalName', 'mime', $error); $constraint = new File(array( $message => 'myMessage',