diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index 612eb9da2..73e5d61b0 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -30,7 +30,10 @@ use ZipArchive; class ZipDownloader extends ArchiveDownloader { protected $process; - protected static $hasSystemUnzip; + 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) { @@ -48,7 +51,15 @@ 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 (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(); $error = "The zip extension and unzip command are both missing, skipping.\n" . $iniMessage; @@ -59,42 +70,120 @@ class ZipDownloader extends ArchiveDownloader return parent::download($package, $path, $output); } - protected function extract($file, $path) + /** + * extract $file to $path with "unzip" command + * + * @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 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 = $isLastChance ? '-o' : ''; + + $command = 'unzip -qq '.$overwrite.' '.ProcessExecutor::escape($file).' -d '.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 true; } - try { - if (0 === $this->process->execute($command, $ignoredOutput)) { - return; - } + $processError = new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); + } catch (\Exception $e) { + $processError = $e; + } - $processError = 'Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(); - } catch (\Exception $e) { - $processError = 'Failed to execute ' . $command . "\n\n" . $e->getMessage(); - } + if (! self::$hasZipArchive) { + $isLastChance = true; + } - if (!class_exists('ZipArchive')) { - throw new \RuntimeException($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); + } + } + + /** + * extract $file to $path with ZipArchive + * + * @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, $isLastChance) + { + if (! self::$hasSystemUnzip) { + // Force Exception throwing if the Other alternative is not available + $isLastChance = true; + } + + 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); } - $zipArchive = new ZipArchive(); + $processError = null; + $zipArchive = $this->zipArchiveObject ?: new ZipArchive(); + + try { + if (true === ($retval = $zipArchive->open($file))) { + $extractResult = $zipArchive->extractTo($path); - if (true !== ($retval = $zipArchive->open($file))) { - throw new \UnexpectedValueException(rtrim($this->getErrorMessage($retval, $file)."\n".$processError), $retval); + 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 !== $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)); + 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(); + /** + * extract $file to $path + * + * @param string $file File to extract + * @param string $path Path where to extract file + */ + public function extract($file, $path) + { + // Each extract calls its alternative if not available or fails + if (self::$isWindows) { + $this->extractWithZipArchive($file, $path, false); + } else { + $this->extractWithSystemUnzip($file, $path, false); + } } /** diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index f70d9e44c..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() { - if (!class_exists('ZipArchive')) { - $this->markTestSkipped('zip extension missing'); - } - $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,31 +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 \RuntimeException + * @expectedExceptionMessage There was an error extracting the ZIP file + */ + 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'); + } + + /** + * @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 Failed to execute unzip + */ + 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'); + } + + 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'); + } + + 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 There was an error extracting the ZIP file + */ + 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'); + } + + public function testWindowsFallbackGood() + { + MockedZipDownloader::$isWindows = true; + MockedZipDownloader::$hasSystemUnzip = true; + MockedZipDownloader::$hasZipArchive = true; + + $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'); + } + + /** + * @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'); + } +} + +class MockedZipDownloader extends ZipDownloader +{ + public function download(PackageInterface $package, $path, $output = true) + { + return; + } + + public function extract($file, $path) + { + parent::extract($file, $path); + } }