From 605e1cb925021febf526f9f7395100c4b22f1103 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 26 Mar 2012 01:49:09 +0200 Subject: [PATCH] Clean up in case of download/extraction failure, fixes #356 --- src/Composer/Downloader/ArchiveDownloader.php | 39 +++++++++++-------- src/Composer/Downloader/FileDownloader.php | 38 +++++++++--------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index 8e921a3e4..c275689c1 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/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(''); diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index d2bfc845b..bcf21c153 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/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); } /**