[HttpFoundation] refactored File management to be safer

src/Symfony/Component/HttpFoundation/File/File.php

@@ -574,34 +574,7 @@ class File
      */
     public function move($directory, $name = null)
     {
-        $this->doMove($directory, $this->getName());
-
-        if (null !== $name) {
-            $this->rename($name);
-        }
-    }
-
-    /**
-     * Renames the file.
-     *
-     * @param string $name The new file name
-     */
-    public function rename($name)
-    {
-        $this->doMove($this->getDirectory(), $name);
-    }
-
-    /**
-     * Moves the file to a new directory and gives it a new filename
-     *
-     * @param string $directory The new directory
-     * @param string $filename  The new file name
-     *
-     * @throws FileException When the file could not be moved
-     */
-    protected function doMove($directory, $filename)
-    {
-        $newPath = $directory.DIRECTORY_SEPARATOR.$filename;
+        $newPath = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getName() : $name);
 
         if (!@rename($this->getPath(), $newPath)) {
             $error = error_get_last();

src/Symfony/Component/HttpFoundation/File/UploadedFile.php

@@ -19,6 +19,7 @@ use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException;
  *
  * @author Bernhard Schussek <bernhard.schussek@symfony.com>
  * @author Florian Eckerstorfer <florian@eckerstorfer.org>
+ * @author Fabien Potencier <fabien@symfony.com>
  */
 class UploadedFile extends File
 {
@@ -105,13 +106,33 @@ class UploadedFile extends File
     }
 
     /**
-     * Returns the absolute file name without dots.
+     * @inheritDoc
+     */
+    public function getExtension()
+    {
+        if ($this->moved) {
+            return parent::getExtension();
+        }
+
+        if ($ext = pathinfo($this->getOriginalName(), PATHINFO_EXTENSION)) {
+            return '.'.$ext;
+        }
+
+        return '';
+    }
+
+    /**
+     * Gets the original uploaded 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.
      *
-     * @return string The file path
+     * @return string
      */
-    public function getName()
+    public function getOriginalName()
     {
-        return $this->moved ? parent::getName() : $this->originalName;
+        return $this->originalName;
     }
 
     /**
@@ -143,29 +164,14 @@ class UploadedFile extends File
     public function move($directory, $name = null)
     {
         if ($this->moved) {
-            return parent::move($directory, $name);
-        }
-
-        $this->doMove($directory, $this->originalName);
-
-        if (null !== $name) {
-            $this->rename($name);
-        }
-    }
-
-    /**
-     * @inheritDoc
-     */
-    protected function doMove($directory, $filename)
-    {
-        if ($this->moved) {
-            return parent::doMove($directory, $filename);
+            return parent::doMove($directory, $name);
         }
 
-        $newPath = $directory.DIRECTORY_SEPARATOR.$filename;
+        $newPath = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getName() : $name);
 
-        if (!move_uploaded_file($this->getPath(), $newPath)) {
-            throw new FileException(sprintf('Could not move file %s to %s', $this->getPath(), $newPath));
+        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'])));
         }
 
         $this->moved = true;

tests/Symfony/Tests/Component/HttpFoundation/File/FileTest.php

@@ -167,25 +167,6 @@ class FileTest extends \PHPUnit_Framework_TestCase
         @unlink($targetPath);
     }
 
-    public function testRename()
-    {
-        $path = __DIR__.'/Fixtures/test.copy.gif';
-        $targetPath = realpath(__DIR__.'/Fixtures').DIRECTORY_SEPARATOR.'test.target.gif';
-        @unlink($path);
-        @unlink($targetPath);
-        copy(realpath(__DIR__.'/Fixtures/test.gif'), $path);
-
-        $file = new File($path);
-        $file->rename('test.target.gif');
-
-        $this->assertTrue(file_exists($targetPath));
-        $this->assertFalse(file_exists($path));
-        $this->assertEquals($targetPath, $file->getPath());
-
-        @unlink($path);
-        @unlink($targetPath);
-    }
-
     protected function createMockGuesser($path, $mimeType)
     {
         $guesser = $this->getMock('Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface');

tests/Symfony/Tests/Component/HttpFoundation/File/UploadedFileTest.php

@@ -77,6 +77,6 @@ class UploadedFileTest extends \PHPUnit_Framework_TestCase
             null
         );
 
-        $this->assertEquals('original.gif', $file->getName());
+        $this->assertEquals('test.gif', $file->getName());
     }
 }