From ac0c00c6e8111e91231412b1c9d9e8a497027c29 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 14 Jun 2011 10:47:04 +0200 Subject: [PATCH 1/4] [HttpFoundation] Make File extends \SplFileInfo --- .../Component/HttpFoundation/File/File.php | 118 ++++------------- .../HttpFoundation/File/UploadedFile.php | 115 ++++++++-------- src/Symfony/Component/HttpKernel/Client.php | 2 +- .../Validator/Constraints/FileValidator.php | 18 +-- .../Form/Extension/Core/Type/FileTypeTest.php | 28 ++-- .../HttpFoundation/File/FileTest.php | 125 +++++------------- .../HttpFoundation/File/UploadedFileTest.php | 62 +++++++-- .../Constraints/FileValidatorTest.php | 109 +++++++++------ 8 files changed, 268 insertions(+), 309 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/File/File.php b/src/Symfony/Component/HttpFoundation/File/File.php index 33bfcdd961..bb6fda2a50 100644 --- a/src/Symfony/Component/HttpFoundation/File/File.php +++ b/src/Symfony/Component/HttpFoundation/File/File.php @@ -20,7 +20,7 @@ use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser; * * @author Bernhard Schussek */ -class File +class File extends \SplFileInfo { /** * A map of mime types and their default extensions. @@ -440,13 +440,6 @@ class File 'x-world/x-vrml' => 'wrl', ); - /** - * The absolute path to the file without dots. - * - * @var string - */ - protected $path; - /** * Constructs a new file from the given path. * @@ -460,41 +453,7 @@ class File throw new FileNotFoundException($path); } - $this->path = realpath($path); - } - - /** - * Alias for getPath(). - * - * @return string - */ - public function __toString() - { - return (string) $this->path; - } - - /** - * Returns the file name. - * - * @return string - */ - public function getName() - { - return basename($this->path); - } - - /** - * Returns the file extension. - * - * @return string - */ - public function getExtension() - { - if ($ext = pathinfo($this->getName(), PATHINFO_EXTENSION)) { - return $ext; - } - - return ''; + parent::__construct($path); } /** @@ -508,31 +467,7 @@ class File { $type = $this->getMimeType(); - if (isset(self::$defaultExtensions[$type])) { - return self::$defaultExtensions[$type]; - } - - return null; - } - - /** - * Returns the directory of the file. - * - * @return string - */ - public function getDirectory() - { - return dirname($this->path); - } - - /** - * Returns the absolute file path, without dots. - * - * @return string - */ - public function getPath() - { - return $this->path; + return isset(static::$defaultExtensions[$type]) ? static::$defaultExtensions[$type] : null; } /** @@ -542,28 +477,13 @@ class File * and the system binary "file" (in this order), depending on which of those * is available on the current operating system. * - * @return string The guessed mime type (i.e. "application/pdf") + * @return string|null The guessed mime type (i.e. "application/pdf") */ public function getMimeType() { $guesser = MimeTypeGuesser::getInstance(); - return $guesser->guess($this->getPath()); - } - - /** - * Returns the size of this file. - * - * @return integer The file size in bytes - */ - public function getSize() - { - if (false === $size = @filesize($this->getPath())) { - $error = error_get_last(); - throw new FileException(sprintf('Could not read file size of %s (%s)', $this->getPath(), strip_tags($error['message']))); - } - - return $size; + return $guesser->guess($this->getPathname()); } /** @@ -571,16 +491,30 @@ class File * * @param string $directory The destination folder * @param string $name The new file name + * + * @return File A File object representing the new file + * + * @throws FileException if the target file could not be created */ public function move($directory, $name = null) { - $newPath = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getName() : $name); - - if (!@rename($this->getPath(), $newPath)) { - $error = error_get_last(); - throw new FileException(sprintf('Could not move file %s to %s (%s)', $this->getPath(), $newPath, strip_tags($error['message']))); + if (!is_dir($directory)) { + if (false === @mkdir($directory, 0777, true)) { + throw new FileException(sprintf('Unable to create the "%s" directory', $directory)); + } + } elseif (!is_writable($directory)) { + throw new FileException(sprintf('Unable to write in the "%s" directory', $directory)); } - $this->path = realpath($newPath); + $target = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getBasename() : basename($name)); + + if (!@rename($this->getPathname(), $target)) { + $error = error_get_last(); + throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); + } + + chmod($target, 0666); + + return new File($target); } -} +} \ No newline at end of file diff --git a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php index fe9449ce74..af3879a444 100644 --- a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php +++ b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php @@ -23,33 +23,42 @@ use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; */ class UploadedFile extends File { + /** + * Whether the test mode is activated. + * + * Local files are used in test mode hence the code should not enforce HTTP uploads. + * + * @var Boolean + */ + private $test = false; + /** * The original name of the uploaded file. * * @var string */ - protected $originalName; + private $originalName; /** * The mime type provided by the uploader. * * @var string */ - protected $mimeType; + private $mimeType; /** * The file size provided by the uploader. * - * @var integer + * @var string */ - protected $size; + private $size; /** * The UPLOAD_ERR_XXX constant provided by the uploader. * * @var integer */ - protected $error; + private $error; /** * Accepts the information of the uploaded file as provided by the PHP global $_FILES. @@ -59,69 +68,65 @@ class UploadedFile extends File * @param string $mimeType The type of the file as provided by PHP * @param integer $size The file size * @param integer $error The error constant of the upload (one of PHP's UPLOAD_ERR_XXX constants) + * @param Boolean $test Whether the test mode is active * * @throws FileException If file_uploads is disabled * @throws FileNotFoundException If the file does not exist */ - public function __construct($path, $originalName, $mimeType = null, $size = null, $error = null) + public function __construct($path, $originalName, $mimeType = null, $size = null, $error = null, $test = false) { if (!ini_get('file_uploads')) { throw new FileException(sprintf('Unable to create UploadedFile because "file_uploads" is disabled in your php.ini file (%s)', get_cfg_var('cfg_file_path'))); } - if (!is_file($path)) { - throw new FileNotFoundException($path); - } - - $this->path = realpath($path); $this->originalName = basename($originalName); $this->mimeType = $mimeType ?: 'application/octet-stream'; $this->size = $size; $this->error = $error ?: UPLOAD_ERR_OK; + $this->test = (Boolean) $test; + + parent::__construct($path); } /** - * {@inheritDoc} - */ - public function getMimeType() - { - return parent::getMimeType() ?: $this->mimeType; - } - - /** - * {@inheritDoc} - */ - public function getSize() - { - return null === $this->size ? parent::getSize() : $this->size; - } - - /** - * {@inheritDoc} - */ - public function getExtension() - { - if ($ext = pathinfo($this->getOriginalName(), PATHINFO_EXTENSION)) { - return $ext; - } - - return ''; - } - - /** - * Gets the original uploaded name. + * Returns the original file name. * - * Warning: This name is not safe as it can have been manipulated by the end-user. - * Moreover, it can contain characters that are not allowed in file names. - * Never use it in a path. + * It is extracted from the request from which the file has been uploaded. + * Then is should not be considered as a safe value. * - * @return string + * @return string|null The original name */ - public function getOriginalName() + public function getClientOriginalName() { return $this->originalName; } + /** + * Returns the file mime type. + * + * It is extracted from the request from which the file has been uploaded. + * Then is should not be considered as a safe value. + * + * @return string|null The mime type + */ + public function getClientMimeType() + { + return $this->mimeType; + } + + /** + * Returns the file size. + * + * It is extracted from the request from which the file has been uploaded. + * Then is should not be considered as a safe value. + * + * @return integer|null The file size + */ + public function getClientSize() + { + return $this->size; + } + /** * Returns the upload error. * @@ -146,17 +151,21 @@ class UploadedFile extends File } /** - * {@inheritDoc} + * Moves the file to a new location. + * + * @param string $directory The destination folder + * @param string $name The new file name + * + * @return File A File object representing the new file + * + * @throws FileException if the file has not been uploaded via Http */ public function move($directory, $name = null) { - $newPath = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getName() : $name); - - if (!@move_uploaded_file($this->getPath(), $newPath)) { - $error = error_get_last(); - throw new FileException(sprintf('Could not move file %s to %s (%s)', $this->getPath(), $newPath, strip_tags($error['message']))); + if (!$this->test && !is_uploaded_file($this->getPathname())) { + throw new FileException(sprintf('The file "%s" has not been uploaded via Http', $this->getPathname())); } - $this->path = realpath($newPath); + return parent::move($directory, $name); } -} +} \ No newline at end of file diff --git a/src/Symfony/Component/HttpKernel/Client.php b/src/Symfony/Component/HttpKernel/Client.php index 1dfcdd5d5c..b67cd91b2e 100644 --- a/src/Symfony/Component/HttpKernel/Client.php +++ b/src/Symfony/Component/HttpKernel/Client.php @@ -120,7 +120,7 @@ EOF; if (is_array($value)) { $filtered[$key] = $this->filterFiles($value); } elseif ($value instanceof UploadedFile) { - // create an already-moved uploaded file + // Create a test mode UploadedFile $filtered[$key] = new UploadedFile($value->getPath(), $value->getName(), $value->getMimeType(), $value->getSize(), $value->getError(), true); } else { $filtered[$key] = $value; diff --git a/src/Symfony/Component/Validator/Constraints/FileValidator.php b/src/Symfony/Component/Validator/Constraints/FileValidator.php index ba2ee30eae..2e156f4c3c 100644 --- a/src/Symfony/Component/Validator/Constraints/FileValidator.php +++ b/src/Symfony/Component/Validator/Constraints/FileValidator.php @@ -29,11 +29,7 @@ class FileValidator extends ConstraintValidator throw new UnexpectedTypeException($value, 'string'); } - if ($value instanceof FileObject && null === $value->getPath()) { - return true; - } - - $path = $value instanceof FileObject ? $value->getPath() : (string) $value; + $path = $value instanceof FileObject ? $value->getPathname() : (string) $value; if (!file_exists($path)) { $this->setMessage($constraint->notFoundMessage, array('{{ file }}' => $path)); @@ -66,9 +62,9 @@ class FileValidator extends ConstraintValidator if ($size > $limit) { $this->setMessage($constraint->maxSizeMessage, array( - '{{ size }}' => $size . $suffix, - '{{ limit }}' => $limit . $suffix, - '{{ file }}' => $path, + '{{ size }}' => $size . $suffix, + '{{ limit }}' => $limit . $suffix, + '{{ file }}' => $path, )); return false; @@ -82,9 +78,9 @@ class FileValidator extends ConstraintValidator if (!in_array($value->getMimeType(), (array) $constraint->mimeTypes)) { $this->setMessage($constraint->mimeTypesMessage, array( - '{{ type }}' => '"'.$value->getMimeType().'"', - '{{ types }}' => '"'.implode('", "', (array) $constraint->mimeTypes).'"', - '{{ file }}' => $path, + '{{ type }}' => '"'.$value->getMimeType().'"', + '{{ types }}' => '"'.implode('", "', (array) $constraint->mimeTypes).'"', + '{{ file }}' => $path, )); return false; diff --git a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FileTypeTest.php b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FileTypeTest.php index 87f46f7819..4643021d13 100644 --- a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FileTypeTest.php +++ b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FileTypeTest.php @@ -30,18 +30,26 @@ class FileTypeTest extends TypeTestCase private function createUploadedFileMock($name, $originalName, $valid) { - $file = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile') + $file = $this + ->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile') ->disableOriginalConstructor() - ->getMock(); - $file->expects($this->any()) - ->method('getName') - ->will($this->returnValue($name)); - $file->expects($this->any()) - ->method('getOriginalName') - ->will($this->returnValue($originalName)); - $file->expects($this->any()) + ->getMock() + ; + $file + ->expects($this->any()) + ->method('getBasename') + ->will($this->returnValue($name)) + ; + $file + ->expects($this->any()) + ->method('getClientOriginalName') + ->will($this->returnValue($originalName)) + ; + $file + ->expects($this->any()) ->method('isValid') - ->will($this->returnValue($valid)); + ->will($this->returnValue($valid)) + ; return $file; } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php b/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php index 21db21c0cc..0a5fe85548 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php @@ -19,49 +19,15 @@ class FileTest extends \PHPUnit_Framework_TestCase { protected $file; - protected function setUp() - { - $this->file = new File(__DIR__.'/Fixtures/test.gif'); - } - - public function testGetPathReturnsAbsolutePath() - { - $this->assertEquals(__DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'test.gif', $this->file->getPath()); - } - - public function test__toString() - { - $this->assertEquals(__DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'test.gif', (string) $this->file); - } - - public function testGetNameReturnsNameWithExtension() - { - $this->assertEquals('test.gif', $this->file->getName()); - } - - public function testGetExtensionReturnsEmptyString() - { - $file = new File(__DIR__.'/Fixtures/test'); - $this->assertEquals('', $file->getExtension()); - } - - public function testGetExtensionReturnsExtensionWithDot() - { - $this->assertEquals('gif', $this->file->getExtension()); - } - - public function testGetDirectoryReturnsDirectoryName() - { - $this->assertEquals(__DIR__.DIRECTORY_SEPARATOR.'Fixtures', $this->file->getDirectory()); - } public function testGetMimeTypeUsesMimeTypeGuessers() { - $guesser = $this->createMockGuesser($this->file->getPath(), 'image/gif'); + $file = new File(__DIR__.'/Fixtures/test.gif'); + $guesser = $this->createMockGuesser($file->getPathname(), 'image/gif'); MimeTypeGuesser::getInstance()->register($guesser); - $this->assertEquals('image/gif', $this->file->getMimeType()); + $this->assertEquals('image/gif', $file->getMimeType()); } public function testGuessExtensionWithoutGuesser() @@ -74,7 +40,7 @@ class FileTest extends \PHPUnit_Framework_TestCase public function testGuessExtensionIsBasedOnMimeType() { $file = new File(__DIR__.'/Fixtures/test'); - $guesser = $this->createMockGuesser($file->getPath(), 'image/gif'); + $guesser = $this->createMockGuesser($file->getPathname(), 'image/gif'); MimeTypeGuesser::getInstance()->register($guesser); @@ -88,100 +54,77 @@ class FileTest extends \PHPUnit_Framework_TestCase new File(__DIR__.'/Fixtures/not_here'); } - public function testSizeReturnsFileSize() - { - $this->assertEquals(filesize($this->file->getPath()), $this->file->getSize()); - } - - public function testSizeFailing() - { - $dir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'directory'; - $path = $dir.DIRECTORY_SEPARATOR.'test.copy.gif'; - @unlink($path); - copy(__DIR__.'/Fixtures/test.gif', $path); - - $file = new File($path); - @unlink($path); - - try { - $file->getSize(); - $this->fail('File::getSize should throw an exception.'); - } catch (FileException $e) { - } - } - public function testMove() { $path = __DIR__.'/Fixtures/test.copy.gif'; - $targetDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'directory'; - $targetPath = $targetDir.DIRECTORY_SEPARATOR.'test.copy.gif'; + $targetDir = __DIR__.'/Fixtures/directory'; + $targetPath = $targetDir.'/test.copy.gif'; @unlink($path); @unlink($targetPath); copy(__DIR__.'/Fixtures/test.gif', $path); $file = new File($path); - $file->move($targetDir); + $movedFile = $file->move($targetDir); + $this->assertInstanceOf('Symfony\Component\HttpFoundation\File\File', $movedFile); $this->assertTrue(file_exists($targetPath)); $this->assertFalse(file_exists($path)); - $this->assertEquals($targetPath, $file->getPath()); + $this->assertEquals(realpath($targetPath), $movedFile->getRealPath()); - @unlink($path); @unlink($targetPath); } public function testMoveWithNewName() { $path = __DIR__.'/Fixtures/test.copy.gif'; - $targetDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'directory'; - $targetPath = $targetDir.DIRECTORY_SEPARATOR.'test.newname.gif'; + $targetDir = __DIR__.'/Fixtures/directory'; + $targetPath = $targetDir.'/test.newname.gif'; @unlink($path); @unlink($targetPath); copy(__DIR__.'/Fixtures/test.gif', $path); $file = new File($path); - $file->move($targetDir, 'test.newname.gif'); + $movedFile = $file->move($targetDir, 'test.newname.gif'); $this->assertTrue(file_exists($targetPath)); $this->assertFalse(file_exists($path)); - $this->assertEquals($targetPath, $file->getPath()); + $this->assertEquals(realpath($targetPath), $movedFile->getRealPath()); - @unlink($path); @unlink($targetPath); } - public function testMoveFailing() + public function testMoveToAnUnexistentDirectory() { - $path = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'test.copy.gif'; - $targetPath = '/thisfolderwontexist'; - @unlink($path); + $sourcePath = __DIR__.'/Fixtures/test.copy.gif'; + $targetDir = __DIR__.'/Fixtures/directory/sub'; + $targetPath = $targetDir.'/test.copy.gif'; + @unlink($sourcePath); @unlink($targetPath); - copy(__DIR__.'/Fixtures/test.gif', $path); + @rmdir($targetDir); + copy(__DIR__.'/Fixtures/test.gif', $sourcePath); - $file = new File($path); + $file = new File($sourcePath); + $movedFile = $file->move($targetDir); - try { - $file->move($targetPath); - $this->fail('File::move should throw an exception.'); - } catch (FileException $e) { - } + $this->assertFileExists($targetPath); + $this->assertFileNotExists($sourcePath); + $this->assertEquals(realpath($targetPath), $movedFile->getRealPath()); - $this->assertFileExists($path); - $this->assertFileNotExists($path.$targetPath.'test.gif'); - $this->assertEquals($path, $file->getPath()); - - @unlink($path); + @unlink($sourcePath); @unlink($targetPath); + @rmdir($targetDir); } protected function createMockGuesser($path, $mimeType) { $guesser = $this->getMock('Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface'); - $guesser->expects($this->once()) - ->method('guess') - ->with($this->equalTo($path)) - ->will($this->returnValue($mimeType)); + $guesser + ->expects($this->once()) + ->method('guess') + ->with($this->equalTo($path)) + ->will($this->returnValue($mimeType)) + ; return $guesser; } -} +} \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/HttpFoundation/File/UploadedFileTest.php b/tests/Symfony/Tests/Component/HttpFoundation/File/UploadedFileTest.php index cbd133d155..f196b2d766 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/File/UploadedFileTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/File/UploadedFileTest.php @@ -32,11 +32,10 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase UPLOAD_ERR_OK ); - $this->assertAttributeEquals('application/octet-stream', 'mimeType', $file); + $this->assertEquals('application/octet-stream', $file->getClientMimeType()); + if (extension_loaded('fileinfo')) { $this->assertEquals('image/gif', $file->getMimeType()); - } else { - $this->assertEquals('application/octet-stream', $file->getMimeType()); } } @@ -50,8 +49,7 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase UPLOAD_ERR_OK ); - $this->assertAttributeEquals('application/octet-stream', 'mimeType', $file); - $this->assertEquals('application/octet-stream', $file->getMimeType()); + $this->assertEquals('application/octet-stream', $file->getClientMimeType()); } public function testErrorIsOkByDefault() @@ -67,7 +65,7 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase $this->assertEquals(UPLOAD_ERR_OK, $file->getError()); } - public function testGetOriginalName() + public function testGetClientOriginalName() { $file = new UploadedFile( __DIR__.'/Fixtures/test.gif', @@ -77,10 +75,54 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase null ); - $this->assertEquals('original.gif', $file->getOriginalName()); + $this->assertEquals('original.gif', $file->getClientOriginalName()); } - public function testGetOriginalNameSanitizeFilename() + /** + * @expectedException Symfony\Component\HttpFoundation\File\Exception\FileException + */ + public function testMoveLocalFileIsNotAllowed() + { + $file = new UploadedFile( + __DIR__.'/Fixtures/test.gif', + 'original.gif', + 'image/gif', + filesize(__DIR__.'/Fixtures/test.gif'), + UPLOAD_ERR_OK + ); + + $movedFile = $file->move(__DIR__.'/Fixtures/directory'); + } + + public function testMoveLocalFileIsAllowedInTestMode() + { + $path = __DIR__.'/Fixtures/test.copy.gif'; + $targetDir = __DIR__.'/Fixtures/directory'; + $targetPath = $targetDir.'/test.copy.gif'; + @unlink($path); + @unlink($targetPath); + copy(__DIR__.'/Fixtures/test.gif', $path); + + $file = new UploadedFile( + $path, + 'original.gif', + 'image/gif', + filesize($path), + UPLOAD_ERR_OK, + true + ); + + $movedFile = $file->move(__DIR__.'/Fixtures/directory'); + + $this->assertTrue(file_exists($targetPath)); + $this->assertFalse(file_exists($path)); + $this->assertEquals(realpath($targetPath), $movedFile->getRealPath()); + + @unlink($targetPath); + } + + + public function testGetClientOriginalNameSanitizeFilename() { $file = new UploadedFile( __DIR__.'/Fixtures/test.gif', @@ -90,6 +132,6 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase null ); - $this->assertEquals('original.gif', $file->getOriginalName()); + $this->assertEquals('original.gif', $file->getClientOriginalName()); } -} +} \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/Validator/Constraints/FileValidatorTest.php b/tests/Symfony/Tests/Component/Validator/Constraints/FileValidatorTest.php index 5540daac9a..f56d645b6b 100644 --- a/tests/Symfony/Tests/Component/Validator/Constraints/FileValidatorTest.php +++ b/tests/Symfony/Tests/Component/Validator/Constraints/FileValidatorTest.php @@ -13,6 +13,7 @@ namespace Symfony\Tests\Component\Validator\Constraints; use Symfony\Component\Validator\Constraints\File; use Symfony\Component\Validator\Constraints\FileValidator; +use Symfony\Component\HttpFoundation\File\File as FileObject; class FileValidatorTest extends \PHPUnit_Framework_TestCase { @@ -42,10 +43,11 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase $this->assertTrue($this->validator->isValid('', new File())); } + /** + * @expectedException Symfony\Component\Validator\Exception\UnexpectedTypeException + */ public function testExpectsStringCompatibleTypeOrFile() { - $this->setExpectedException('Symfony\Component\Validator\Exception\UnexpectedTypeException'); - $this->validator->isValid(new \stdClass(), new File()); } @@ -59,16 +61,16 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase fwrite($this->file, str_repeat('0', 11)); $constraint = new File(array( - 'maxSize' => 10, - 'maxSizeMessage' => 'myMessage', + 'maxSize' => 10, + 'maxSizeMessage' => 'myMessage', )); - $this->assertFalse($this->validator->isValid($this->path, $constraint)); + $this->assertFileValid($this->path, $constraint, false); $this->assertEquals($this->validator->getMessageTemplate(), 'myMessage'); $this->assertEquals($this->validator->getMessageParameters(), array( - '{{ limit }}' => '10 bytes', - '{{ size }}' => '11 bytes', - '{{ file }}' => $this->path, + '{{ limit }}' => '10 bytes', + '{{ size }}' => '11 bytes', + '{{ file }}' => $this->path, )); } @@ -77,16 +79,16 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase fwrite($this->file, str_repeat('0', 1400)); $constraint = new File(array( - 'maxSize' => '1k', - 'maxSizeMessage' => 'myMessage', + 'maxSize' => '1k', + 'maxSizeMessage' => 'myMessage', )); - $this->assertFalse($this->validator->isValid($this->path, $constraint)); + $this->assertFileValid($this->path, $constraint, false); $this->assertEquals($this->validator->getMessageTemplate(), 'myMessage'); $this->assertEquals($this->validator->getMessageParameters(), array( - '{{ limit }}' => '1 kB', - '{{ size }}' => '1.4 kB', - '{{ file }}' => $this->path, + '{{ limit }}' => '1 kB', + '{{ size }}' => '1.4 kB', + '{{ file }}' => $this->path, )); } @@ -95,27 +97,28 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase fwrite($this->file, str_repeat('0', 1400000)); $constraint = new File(array( - 'maxSize' => '1M', - 'maxSizeMessage' => 'myMessage', + 'maxSize' => '1M', + 'maxSizeMessage' => 'myMessage', )); - $this->assertFalse($this->validator->isValid($this->path, $constraint)); + $this->assertFileValid($this->path, $constraint, false); $this->assertEquals($this->validator->getMessageTemplate(), 'myMessage'); $this->assertEquals($this->validator->getMessageParameters(), array( - '{{ limit }}' => '1 MB', - '{{ size }}' => '1.4 MB', - '{{ file }}' => $this->path, + '{{ limit }}' => '1 MB', + '{{ size }}' => '1.4 MB', + '{{ file }}' => $this->path, )); } + /** + * @expectedException Symfony\Component\Validator\Exception\ConstraintDefinitionException + */ public function testInvalidMaxSize() { $constraint = new File(array( 'maxSize' => '1abc', )); - $this->setExpectedException('Symfony\Component\Validator\Exception\ConstraintDefinitionException'); - $this->validator->isValid($this->path, $constraint); } @@ -125,7 +128,7 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase 'notFoundMessage' => 'myMessage', )); - $this->assertFalse($this->validator->isValid('foobar', $constraint)); + $this->assertFileValid('foobar', $constraint, false); $this->assertEquals($this->validator->getMessageTemplate(), 'myMessage'); $this->assertEquals($this->validator->getMessageParameters(), array( '{{ file }}' => 'foobar', @@ -134,13 +137,21 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase public function testValidMimeType() { - $file = $this->getMock('Symfony\Component\HttpFoundation\File\File', array(), array(), '', false); - $file->expects($this->any()) - ->method('getPath') - ->will($this->returnValue($this->path)); - $file->expects($this->any()) - ->method('getMimeType') - ->will($this->returnValue('image/jpg')); + $file = $this + ->getMockBuilder('Symfony\Component\HttpFoundation\File\File') + ->disableOriginalConstructor() + ->getMock() + ; + $file + ->expects($this->once()) + ->method('getPathname') + ->will($this->returnValue($this->path)) + ; + $file + ->expects($this->once()) + ->method('getMimeType') + ->will($this->returnValue('image/jpg')) + ; $constraint = new File(array( 'mimeTypes' => array('image/png', 'image/jpg'), @@ -151,13 +162,21 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase public function testInvalidMimeType() { - $file = $this->getMock('Symfony\Component\HttpFoundation\File\File', array(), array(), '', false); - $file->expects($this->any()) - ->method('getPath') - ->will($this->returnValue($this->path)); - $file->expects($this->any()) - ->method('getMimeType') - ->will($this->returnValue('application/pdf')); + $file = $this + ->getMockBuilder('Symfony\Component\HttpFoundation\File\File') + ->disableOriginalConstructor() + ->getMock() + ; + $file + ->expects($this->once()) + ->method('getPathname') + ->will($this->returnValue($this->path)) + ; + $file + ->expects($this->exactly(2)) + ->method('getMimeType') + ->will($this->returnValue('application/pdf')) + ; $constraint = new File(array( 'mimeTypes' => array('image/png', 'image/jpg'), @@ -167,9 +186,17 @@ class FileValidatorTest extends \PHPUnit_Framework_TestCase $this->assertFalse($this->validator->isValid($file, $constraint)); $this->assertEquals($this->validator->getMessageTemplate(), 'myMessage'); $this->assertEquals($this->validator->getMessageParameters(), array( - '{{ type }}' => '"application/pdf"', - '{{ types }}' => '"image/png", "image/jpg"', - '{{ file }}' => $this->path, + '{{ type }}' => '"application/pdf"', + '{{ types }}' => '"image/png", "image/jpg"', + '{{ file }}' => $this->path, )); } -} + + protected function assertFileValid($filename, File $constraint, $valid = true) + { + $this->assertEquals($this->validator->isValid($filename, $constraint), $valid); + if (file_exists($filename)) { + $this->assertEquals($this->validator->isValid(new FileObject($filename), $constraint), $valid); + } + } +} \ No newline at end of file From 38b3b7474f52d49eab81e0f78a4634b7d84203b1 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 14 Jun 2011 11:54:55 +0200 Subject: [PATCH 2/4] [HttpKernel] Fix and test previous commit --- src/Symfony/Component/HttpKernel/Client.php | 9 +++++- .../Tests/Component/HttpKernel/ClientTest.php | 29 +++++++++++++++++++ .../Component/HttpKernel/TestHttpKernel.php | 3 ++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpKernel/Client.php b/src/Symfony/Component/HttpKernel/Client.php index b67cd91b2e..adb9f487b3 100644 --- a/src/Symfony/Component/HttpKernel/Client.php +++ b/src/Symfony/Component/HttpKernel/Client.php @@ -121,7 +121,14 @@ EOF; $filtered[$key] = $this->filterFiles($value); } elseif ($value instanceof UploadedFile) { // Create a test mode UploadedFile - $filtered[$key] = new UploadedFile($value->getPath(), $value->getName(), $value->getMimeType(), $value->getSize(), $value->getError(), true); + $filtered[$key] = new UploadedFile( + $value->getPathname(), + $value->getClientOriginalName(), + $value->getClientMimeType(), + $value->getClientSize(), + $value->getError(), + true + ); } else { $filtered[$key] = $value; } diff --git a/tests/Symfony/Tests/Component/HttpKernel/ClientTest.php b/tests/Symfony/Tests/Component/HttpKernel/ClientTest.php index 2143891533..a9a5145617 100644 --- a/tests/Symfony/Tests/Component/HttpKernel/ClientTest.php +++ b/tests/Symfony/Tests/Component/HttpKernel/ClientTest.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\HttpKernel; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Cookie; +use Symfony\Component\HttpFoundation\File\UploadedFile; require_once __DIR__.'/TestHttpKernel.php'; @@ -76,4 +77,32 @@ class ClientTest extends \PHPUnit_Framework_TestCase $domResponse = $m->invoke($client, $response); $this->assertEquals('foo=bar; expires=Sun, 15-Feb-2009 20:00:00 GMT; domain=http://example.com; path=/foo; secure; httponly, foo1=bar1; expires=Sun, 15-Feb-2009 20:00:00 GMT; domain=http://example.com; path=/foo; secure; httponly', $domResponse->getHeader('Set-Cookie')); } + + public function testUploadedFile() + { + $source = tempnam(sys_get_temp_dir(), 'source'); + $target = sys_get_temp_dir().'/sf.moved.file'; + @unlink($target); + + $kernel = new TestHttpKernel(); + $client = new Client($kernel); + + $client->request('POST', '/', array(), array(new UploadedFile($source, 'original', 'mime/original', 123, UPLOAD_ERR_OK))); + + $files = $kernel->request->files->all(); + + $this->assertEquals(1, count($files)); + + $file = $files[0]; + + $this->assertEquals('original', $file->getClientOriginalName()); + $this->assertEquals('mime/original', $file->getClientMimeType()); + $this->assertEquals('123', $file->getClientSize()); + $this->assertTrue($file->isValid()); + + $file->move(dirname($target), basename($target)); + + $this->assertFileExists($target); + unlink($target); + } } diff --git a/tests/Symfony/Tests/Component/HttpKernel/TestHttpKernel.php b/tests/Symfony/Tests/Component/HttpKernel/TestHttpKernel.php index 35eda847da..05db32331c 100644 --- a/tests/Symfony/Tests/Component/HttpKernel/TestHttpKernel.php +++ b/tests/Symfony/Tests/Component/HttpKernel/TestHttpKernel.php @@ -19,6 +19,8 @@ use Symfony\Component\EventDispatcher\EventDispatcher; class TestHttpKernel extends HttpKernel implements ControllerResolverInterface { + public $request; + public function __construct() { parent::__construct(new EventDispatcher(), $this); @@ -36,6 +38,7 @@ class TestHttpKernel extends HttpKernel implements ControllerResolverInterface public function callController(Request $request) { + $this->request = $request; return new Response('Request: '.$request->getRequestUri()); } } From 136b80ae63a16198071030c2147f22d3b6fffe76 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 14 Jun 2011 18:18:43 +0200 Subject: [PATCH 3/4] [HttFoundation] Add File::getExtension() as \SplFileInfo::getExtension() was introduced in PHP 5.3.6 --- src/Symfony/Component/HttpFoundation/File/File.php | 12 ++++++++++++ .../Tests/Component/HttpFoundation/File/FileTest.php | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/src/Symfony/Component/HttpFoundation/File/File.php b/src/Symfony/Component/HttpFoundation/File/File.php index bb6fda2a50..a6fa665331 100644 --- a/src/Symfony/Component/HttpFoundation/File/File.php +++ b/src/Symfony/Component/HttpFoundation/File/File.php @@ -486,6 +486,18 @@ class File extends \SplFileInfo return $guesser->guess($this->getPathname()); } + /** + * Returns the extension of the file. + * + * \SplFileInfo::getExtension() is not available before PHP 5.3.6 + * + * @return string The extension + */ + public function getExtension() + { + return pathinfo($this->getBasename(), PATHINFO_EXTENSION); + } + /** * Moves the file to a new location. * diff --git a/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php b/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php index 0a5fe85548..8411020528 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php @@ -115,6 +115,12 @@ class FileTest extends \PHPUnit_Framework_TestCase @rmdir($targetDir); } + public function testGetExtension() + { + $file = new File(__DIR__.'/Fixtures/test.gif'); + $this->assertEquals('gif', $file->getExtension()); + } + protected function createMockGuesser($path, $mimeType) { $guesser = $this->getMock('Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface'); From 9d6357c35b9e38eeab303a9b6bff045bf2ab27a7 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 14 Jun 2011 18:21:04 +0200 Subject: [PATCH 4/4] [HttpFoundation] Document the changes to the File classes --- UPDATE.md | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/UPDATE.md b/UPDATE.md index 5a3f08ecce..c2f464d472 100644 --- a/UPDATE.md +++ b/UPDATE.md @@ -9,6 +9,36 @@ timeline closely anyway. beta4 to beta5 -------------- +* The File classes from `HttpFoundation` have been refactored: + + * `Symfony\Component\HttpFoundation\File\File` has a new API: + + * It now extends `\splFileInfo`: + + * former `getName()` equivalent is `getBasename()`, + * former `getDirectory()` equivalent is `getPath()`, + * former `getPath()` equivalent is `getRealPath()`. + + * the `move()` method now creates the target directory when it does not exist, + + * `getExtension()` and `guessExtension()` do not return the extension + with a leading `.` anymore + + * `Symfony\Component\HttpFoundation\File\UploadedFile` has a new API: + + * The constructor has a new Boolean parameter that must be set to true + in test mode only in order to be able to move the file. This parameter + is not intended to be set to true from outside of the core files. + + * `getMimeType()` now always returns the mime type of the underlying file. + Use `getClientMimeType()` to get the mime type from the request. + + * `getSize()` now always returns the size of the underlying file. + Use `getClientSize()` to get the file size from the request. + + * Use `getClientOriginalName()` to retrieve the original name from the + request. + * The `extensions` setting for Twig has been removed. There is now only one way to register Twig extensions, via the `twig.extension` tag. @@ -28,9 +58,6 @@ beta4 to beta5 * The file input is now rendered as any other input field. -* The `Symfony\Component\HttpFoundation\File\File::getExtension()` and - `guessExtension()` methods do not return the extension with a `.` anymore. - * The `em` option of the Doctrine `EntityType` class now takes the entity manager name instead of the EntityManager instance. If you don't pass this option, the default Entity Manager will be used as before.