From 374ada691417af595e6963fe9d6d5f35e47159f6 Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Mon, 13 Feb 2017 13:43:36 +0100 Subject: [PATCH 1/7] useless if, condition il already true --- src/Composer/Downloader/ZipDownloader.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index 612eb9da2..b84f0d836 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -59,6 +59,12 @@ class ZipDownloader extends ArchiveDownloader return parent::download($package, $path, $output); } + /* + * extract $file to $path + * + * @param string $file File to extract + * @param string $path Path where to extract file + */ protected function extract($file, $path) { $processError = null; @@ -79,9 +85,7 @@ class ZipDownloader extends ArchiveDownloader $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); } - if (!class_exists('ZipArchive')) { - throw new \RuntimeException($processError); - } + throw new \RuntimeException($processError); } $zipArchive = new ZipArchive(); From 211c874b93b1a24a65ad591380e7e6ce859a130b Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Mon, 13 Feb 2017 14:00:48 +0100 Subject: [PATCH 2/7] split into 2 extract methods --- src/Composer/Downloader/ZipDownloader.php | 67 ++++++++++++++++------- 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index b84f0d836..00dd61a2b 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -59,46 +59,73 @@ class ZipDownloader extends ArchiveDownloader return parent::download($package, $path, $output); } - /* - * extract $file to $path + /** + * extract $file to $path with "unzip" command * * @param string $file File to extract * @param string $path Path where to extract file + * @return bool True if succeed */ - protected function extract($file, $path) + protected function extractWithUnzip($file, $path) { $processError = null; + $command = 'unzip -qq '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); + if (!Platform::isWindows()) { + $command .= ' && chmod -R u+w ' . ProcessExecutor::escape($path); + } - if (self::$hasSystemUnzip && !(class_exists('ZipArchive') && Platform::isWindows())) { - $command = 'unzip -qq '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); - if (!Platform::isWindows()) { - $command .= ' && chmod -R u+w ' . ProcessExecutor::escape($path); - } - - try { - if (0 === $this->process->execute($command, $ignoredOutput)) { - return; - } - - $processError = 'Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(); - } catch (\Exception $e) { - $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); + try { + if (0 === $this->process->execute($command, $ignoredOutput)) { + return TRUE; } - throw new \RuntimeException($processError); + $processError = 'Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(); + } catch (\Exception $e) { + $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); } + throw new \RuntimeException($processError); + } + + /** + * extract $file to $path with ZipArchive + * + * @param string $file File to extract + * @param string $path Path where to extract file + * @return bool True if succeed + */ + protected function extractWithZipArchive($file, $path) + { $zipArchive = new ZipArchive(); if (true !== ($retval = $zipArchive->open($file))) { - throw new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n".$processError), $retval); + throw new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n"), $retval); } if (true !== $zipArchive->extractTo($path)) { - throw new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n".$processError)); + throw new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); } $zipArchive->close(); + + return TRUE; + } + + /** + * extract $file to $path + * + * @param string $file File to extract + * @param string $path Path where to extract file + */ + protected function extract($file, $path) + { + if (self::$hasSystemUnzip && !(class_exists('ZipArchive') && Platform::isWindows())) { + if ( $this->extractWithUnzip($file, $path) ) { + return; + } + } + + $this->extractWithZipArchive($file, $path); } /** From 921ffe741f43ea19f6c22f5eba99d2fbd19f0443 Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Mon, 13 Feb 2017 15:54:55 +0100 Subject: [PATCH 3/7] Cleaner fallback Algorithm --- src/Composer/Downloader/ZipDownloader.php | 76 ++++++++++++---- .../Test/Downloader/ZipDownloaderTest.php | 89 ++++++++++++++++++- 2 files changed, 145 insertions(+), 20 deletions(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index 00dd61a2b..cbc96cef4 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -31,6 +31,7 @@ class ZipDownloader extends ArchiveDownloader { protected $process; protected static $hasSystemUnzip; + protected static $hasZipArchive; public function __construct(IOInterface $io, Config $config, EventDispatcher $eventDispatcher = null, Cache $cache = null, ProcessExecutor $process = null, RemoteFilesystem $rfs = null) { @@ -48,7 +49,11 @@ class ZipDownloader extends ArchiveDownloader self::$hasSystemUnzip = (bool) $finder->find('unzip'); } - if (!class_exists('ZipArchive') && !self::$hasSystemUnzip) { + if (null === self::$hasZipArchive) { + self::$hasZipArchive = class_exists('ZipArchive'); + } + + if (!self::$hasZipArchive && !self::$hasSystemUnzip) { // php.ini path is added to the error message to help users find the correct file $iniMessage = IniHelper::getMessage(); $error = "The zip extension and unzip command are both missing, skipping.\n" . $iniMessage; @@ -62,17 +67,18 @@ class ZipDownloader extends ArchiveDownloader /** * extract $file to $path with "unzip" command * - * @param string $file File to extract - * @param string $path Path where to extract file + * @param string $file File to extract + * @param string $path Path where to extract file + * @param bool $isFallback If true it is called as a fallback and should not throw exception * @return bool True if succeed */ - protected function extractWithUnzip($file, $path) + protected function extractWithSystemUnzip($file, $path, $isFallback) { $processError = null; - $command = 'unzip -qq '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); - if (!Platform::isWindows()) { - $command .= ' && chmod -R u+w ' . ProcessExecutor::escape($path); - } + // When called after a ZipArchive failed, perhaps there is some files to overwrite + $overwrite = $isFallback ? '-o' : ''; + + $command = 'unzip -qq '.$overwrite.' '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); try { if (0 === $this->process->execute($command, $ignoredOutput)) { @@ -84,7 +90,11 @@ class ZipDownloader extends ArchiveDownloader $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); } - throw new \RuntimeException($processError); + if ( $isFallback ) { + $this->io->write($processError); + return; + } + return new \RuntimeException($processError); } /** @@ -99,11 +109,18 @@ class ZipDownloader extends ArchiveDownloader $zipArchive = new ZipArchive(); if (true !== ($retval = $zipArchive->open($file))) { - throw new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n"), $retval); + return new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n"), $retval); + } + + $extractResult = FALSE; + try { + $extractResult = $zipArchive->extractTo($path); + } catch (\Exception $e ) { + return $e; } - if (true !== $zipArchive->extractTo($path)) { - throw new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); + if (true !== $extractResult) { + return new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); } $zipArchive->close(); @@ -117,15 +134,42 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file */ - protected function extract($file, $path) + public function extract($file, $path) { - if (self::$hasSystemUnzip && !(class_exists('ZipArchive') && Platform::isWindows())) { - if ( $this->extractWithUnzip($file, $path) ) { + $resultZipArchive = NULL; + $resultUnzip = NULL; + + if ( self::$hasZipArchive ) { + // zip module is present + $resultZipArchive = $this->extractWithZipArchive($file, $path); + if ($resultZipArchive === TRUE) { return; } } - $this->extractWithZipArchive($file, $path); + if ( self::$hasSystemUnzip ) { + // we have unzip in the path + $isFallback=FALSE; + if ( $resultZipArchive !== NULL) { + $this->io->writeError("\nUnzip using ZipArchive failed, trying with unzip"); + $isFallback=TRUE; + }; + $resultUnzip = $this->extractWithSystemUnzip($file, $path, $isFallback); + if ( $resultUnzip === TRUE ) { + return ; + } + }; + + // extract functions return TRUE or an exception + if ( $resultZipArchive !== NULL ) { + // zipArchive failed + // unZip not present or failed too + throw $resultZipArchive; + } else { + // unZip failed + // zipArchive not available + throw $resultUnzip; + }; } /** diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index f70d9e44c..b5ca6a33f 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -25,10 +25,6 @@ class ZipDownloaderTest extends TestCase public function setUp() { - if (!class_exists('ZipArchive')) { - $this->markTestSkipped('zip extension missing'); - } - $this->testDir = $this->getUniqueTmpDirectory(); } @@ -40,6 +36,10 @@ class ZipDownloaderTest extends TestCase public function testErrorMessages() { + if (!class_exists('ZipArchive')) { + $this->markTestSkipped('zip extension missing'); + } + $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) ->method('getDistUrl') @@ -81,4 +81,85 @@ class ZipDownloaderTest extends TestCase $this->assertContains('is not a zip archive', $e->getMessage()); } } + + /** + * @expectedException \Exception + * @expectedExceptionMessage ZipArchive Failed + */ + function testZipArchiveOnlyFailed() { + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $e = new \Exception("ZipArchive Failed"); + $downloader->setUp(TRUE, FALSE, $e, NULL); + $downloader->extract('testfile.zip', 'vendor/dir'); + } + + function testZipArchiveOnlyGood() { + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $downloader->setUp(TRUE, FALSE, TRUE, NULL); + $downloader->extract('testfile.zip', 'vendor/dir'); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage SystemUnzip Failed + */ + function testSystemUnzipOnlyFailed() { + $this->setExpectedException(\Exception::class); + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $e = new \Exception("SystemUnzip Failed"); + $downloader->setUp(FALSE, TRUE, NULL, $e); + $downloader->extract('testfile.zip', 'vendor/dir'); + } + + function testSystemUnzipOnlyGood() { + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $downloader->setUp(FALSE, TRUE, NULL, TRUE); + $downloader->extract('testfile.zip', 'vendor/dir'); + } + + function testSystemUnzipFallbackGood() { + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $e = new \Exception("test"); + $downloader->setUp(TRUE, TRUE, $e, TRUE); + $downloader->extract('testfile.zip', 'vendor/dir'); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage ZipArchive Failed + */ + function testSystemUnzipFallbackFailed() { + $this->setExpectedException(\Exception::class); + $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); + $e1 = new \Exception("ZipArchive Failed"); + $e2 = new \Exception("SystemUnzip Failed"); + $downloader->setUp(TRUE, TRUE, $e1, $e2); + $downloader->extract('testfile.zip', 'vendor/dir'); + } +} + +class TestDownloader extends ZipDownloader { + public function __construct($io) + { + $this->io = $io; + } + + public function extract($file, $path) { + parent::extract($file, $path); + } + + public function setUp($zipArchive, $systemUnzip, $zipArchiveResponse, $systemUnzipResponse) { + self::$hasZipArchive = $zipArchive; + self::$hasSystemUnzip = $systemUnzip; + $this->zipArchiveResponse = $zipArchiveResponse; + $this->systemUnzipResponse = $systemUnzipResponse; + } + + protected function extractWithZipArchive($file, $path) { + return $this->zipArchiveResponse; + } + + protected function extractWithSystemUnzip($file, $path, $fallback) { + return $this->systemUnzipResponse; + } } From abf06913a21a2978c1ee54a3f1cc93aef8eb472e Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Tue, 14 Feb 2017 17:37:40 +0100 Subject: [PATCH 4/7] remove useless expect (already done by docstring) --- tests/Composer/Test/Downloader/ZipDownloaderTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index b5ca6a33f..d2204d682 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -104,7 +104,6 @@ class ZipDownloaderTest extends TestCase * @expectedExceptionMessage SystemUnzip Failed */ function testSystemUnzipOnlyFailed() { - $this->setExpectedException(\Exception::class); $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); $e = new \Exception("SystemUnzip Failed"); $downloader->setUp(FALSE, TRUE, NULL, $e); @@ -129,7 +128,6 @@ class ZipDownloaderTest extends TestCase * @expectedExceptionMessage ZipArchive Failed */ function testSystemUnzipFallbackFailed() { - $this->setExpectedException(\Exception::class); $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); $e1 = new \Exception("ZipArchive Failed"); $e2 = new \Exception("SystemUnzip Failed"); From 98b7bd68b4ae43c68b369212d4a61df0b335a83c Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Tue, 14 Feb 2017 17:44:56 +0100 Subject: [PATCH 5/7] fix docstring and useless return --- src/Composer/Downloader/ZipDownloader.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index cbc96cef4..e1ee74381 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -70,7 +70,7 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file * @param bool $isFallback If true it is called as a fallback and should not throw exception - * @return bool True if succeed + * @return bool|\Exception True if succeed, an Exception if not */ protected function extractWithSystemUnzip($file, $path, $isFallback) { @@ -92,7 +92,6 @@ class ZipDownloader extends ArchiveDownloader if ( $isFallback ) { $this->io->write($processError); - return; } return new \RuntimeException($processError); } @@ -102,7 +101,7 @@ class ZipDownloader extends ArchiveDownloader * * @param string $file File to extract * @param string $path Path where to extract file - * @return bool True if succeed + * @return bool|\Exception True if succeed, an Exception if not */ protected function extractWithZipArchive($file, $path) { From 2e8d715c2f7101a03dbff23faf2326ac47686811 Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Tue, 14 Feb 2017 18:02:22 +0100 Subject: [PATCH 6/7] fix bad visibility --- src/Composer/Downloader/ZipDownloader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index e1ee74381..88b2b51cf 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -133,7 +133,7 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file */ - public function extract($file, $path) + protected function extract($file, $path) { $resultZipArchive = NULL; $resultUnzip = NULL; From f89e01d622cad70f302a2a3cbfb307e996645c59 Mon Sep 17 00:00:00 2001 From: Guillaume ZITTA Date: Tue, 14 Mar 2017 23:43:48 +0100 Subject: [PATCH 7/7] switch first / fallback order --- src/Composer/Downloader/ZipDownloader.php | 141 +++++----- .../Test/Downloader/ZipDownloaderTest.php | 260 +++++++++++++----- 2 files changed, 275 insertions(+), 126 deletions(-) diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index 88b2b51cf..73e5d61b0 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -30,8 +30,10 @@ use ZipArchive; class ZipDownloader extends ArchiveDownloader { protected $process; - protected static $hasSystemUnzip; - protected static $hasZipArchive; + public static $hasSystemUnzip; + public static $hasZipArchive; + public static $isWindows; + private $zipArchiveObject; public function __construct(IOInterface $io, Config $config, EventDispatcher $eventDispatcher = null, Cache $cache = null, ProcessExecutor $process = null, RemoteFilesystem $rfs = null) { @@ -53,6 +55,10 @@ class ZipDownloader extends ArchiveDownloader self::$hasZipArchive = class_exists('ZipArchive'); } + if (null === self::$isWindows) { + self::$isWindows = Platform::isWindows(); + } + if (!self::$hasZipArchive && !self::$hasSystemUnzip) { // php.ini path is added to the error message to help users find the correct file $iniMessage = IniHelper::getMessage(); @@ -69,62 +75,99 @@ class ZipDownloader extends ArchiveDownloader * * @param string $file File to extract * @param string $path Path where to extract file - * @param bool $isFallback If true it is called as a fallback and should not throw exception - * @return bool|\Exception True if succeed, an Exception if not + * @param bool $isLastChance If true it is called as a fallback and should throw an exception + * @return bool Success status */ - protected function extractWithSystemUnzip($file, $path, $isFallback) + protected function extractWithSystemUnzip($file, $path, $isLastChance) { + if (! self::$hasZipArchive) { + // Force Exception throwing if the Other alternative is not available + $isLastChance = true; + } + + if (! self::$hasSystemUnzip && ! $isLastChance) { + // This was call as the favorite extract way, but is not available + // We switch to the alternative + return $this->extractWithZipArchive($file, $path, true); + } + $processError = null; // When called after a ZipArchive failed, perhaps there is some files to overwrite - $overwrite = $isFallback ? '-o' : ''; + $overwrite = $isLastChance ? '-o' : ''; $command = 'unzip -qq '.$overwrite.' '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); try { if (0 === $this->process->execute($command, $ignoredOutput)) { - return TRUE; + return true; } - $processError = 'Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(); + $processError = new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } catch (\Exception $e) { - $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); + $processError = $e; + } + + if (! self::$hasZipArchive) { + $isLastChance = true; } - if ( $isFallback ) { - $this->io->write($processError); + if ($isLastChance) { + throw $processError; + } else { + $this->io->write($processError->getMessage()); + $this->io->write('Unzip with unzip command failed, falling back to ZipArchive class'); + return $this->extractWithZipArchive($file, $path, true); } - return new \RuntimeException($processError); } /** * extract $file to $path with ZipArchive * - * @param string $file File to extract - * @param string $path Path where to extract file - * @return bool|\Exception True if succeed, an Exception if not + * @param string $file File to extract + * @param string $path Path where to extract file + * @param bool $isLastChance If true it is called as a fallback and should throw an exception + * @return bool Success status */ - protected function extractWithZipArchive($file, $path) + protected function extractWithZipArchive($file, $path, $isLastChance) { - $zipArchive = new ZipArchive(); + if (! self::$hasSystemUnzip) { + // Force Exception throwing if the Other alternative is not available + $isLastChance = true; + } - if (true !== ($retval = $zipArchive->open($file))) { - return new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n"), $retval); + if (! self::$hasZipArchive && ! $isLastChance) { + // This was call as the favorite extract way, but is not available + // We switch to the alternative + return $this->extractWithSystemUnzip($file, $path, true); } - $extractResult = FALSE; + $processError = null; + $zipArchive = $this->zipArchiveObject ?: new ZipArchive(); + try { - $extractResult = $zipArchive->extractTo($path); - } catch (\Exception $e ) { - return $e; + if (true === ($retval = $zipArchive->open($file))) { + $extractResult = $zipArchive->extractTo($path); + + if (true === $extractResult) { + $zipArchive->close(); + return true; + } else { + $processError = new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); + } + } else { + $processError = new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n"), $retval); + } + } catch (\Exception $e) { + $processError = $e; } - if (true !== $extractResult) { - return new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); + if ($isLastChance) { + throw $processError; + } else { + $this->io->write($processError->getMessage()); + $this->io->write('Unzip with ZipArchive class failed, falling back to unzip command'); + return $this->extractWithSystemUnzip($file, $path, true); } - - $zipArchive->close(); - - return TRUE; } /** @@ -133,42 +176,14 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file */ - protected function extract($file, $path) + public function extract($file, $path) { - $resultZipArchive = NULL; - $resultUnzip = NULL; - - if ( self::$hasZipArchive ) { - // zip module is present - $resultZipArchive = $this->extractWithZipArchive($file, $path); - if ($resultZipArchive === TRUE) { - return; - } - } - - if ( self::$hasSystemUnzip ) { - // we have unzip in the path - $isFallback=FALSE; - if ( $resultZipArchive !== NULL) { - $this->io->writeError("\nUnzip using ZipArchive failed, trying with unzip"); - $isFallback=TRUE; - }; - $resultUnzip = $this->extractWithSystemUnzip($file, $path, $isFallback); - if ( $resultUnzip === TRUE ) { - return ; - } - }; - - // extract functions return TRUE or an exception - if ( $resultZipArchive !== NULL ) { - // zipArchive failed - // unZip not present or failed too - throw $resultZipArchive; + // Each extract calls its alternative if not available or fails + if (self::$isWindows) { + $this->extractWithZipArchive($file, $path, false); } else { - // unZip failed - // zipArchive not available - throw $resultUnzip; - }; + $this->extractWithSystemUnzip($file, $path, false); + } } /** diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index d2204d682..4cadda847 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -13,6 +13,7 @@ namespace Composer\Test\Downloader; use Composer\Downloader\ZipDownloader; +use Composer\Package\PackageInterface; use Composer\TestCase; use Composer\Util\Filesystem; @@ -22,24 +23,62 @@ class ZipDownloaderTest extends TestCase * @var string */ private $testDir; + private $prophet; public function setUp() { $this->testDir = $this->getUniqueTmpDirectory(); + $this->io = $this->getMock('Composer\IO\IOInterface'); + $this->config = $this->getMock('Composer\Config'); } + public function tearDown() { $fs = new Filesystem; $fs->removeDirectory($this->testDir); + ZipDownloader::$hasSystemUnzip = null; + ZipDownloader::$hasZipArchive = null; + } + + public function setPrivateProperty($name, $value, $obj = null) + { + $reflectionClass = new \ReflectionClass('Composer\Downloader\ZipDownloader'); + $reflectedProperty = $reflectionClass->getProperty($name); + $reflectedProperty->setAccessible(true); + if ($obj === null) { + $reflectedProperty = $reflectedProperty->setValue($value); + } else { + $reflectedProperty = $reflectedProperty->setValue($obj, $value); + } } + /** + * @group only + */ public function testErrorMessages() { if (!class_exists('ZipArchive')) { $this->markTestSkipped('zip extension missing'); } + $this->config->expects($this->at(0)) + ->method('get') + ->with('disable-tls') + ->will($this->returnValue(false)); + $this->config->expects($this->at(1)) + ->method('get') + ->with('cafile') + ->will($this->returnValue(null)); + $this->config->expects($this->at(2)) + ->method('get') + ->with('capath') + ->will($this->returnValue(null)); + $this->config->expects($this->at(3)) + ->method('get') + ->with('vendor-dir') + ->will($this->returnValue($this->testDir)); + $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) ->method('getDistUrl') @@ -54,110 +93,205 @@ class ZipDownloaderTest extends TestCase ->will($this->returnValue(array())) ; - $io = $this->getMock('Composer\IO\IOInterface'); - $config = $this->getMock('Composer\Config'); - $config->expects($this->at(0)) - ->method('get') - ->with('disable-tls') - ->will($this->returnValue(false)); - $config->expects($this->at(1)) - ->method('get') - ->with('cafile') - ->will($this->returnValue(null)); - $config->expects($this->at(2)) - ->method('get') - ->with('capath') - ->will($this->returnValue(null)); - $config->expects($this->at(3)) - ->method('get') - ->with('vendor-dir') - ->will($this->returnValue($this->testDir)); - $downloader = new ZipDownloader($io, $config); + $downloader = new ZipDownloader($this->io, $this->config); + + ZipDownloader::$hasSystemUnzip = false; try { $downloader->download($packageMock, sys_get_temp_dir().'/composer-zip-test'); $this->fail('Download of invalid zip files should throw an exception'); - } catch (\UnexpectedValueException $e) { + } catch (\Exception $e) { $this->assertContains('is not a zip archive', $e->getMessage()); } } /** - * @expectedException \Exception - * @expectedExceptionMessage ZipArchive Failed + * @expectedException \RuntimeException + * @expectedExceptionMessage There was an error extracting the ZIP file */ - function testZipArchiveOnlyFailed() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $e = new \Exception("ZipArchive Failed"); - $downloader->setUp(TRUE, FALSE, $e, NULL); + public function testZipArchiveOnlyFailed() + { + MockedZipDownloader::$hasSystemUnzip = false; + MockedZipDownloader::$hasZipArchive = true; + $downloader = new MockedZipDownloader($this->io, $this->config); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(false)); + + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); $downloader->extract('testfile.zip', 'vendor/dir'); } - function testZipArchiveOnlyGood() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $downloader->setUp(TRUE, FALSE, TRUE, NULL); + /** + * @group only + */ + public function testZipArchiveOnlyGood() + { + MockedZipDownloader::$hasSystemUnzip = false; + MockedZipDownloader::$hasZipArchive = true; + $downloader = new MockedZipDownloader($this->io, $this->config); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(true)); + + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); $downloader->extract('testfile.zip', 'vendor/dir'); } /** * @expectedException \Exception - * @expectedExceptionMessage SystemUnzip Failed + * @expectedExceptionMessage Failed to execute unzip */ - function testSystemUnzipOnlyFailed() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $e = new \Exception("SystemUnzip Failed"); - $downloader->setUp(FALSE, TRUE, NULL, $e); + public function testSystemUnzipOnlyFailed() + { + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = false; + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->at(0)) + ->method('execute') + ->will($this->returnValue(1)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); $downloader->extract('testfile.zip', 'vendor/dir'); } - function testSystemUnzipOnlyGood() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $downloader->setUp(FALSE, TRUE, NULL, TRUE); + public function testSystemUnzipOnlyGood() + { + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = false; + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->at(0)) + ->method('execute') + ->will($this->returnValue(0)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); $downloader->extract('testfile.zip', 'vendor/dir'); } - function testSystemUnzipFallbackGood() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $e = new \Exception("test"); - $downloader->setUp(TRUE, TRUE, $e, TRUE); + public function testNonWindowsFallbackGood() + { + MockedZipDownloader::$isWindows = false; + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = true; + + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->at(0)) + ->method('execute') + ->will($this->returnValue(1)); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(true)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); $downloader->extract('testfile.zip', 'vendor/dir'); } /** * @expectedException \Exception - * @expectedExceptionMessage ZipArchive Failed + * @expectedExceptionMessage There was an error extracting the ZIP file */ - function testSystemUnzipFallbackFailed() { - $downloader = new TestDownloader($this->getMock('Composer\IO\IOInterface')); - $e1 = new \Exception("ZipArchive Failed"); - $e2 = new \Exception("SystemUnzip Failed"); - $downloader->setUp(TRUE, TRUE, $e1, $e2); + public function testNonWindowsFallbackFailed() + { + MockedZipDownloader::$isWindows = false; + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = true; + + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->at(0)) + ->method('execute') + ->will($this->returnValue(1)); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(false)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); $downloader->extract('testfile.zip', 'vendor/dir'); } -} -class TestDownloader extends ZipDownloader { - public function __construct($io) + public function testWindowsFallbackGood() { - $this->io = $io; - } + MockedZipDownloader::$isWindows = true; + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = true; - public function extract($file, $path) { - parent::extract($file, $path); + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->atLeastOnce()) + ->method('execute') + ->will($this->returnValue(0)); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(false)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); + $downloader->extract('testfile.zip', 'vendor/dir'); } - public function setUp($zipArchive, $systemUnzip, $zipArchiveResponse, $systemUnzipResponse) { - self::$hasZipArchive = $zipArchive; - self::$hasSystemUnzip = $systemUnzip; - $this->zipArchiveResponse = $zipArchiveResponse; - $this->systemUnzipResponse = $systemUnzipResponse; + /** + * @expectedException \Exception + * @expectedExceptionMessage Failed to execute unzip + */ + public function testWindowsFallbackFailed() + { + MockedZipDownloader::$isWindows = true; + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = true; + + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->atLeastOnce()) + ->method('execute') + ->will($this->returnValue(1)); + + $zipArchive = $this->getMock('ZipArchive'); + $zipArchive->expects($this->at(0)) + ->method('open') + ->will($this->returnValue(true)); + $zipArchive->expects($this->at(1)) + ->method('extractTo') + ->will($this->returnValue(false)); + + $downloader = new MockedZipDownloader($this->io, $this->config, null, null, $processExecutor); + $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); + $downloader->extract('testfile.zip', 'vendor/dir'); } +} - protected function extractWithZipArchive($file, $path) { - return $this->zipArchiveResponse; +class MockedZipDownloader extends ZipDownloader +{ + public function download(PackageInterface $package, $path, $output = true) + { + return; } - protected function extractWithSystemUnzip($file, $path, $fallback) { - return $this->systemUnzipResponse; + public function extract($file, $path) + { + parent::extract($file, $path); } }