Merge pull request #898 from Seldaek/providefix

Fix hijacking possibility via provide bug

src/Composer/DependencyResolver/DefaultPolicy.php

@@ -141,15 +141,15 @@ class DefaultPolicy implements PolicyInterface
     }
 
     /**
-    * Checks if source replaces a package with the same name as target.
-    *
-    * Replace constraints are ignored. This method should only be used for
-    * prioritisation, not for actual constraint verification.
-    *
-    * @param PackageInterface $source
-    * @param PackageInterface $target
-    * @return bool
-    */
+     * Checks if source replaces a package with the same name as target.
+     *
+     * Replace constraints are ignored. This method should only be used for
+     * prioritisation, not for actual constraint verification.
+     *
+     * @param PackageInterface $source
+     * @param PackageInterface $target
+     * @return bool
+     */
     protected function replaces(PackageInterface $source, PackageInterface $target)
     {
         foreach ($source->getReplaces() as $link) {

src/Composer/DependencyResolver/Pool.php

@@ -140,15 +140,42 @@ class Pool
             return $candidates;
         }
 
-        $result = array();
+        $matches = $provideMatches = array();
+        $nameMatch = false;
 
         foreach ($candidates as $candidate) {
-            if ($candidate->matches($name, $constraint)) {
-                $result[] = $candidate;
+            switch ($candidate->matches($name, $constraint)) {
+                case BasePackage::MATCH_NONE:
+                    break;
+
+                case BasePackage::MATCH_NAME:
+                    $nameMatch = true;
+                    break;
+
+                case BasePackage::MATCH:
+                    $nameMatch = true;
+                    $matches[] = $candidate;
+                    break;
+
+                case BasePackage::MATCH_PROVIDE:
+                    $provideMatches[] = $candidate;
+                    break;
+
+                case BasePackage::MATCH_REPLACE:
+                    $matches[] = $candidate;
+                    break;
+
+                default:
+                    throw new \UnexpectedValueException('Unexpected match type');
             }
         }
 
-        return $result;
+        // if a package with the required name exists, we ignore providers
+        if ($nameMatch) {
+            return $matches;
+        }
+
+        return array_merge($matches, $provideMatches);
     }
 
     public function literalToPackage($literal)

src/Composer/Package/BasePackage.php

@@ -38,6 +38,12 @@ abstract class BasePackage implements PackageInterface
     const STABILITY_ALPHA   = 15;
     const STABILITY_DEV     = 20;
 
+    const MATCH_NAME = -1;
+    const MATCH_NONE = 0;
+    const MATCH = 1;
+    const MATCH_PROVIDE = 2;
+    const MATCH_REPLACE = 3;
+
     public static $stabilities = array(
         'stable' => self::STABILITY_STABLE,
         'RC'     => self::STABILITY_RC,
@@ -122,27 +128,27 @@ abstract class BasePackage implements PackageInterface
      *
      * @param  string                  $name       Name of the package to be matched
      * @param  LinkConstraintInterface $constraint The constraint to verify
-     * @return bool                    Whether this package matches the name and constraint
+     * @return int                     One of the MATCH* constants of this class or 0 if there is no match
      */
     public function matches($name, LinkConstraintInterface $constraint)
     {
         if ($this->name === $name) {
-            return $constraint->matches(new VersionConstraint('==', $this->getVersion()));
+            return $constraint->matches(new VersionConstraint('==', $this->getVersion())) ? self::MATCH : self::MATCH_NAME;
         }
 
         foreach ($this->getProvides() as $link) {
             if ($link->getTarget() === $name && $constraint->matches($link->getConstraint())) {
-                return true;
+                return self::MATCH_PROVIDE;
             }
         }
 
         foreach ($this->getReplaces() as $link) {
             if ($link->getTarget() === $name && $constraint->matches($link->getConstraint())) {
-                return true;
+                return self::MATCH_REPLACE;
             }
         }
 
-        return false;
+        return self::MATCH_NONE;
     }
 
     public function getRepository()

tests/Composer/Test/DependencyResolver/SolverTest.php

@@ -430,7 +430,6 @@ class SolverTest extends TestCase
     {
         $this->repo->addPackage($packageA = $this->getPackage('A', '1.0'));
         $this->repo->addPackage($packageQ = $this->getPackage('Q', '1.0'));
-        $this->repo->addPackage($packageB = $this->getPackage('B', '0.8'));
         $packageA->setRequires(array(new Link('A', 'B', $this->getVersionConstraint('>=', '1.0'), 'requires')));
         $packageQ->setProvides(array(new Link('Q', 'B', $this->getVersionConstraint('=', '1.0'), 'provides')));
 

tests/Composer/Test/Fixtures/installer/provide-priorities.test

@@ -0,0 +1,34 @@
+--TEST--
+Provide only applies when no existing package has the given name
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                { "name": "higher-prio-hijacker", "version": "1.1.0", "provide": { "package": "1.0.0" } },
+                { "name": "provider2", "version": "1.1.0", "provide": { "package2": "1.0.0" } }
+            ]
+        },
+        {
+            "type": "package",
+            "package": [
+                { "name": "package", "version": "0.9.0" },
+                { "name": "package", "version": "1.0.0" },
+                { "name": "hijacker", "version": "1.1.0", "provide": { "package": "1.0.0" } },
+                { "name": "provider3", "version": "1.1.0", "provide": { "package3": "1.0.0" } }
+            ]
+        }
+    ],
+    "require": {
+        "package": "1.*",
+        "package2": "1.*",
+        "provider3": "1.1.0"
+    }
+}
+--RUN--
+install
+--EXPECT--
+Installing package (1.0.0)
+Installing provider2 (1.1.0)
+Installing provider3 (1.1.0)

tests/Composer/Test/Fixtures/installer/replace-priorities.test

@@ -6,7 +6,7 @@ Replace takes precedence only in higher priority repositories
         {
             "type": "package",
             "package": [
-                { "name": "forked", "version": "1.1.0", "provide": { "package2": "1.1.0" } }
+                { "name": "forked", "version": "1.1.0", "replace": { "package2": "1.1.0" } }
             ]
         },
         {
@@ -14,7 +14,7 @@ Replace takes precedence only in higher priority repositories
             "package": [
                 { "name": "package", "version": "1.0.0" },
                 { "name": "package2", "version": "1.0.0" },
-                { "name": "hijacker", "version": "1.1.0", "provide": { "package": "1.1.0" } }
+                { "name": "hijacker", "version": "1.1.0", "replace": { "package": "1.1.0" } }
             ]
         }
     ],