Clean up in case of download/extraction failure, fixes #356

src/Composer/Downloader/ArchiveDownloader.php

@@ -14,7 +14,6 @@ namespace Composer\Downloader;
 
 use Composer\IO\IOInterface;
 use Composer\Package\PackageInterface;
-use Composer\Util\Filesystem;
 
 /**
  * Base downloader for archives
@@ -34,28 +33,34 @@ abstract class ArchiveDownloader extends FileDownloader
 
         $fileName = $this->getFileName($package, $path);
         $this->io->write('    Unpacking archive');
-        $this->extract($fileName, $path);
+        try {
+            $this->extract($fileName, $path);
 
-        $this->io->write('    Cleaning up');
-        unlink($fileName);
+            $this->io->write('    Cleaning up');
+            unlink($fileName);
 
-        // If we have only a one dir inside it suppose to be a package itself
-        $contentDir = glob($path . '/*');
-        if (1 === count($contentDir)) {
-            $contentDir = $contentDir[0];
+            // If we have only a one dir inside it suppose to be a package itself
+            $contentDir = glob($path . '/*');
+            if (1 === count($contentDir)) {
+                $contentDir = $contentDir[0];
 
-            // Rename the content directory to avoid error when moving up
-            // a child folder with the same name
-            $temporaryName = md5(time().rand());
-            rename($contentDir, $temporaryName);
-            $contentDir = $temporaryName;
+                // Rename the content directory to avoid error when moving up
+                // a child folder with the same name
+                $temporaryName = md5(time().rand());
+                rename($contentDir, $temporaryName);
+                $contentDir = $temporaryName;
 
-            foreach (array_merge(glob($contentDir . '/.*'), glob($contentDir . '/*')) as $file) {
-                if (trim(basename($file), '.')) {
-                    rename($file, $path . '/' . basename($file));
+                foreach (array_merge(glob($contentDir . '/.*'), glob($contentDir . '/*')) as $file) {
+                    if (trim(basename($file), '.')) {
+                        rename($file, $path . '/' . basename($file));
+                    }
                 }
+                rmdir($contentDir);
             }
-            rmdir($contentDir);
+        } catch (\Exception $e) {
+            // clean up
+            $this->fs->removeDirectory($path);
+            throw $e;
         }
 
         $this->io->write('');

src/Composer/Downloader/FileDownloader.php

@@ -28,16 +28,18 @@ class FileDownloader implements DownloaderInterface
 {
     protected $io;
     protected $rfs;
+    protected $filesystem;
 
     /**
      * Constructor.
      *
      * @param IOInterface  $io  The IO instance
      */
-    public function __construct(IOInterface $io, RemoteFilesystem $rfs = null)
+    public function __construct(IOInterface $io, RemoteFilesystem $rfs = null, Filesystem $filesystem = null)
     {
         $this->io = $io;
         $this->rfs = $rfs ?: new RemoteFilesystem($io);
+        $this->filesystem = $filesystem ?: new Filesystem();
     }
 
     /**
@@ -58,14 +60,7 @@ class FileDownloader implements DownloaderInterface
             throw new \InvalidArgumentException('The given package is missing url information');
         }
 
-        if (!is_dir($path)) {
-            if (file_exists($path)) {
-                throw new \UnexpectedValueException($path.' exists and is not a directory');
-            }
-            if (!mkdir($path, 0777, true)) {
-                throw new \UnexpectedValueException($path.' does not exist and could not be created');
-            }
-        }
+        $this->filesystem->ensureDirectoryExists($path);
 
         $fileName = $this->getFileName($package, $path);
 
@@ -73,16 +68,22 @@ class FileDownloader implements DownloaderInterface
 
         $url = $this->processUrl($url);
 
-        $this->rfs->copy($package->getSourceUrl(), $url, $fileName);
+        try {
+            $this->rfs->copy($package->getSourceUrl(), $url, $fileName);
 
-        if (!file_exists($fileName)) {
-            throw new \UnexpectedValueException($url.' could not be saved to '.$fileName.', make sure the'
-                .' directory is writable and you have internet connectivity');
-        }
+            if (!file_exists($fileName)) {
+                throw new \UnexpectedValueException($url.' could not be saved to '.$fileName.', make sure the'
+                    .' directory is writable and you have internet connectivity');
+            }
 
-        $checksum = $package->getDistSha1Checksum();
-        if ($checksum && hash_file('sha1', $fileName) !== $checksum) {
-            throw new \UnexpectedValueException('The checksum verification of the file failed (downloaded from '.$url.')');
+            $checksum = $package->getDistSha1Checksum();
+            if ($checksum && hash_file('sha1', $fileName) !== $checksum) {
+                throw new \UnexpectedValueException('The checksum verification of the file failed (downloaded from '.$url.')');
+            }
+        } catch (\Exception $e) {
+            // clean up
+            $this->fs->removeDirectory($path);
+            throw $e;
         }
     }
 
@@ -100,8 +101,7 @@ class FileDownloader implements DownloaderInterface
      */
     public function remove(PackageInterface $package, $path)
     {
-        $fs = new Filesystem();
-        $fs->removeDirectory($path);
+        $this->fs->removeDirectory($path);
     }
 
     /**