From 2d19cf2a00ca06e23bcfc3187d166bc74f74166e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 11 Jul 2012 14:04:54 +0200 Subject: [PATCH 1/2] Fix hijacking possibility via provide bug --- .../DependencyResolver/DefaultPolicy.php | 18 +++++----- src/Composer/DependencyResolver/Pool.php | 35 ++++++++++++++++--- src/Composer/Package/BasePackage.php | 16 ++++++--- .../installer/provide-priorities.test | 34 ++++++++++++++++++ .../installer/replace-priorities.test | 4 +-- 5 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 tests/Composer/Test/Fixtures/installer/provide-priorities.test diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 108a8d44e..ff19be8fb 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/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) { diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index f208568ee..0e3030738 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/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) diff --git a/src/Composer/Package/BasePackage.php b/src/Composer/Package/BasePackage.php index a3ae853f3..13c5c1fe5 100644 --- a/src/Composer/Package/BasePackage.php +++ b/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() diff --git a/tests/Composer/Test/Fixtures/installer/provide-priorities.test b/tests/Composer/Test/Fixtures/installer/provide-priorities.test new file mode 100644 index 000000000..f97e16e6c --- /dev/null +++ b/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) diff --git a/tests/Composer/Test/Fixtures/installer/replace-priorities.test b/tests/Composer/Test/Fixtures/installer/replace-priorities.test index c6c77c060..2f27ba7b7 100644 --- a/tests/Composer/Test/Fixtures/installer/replace-priorities.test +++ b/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" } } ] } ], From d4aab7d5b6d3afe4a246de5165ea318e9b6a9c87 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 11 Jul 2012 19:37:02 +0200 Subject: [PATCH 2/2] Fix solver test --- tests/Composer/Test/DependencyResolver/SolverTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index abd32d223..ff420fb00 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/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')));