Clean up Zip Util to be more strict about what is a valid package archive, fixes #8931

src/Composer/Util/Zip.php

@@ -71,37 +71,41 @@ class Zip
      */
     private static function locateFile(\ZipArchive $zip, $filename)
     {
-        $indexOfShortestMatch = false;
+        // return root composer.json if it is there and is a file
-        $lengthOfShortestMatch = -1;
+        if (false !== ($index = $zip->locateName($filename)) && $zip->getFromIndex($index) !== false) {
-
+            return $index;
-        for ($i = 0; $i < $zip->numFiles; $i++) {
-            $stat = $zip->statIndex($i);
-            if (strcmp(basename($stat['name']), $filename) === 0) {
-                $directoryName = dirname($stat['name']);
-                if ($directoryName === '.') {
-                    //if composer.json is in root directory
-                    //it has to be the one to use.
-                    return $i;
         }
 
-                if (strpos($directoryName, '\\') !== false ||
+        $topLevelPaths = array();
-                    strpos($directoryName, '/') !== false) {
+        for ($i = 0; $i < $zip->numFiles; $i++) {
-                    //composer.json files below first directory are rejected
+            $name = $zip->getNameIndex($i);
+            $dirname = dirname($name);
+
+            // handle archives with proper TOC
+            if ($dirname === '.') {
+                $topLevelPaths[$name] = true;
+                if (\count($topLevelPaths) > 1) {
+                    // archive can only contain one top level directory
+                    return false;
+                }
                 continue;
             }
 
-                $length = strlen($stat['name']);
+            // handle archives which do not have a TOC record for the directory itself
-                if ($indexOfShortestMatch === false || $length < $lengthOfShortestMatch) {
+            if (false === strpos('\\', $dirname) && false === strpos('/', $dirname)) {
-                    //Check it's not a directory.
+                $topLevelPaths[$dirname.'/'] = true;
-                    $contents = $zip->getFromIndex($i);
+                if (\count($topLevelPaths) > 1) {
-                    if ($contents !== false) {
+                    // archive can only contain one top level directory
-                        $indexOfShortestMatch = $i;
+                    return false;
-                        $lengthOfShortestMatch = $length;
-                    }
                 }
             }
         }
 
-        return $indexOfShortestMatch;
+        if ($topLevelPaths && false !== ($index = $zip->locateName(key($topLevelPaths).$filename)) && $zip->getFromIndex($index) !== false) {
+            return $index;
+        }
+
+        // no composer.json found either at the top level or within the topmost directory
+        return false;
     }
 }

tests/Composer/Test/Util/ZipTest.php

@@ -20,7 +20,7 @@ use Composer\Test\TestCase;
  */
 class ZipTest extends TestCase
 {
-    public function testThrowsExceptionIfZipExcentionIsNotLoaded()
+    public function testThrowsExceptionIfZipExtensionIsNotLoaded()
     {
         if (extension_loaded('zip')) {
             $this->markTestSkipped('The PHP zip extension is loaded.');
@@ -74,7 +74,7 @@ class ZipTest extends TestCase
             return;
         }
 
-        $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolder.zip');
+        $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolders.zip');
 
         $this->assertNull($result);
     }
@@ -103,7 +103,7 @@ class ZipTest extends TestCase
         $this->assertEquals("{\n    \"name\": \"foo/bar\"\n}\n", $result);
     }
 
-    public function testReturnsRootComposerJsonAndSkipsSubfolders()
+    public function testMultipleTopLevelDirsIsInvalid()
     {
         if (!extension_loaded('zip')) {
             $this->markTestSkipped('The PHP zip extension is not loaded.');
@@ -112,6 +112,18 @@ class ZipTest extends TestCase
 
         $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/multiple.zip');
 
+        $this->assertNull($result);
+    }
+
+    public function testReturnsComposerJsonFromFirstSubfolder()
+    {
+        if (!extension_loaded('zip')) {
+            $this->markTestSkipped('The PHP zip extension is not loaded.');
+            return;
+        }
+
+        $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/single-sub.zip');
+
         $this->assertEquals("{\n    \"name\": \"foo/bar\"\n}\n", $result);
     }
 }