Merge remote-tracking branch 'gza/fix_unzip_on_win_php56'

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);
+        }
     }
 
     /**

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);
+    }
 }