From dc1fd92b9b73a02cb0797ceeaac93e0d208e6b97 Mon Sep 17 00:00:00 2001 From: "Iskander (Alex) Sharipov" Date: Wed, 26 Aug 2020 17:23:10 +0300 Subject: [PATCH 01/10] Util/Zip: fix strpos args order `strpos()` first argument is a haystack, not a needle. `strpos('x', $s)` is identical to `$s === 'x'` which is probably not what we want here. --- src/Composer/Util/Zip.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Util/Zip.php b/src/Composer/Util/Zip.php index ad6617edf..82cd34b71 100644 --- a/src/Composer/Util/Zip.php +++ b/src/Composer/Util/Zip.php @@ -92,7 +92,7 @@ class Zip } // handle archives which do not have a TOC record for the directory itself - if (false === strpos('\\', $dirname) && false === strpos('/', $dirname)) { + if (false === strpos($dirname, '\\') && false === strpos($dirname, '/')) { $topLevelPaths[$dirname.'/'] = true; if (\count($topLevelPaths) > 1) { throw new \RuntimeException('Archive has more than one top level directories, and no composer.json was found on the top level, so it\'s an invalid archive. Top level paths found were: '.implode(',', array_keys($topLevelPaths))); From 87573aab27dc6d80601155b6c31781fd375ad317 Mon Sep 17 00:00:00 2001 From: Ayesh Karunaratne Date: Wed, 26 Aug 2020 23:01:00 +0700 Subject: [PATCH 02/10] Sanitize repo URLs to mask HTTP auth passwords from cache directory When a Composer repository is cached, a directory name is generated created stored package meta information fetched from that repository. The cache directory can contain HTTP basic auth tokens, or access_token query parameters that end up in the directory name of the cache directory. Discovered when trying out [GitLab composer repository feature](https://php.watch/articles/composer-gitlab-repositories), and the HTTP password was visible in a `composer update -vvv` command. Using passwords/tokens in the URL is fundamentally a bad idea, but Composer already has `\Composer\Util\Url::sanitize()` that tries to mitigate such cases, and this same function is applied to the repo URL before deciding the name of the repo cache directory. --- src/Composer/Repository/ComposerRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index e5706e04d..9b244f397 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -129,7 +129,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $this->baseUrl = rtrim(preg_replace('{(?:/[^/\\\\]+\.json)?(?:[?#].*)?$}', '', $this->url), '/'); $this->io = $io; - $this->cache = new Cache($io, $config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $this->url), 'a-z0-9.$~'); + $this->cache = new Cache($io, $config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', Url::sanitize($this->url)), 'a-z0-9.$~'); $this->cache->setReadOnly($config->get('cache-read-only')); $this->versionParser = new VersionParser(); $this->loader = new ArrayLoader($this->versionParser); From 931a1ff1f8d7e567bb27514c196d5ecdf47be726 Mon Sep 17 00:00:00 2001 From: Ayesh Karunaratne Date: Thu, 27 Aug 2020 11:45:49 +0700 Subject: [PATCH 03/10] AuthHelper: Allow fall-through GitLab-specific HTTP headers for auth Previously, `AuthHelper` consumed the authentication credentials for GitLab domains and added access tokens as GitLab-specific headers. [Composer repositories now supported in GitLab](https://php.watch/articles/composer-gitlab-repositories) require standard Authorization headers with a personal access to function, which failed to work due to out GitLab-specific headers. With this commit, AuthHelper checks if the password is an access token, and falls through to HTTP basic authentication even if the domain name is a GitLab domain name. --- src/Composer/Util/AuthHelper.php | 5 ++++- tests/Composer/Test/Util/AuthHelperTest.php | 10 +++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Composer/Util/AuthHelper.php b/src/Composer/Util/AuthHelper.php index a318931e3..5a3f950c7 100644 --- a/src/Composer/Util/AuthHelper.php +++ b/src/Composer/Util/AuthHelper.php @@ -205,7 +205,10 @@ class AuthHelper $headers[] = 'Authorization: token '.$auth['username']; $authenticationDisplayMessage = 'Using GitHub token authentication'; } - } elseif (in_array($origin, $this->config->get('gitlab-domains'), true)) { + } elseif ( + in_array($origin, $this->config->get('gitlab-domains'), true) + && in_array($auth['password'], array('oauth2', 'private-token', 'gitlab-ci-token'), true) + ) { if ($auth['password'] === 'oauth2') { $headers[] = 'Authorization: Bearer '.$auth['username']; $authenticationDisplayMessage = 'Using GitLab OAuth token authentication'; diff --git a/tests/Composer/Test/Util/AuthHelperTest.php b/tests/Composer/Test/Util/AuthHelperTest.php index 567299345..11e546ed1 100644 --- a/tests/Composer/Test/Util/AuthHelperTest.php +++ b/tests/Composer/Test/Util/AuthHelperTest.php @@ -280,6 +280,14 @@ class AuthHelperTest extends TestCase 'password' => 'my_password' ) ), + array( + 'https://gitlab.com', + 'gitlab.com', + array( + 'username' => 'my_username', + 'password' => 'my_password' + ) + ), ); } @@ -302,7 +310,7 @@ class AuthHelperTest extends TestCase $this->config->expects($this->once()) ->method('get') ->with('gitlab-domains') - ->willReturn(array()); + ->willReturn(array($origin)); $this->io->expects($this->once()) ->method('writeError') From c3db4614c9d5ddd26a83cf8db05a6828ec168f9d Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 27 Aug 2020 10:19:23 +0200 Subject: [PATCH 04/10] Also remove credentials from cache dirs in git/svn drivers, fixes #7439, refs #9155 --- src/Composer/Repository/Vcs/GitDriver.php | 3 ++- src/Composer/Repository/Vcs/SvnDriver.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index 499a5a9df..230c24303 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -14,6 +14,7 @@ namespace Composer\Repository\Vcs; use Composer\Util\ProcessExecutor; use Composer\Util\Filesystem; +use Composer\Util\Url; use Composer\Util\Git as GitUtil; use Composer\IO\IOInterface; use Composer\Cache; @@ -74,7 +75,7 @@ class GitDriver extends VcsDriver $this->getTags(); $this->getBranches(); - $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $cacheUrl)); + $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', Url::sanitize($cacheUrl))); $this->cache->setReadOnly($this->config->get('cache-read-only')); } diff --git a/src/Composer/Repository/Vcs/SvnDriver.php b/src/Composer/Repository/Vcs/SvnDriver.php index 87c9781b6..56f1dcfc6 100644 --- a/src/Composer/Repository/Vcs/SvnDriver.php +++ b/src/Composer/Repository/Vcs/SvnDriver.php @@ -17,6 +17,7 @@ use Composer\Config; use Composer\Json\JsonFile; use Composer\Util\ProcessExecutor; use Composer\Util\Filesystem; +use Composer\Util\Url; use Composer\Util\Svn as SvnUtil; use Composer\IO\IOInterface; use Composer\Downloader\TransportException; @@ -77,7 +78,7 @@ class SvnDriver extends VcsDriver $this->baseUrl = substr($this->url, 0, $pos); } - $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $this->baseUrl)); + $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', Url::sanitize($this->baseUrl))); $this->cache->setReadOnly($this->config->get('cache-read-only')); $this->getBranches(); From f18d91bd58b41e914ff72441cdaefc6a24db96a3 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 27 Aug 2020 11:25:43 +0200 Subject: [PATCH 05/10] Fix pre/post-package-install/update/uninstall events receiving a partial list of operations, fixes #9079 --- src/Composer/Installer/InstallationManager.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index 7dfb2ff3e..62b0cade4 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -292,14 +292,12 @@ class InstallationManager if ($batch) { $batches[] = $batch; } - unset($operations[$index]); $batches[] = array($index => $operation); $batch = array(); continue; } } - unset($operations[$index]); $batch[$index] = $operation; } @@ -308,7 +306,7 @@ class InstallationManager } foreach ($batches as $batch) { - $this->executeBatch($repo, $batch, $cleanupPromises, $devMode, $runScripts); + $this->executeBatch($repo, $batch, $cleanupPromises, $devMode, $runScripts, $operations); } } catch (\Exception $e) { $runCleanup(); @@ -336,7 +334,11 @@ class InstallationManager $repo->write($devMode, $this); } - private function executeBatch(RepositoryInterface $repo, array $operations, array $cleanupPromises, $devMode, $runScripts) + /** + * @param array $operations List of operations to execute in this batch + * @param array $allOperations Complete list of operations to be executed in the install job, used for event listeners + */ + private function executeBatch(RepositoryInterface $repo, array $operations, array $cleanupPromises, $devMode, $runScripts, array $allOperations) { foreach ($operations as $index => $operation) { $opType = $operation->getOperationType(); @@ -363,7 +365,7 @@ class InstallationManager $event = 'Composer\Installer\PackageEvents::PRE_PACKAGE_'.strtoupper($opType); if (defined($event) && $runScripts && $this->eventDispatcher) { - $this->eventDispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); + $this->eventDispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $allOperations, $operation); } $dispatcher = $this->eventDispatcher; @@ -378,12 +380,12 @@ class InstallationManager $promise = $promise->then(function () use ($opType, $installManager, $repo, $operation) { return $installManager->$opType($repo, $operation); })->then($cleanupPromises[$index]) - ->then(function () use ($opType, $runScripts, $dispatcher, $installManager, $devMode, $repo, $operations, $operation) { + ->then(function () use ($opType, $runScripts, $dispatcher, $installManager, $devMode, $repo, $allOperations, $operation) { $repo->write($devMode, $installManager); $event = 'Composer\Installer\PackageEvents::POST_PACKAGE_'.strtoupper($opType); if (defined($event) && $runScripts && $dispatcher) { - $dispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); + $dispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $allOperations, $operation); } }, function ($e) use ($opType, $package, $io) { $io->writeError(' ' . ucfirst($opType) .' of '.$package->getPrettyName().' failed'); From 706125fbbff1d7cde55687c8bb357b805068374b Mon Sep 17 00:00:00 2001 From: Brad Jones Date: Thu, 27 Aug 2020 20:05:04 -1000 Subject: [PATCH 06/10] Update config section to note required scope for GitLab tokens --- doc/06-config.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/06-config.md b/doc/06-config.md index ccbdb3b07..f8fcc0615 100644 --- a/doc/06-config.md +++ b/doc/06-config.md @@ -93,7 +93,8 @@ private repositories on gitlab. Using `{"gitlab.com": {"username": "gitlabuser", token functionality (https://docs.gitlab.com/ee/user/project/deploy_tokens/) Please note: If the package is not hosted at gitlab.com the domain names must be also specified with the -[`gitlab-domains`](06-config.md#gitlab-domains) option. +[`gitlab-domains`](06-config.md#gitlab-domains) option. The token must have +`api` or `read_api` scope. ## disable-tls From cf8ff2a75de7693395e60d164683efc44e83430e Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 13:09:18 +0200 Subject: [PATCH 07/10] Fix test filename to end with .test extension so it gets run --- ...-be-installed-together-with-provided-if-both-installable.test} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/Composer/Test/Fixtures/installer/{provider-packages-can-be-installed-together-with-provided-if-both-installable.json => provider-packages-can-be-installed-together-with-provided-if-both-installable.test} (100%) diff --git a/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.json b/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test similarity index 100% rename from tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.json rename to tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test From 85950f8e9a7f9901c061456ceb43317d59b23661 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 13:19:59 +0200 Subject: [PATCH 08/10] Fix provider coexistence test, needs another requirement to install both --- ...talled-together-with-provided-if-both-installable.test | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test b/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test index 9abe90dd8..6153fcbc0 100644 --- a/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test +++ b/tests/Composer/Test/Fixtures/installer/provider-packages-can-be-installed-together-with-provided-if-both-installable.test @@ -15,12 +15,16 @@ Test that providers can be installed in conjunction with the package they provid }, { "name": "foo/standard", - "version": "1.0.0" + "version": "1.0.0", + "provide": { + "foo/also-necessary": "1.0.0" + } } ] } ], "require": { + "foo/also-necessary": "1.0.0", "foo/standard": "1.0.0", "foo/polyfill": "1.0.0" } @@ -30,5 +34,5 @@ Test that providers can be installed in conjunction with the package they provid update --EXPECT-- -Installing foo/standard (1.0.0) Installing foo/polyfill (1.0.0) +Installing foo/standard (1.0.0) From 140665eadd1f7a65918702e17c8c18247b2cc5dc Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 13:39:34 +0200 Subject: [PATCH 09/10] Add another test verifying that a package may provide an incompatible version of sth that actually exists --- ...oexist-with-other-version-of-provided.test | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 tests/Composer/Test/Fixtures/installer/provider-can-coexist-with-other-version-of-provided.test diff --git a/tests/Composer/Test/Fixtures/installer/provider-can-coexist-with-other-version-of-provided.test b/tests/Composer/Test/Fixtures/installer/provider-can-coexist-with-other-version-of-provided.test new file mode 100644 index 000000000..fc7e2e257 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/provider-can-coexist-with-other-version-of-provided.test @@ -0,0 +1,34 @@ +--TEST-- +Test that providers can coexist with a different version of the package they provide +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "foo/provider", + "provide": { + "foo/original": "3.0.0" + }, + "version": "1.0.0" + }, + { + "name": "foo/original", + "version": "1.0.0" + } + ] + } + ], + "require": { + "foo/original": "1.0.0", + "foo/provider": "1.0.0" + } +} + +--RUN-- +update + +--EXPECT-- +Installing foo/original (1.0.0) +Installing foo/provider (1.0.0) From 43093d0eeb3cbae7d7edd430de50e76ef781f07b Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 14:50:28 +0200 Subject: [PATCH 10/10] Add tests for edge cases of packages providing names which exist as real packages --- ...th-other-version-of-provided-conflict.test | 56 +++++++++++++++++++ ...th-other-version-of-provided-indirect.test | 54 ++++++++++++++++++ ...gether-with-other-version-of-provided.test | 46 +++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-conflict.test create mode 100644 tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-indirect.test create mode 100644 tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided.test diff --git a/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-conflict.test b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-conflict.test new file mode 100644 index 000000000..b718a3a7d --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-conflict.test @@ -0,0 +1,56 @@ +--TEST-- +A root requirement for a name exists as well as a dependency's requirement for the same name but in a different version. +Since the root requirement does not allow the dependency's requirement to be installed, this conflicts. + +The difference between this test and the one which does not conflict is that here the root requirement could only be +satisfied with the provided package but would conflict with the actual package by the given name. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "foo/provider", + "provide": { + "foo/original": "3.0.0" + }, + "version": "1.0.0" + }, + { + "name": "foo/original", + "version": "1.0.0" + }, + { + "name": "foo/requirer", + "require": { + "foo/original": "1.0.0" + }, + "version": "1.0.0" + } + ] + } + ], + "require": { + "foo/original": "3.0.0", + "foo/provider": "1.0.0", + "foo/requirer": "1.0.0" + } +} + +--RUN-- +update + +--EXPECT-EXIT-CODE-- +2 + +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Updating dependencies +Your requirements could not be resolved to an installable set of packages. + + Problem 1 + - Root composer.json requires foo/requirer 1.0.0 -> satisfiable by foo/requirer[1.0.0]. + - foo/requirer 1.0.0 requires foo/original 1.0.0 -> found foo/original[1.0.0] but it conflicts with your root composer.json require (3.0.0). + +--EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-indirect.test b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-indirect.test new file mode 100644 index 000000000..c9e629ce3 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided-indirect.test @@ -0,0 +1,54 @@ +--TEST-- +This is a variant of the test provider-gets-picked-together-with-other-version-of-provided-conflict.test which differs +in so far as that the root requirements were all moved into a package which now allows the combination to succeed. + +Only root requirements strictly limit compatibility with versions if a package by a provided name also actually exists. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "foo/root", + "require": { + "foo/original": "3.0.0", + "foo/provider": "1.0.0", + "foo/requirer": "1.0.0" + }, + "version": "1.0.0" + }, + { + "name": "foo/provider", + "provide": { + "foo/original": "3.0.0" + }, + "version": "1.0.0" + }, + { + "name": "foo/original", + "version": "1.0.0" + }, + { + "name": "foo/requirer", + "require": { + "foo/original": "1.0.0" + }, + "version": "1.0.0" + } + ] + } + ], + "require": { + "foo/root": "1.0.0" + } +} + +--RUN-- +update + +--EXPECT-- +Installing foo/original (1.0.0) +Installing foo/provider (1.0.0) +Installing foo/requirer (1.0.0) +Installing foo/root (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided.test b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided.test new file mode 100644 index 000000000..699e26539 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/provider-gets-picked-together-with-other-version-of-provided.test @@ -0,0 +1,46 @@ +--TEST-- +A root requirement for a name exists as well as a dependency's requirement for the same name but in a different version. +The root requirement is satisfied by the actual package by that name in that version. The dependency's requirement can +be satisfied by another package which provides the name in the other version. This will result in all packages being +installed as the provider does not conflict with the actual package and all requirements can be met. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "foo/provider", + "provide": { + "foo/original": "3.0.0" + }, + "version": "1.0.0" + }, + { + "name": "foo/original", + "version": "1.0.0" + }, + { + "name": "foo/requirer", + "require": { + "foo/original": "3.0.0" + }, + "version": "1.0.0" + } + ] + } + ], + "require": { + "foo/original": "1.0.0", + "foo/provider": "1.0.0", + "foo/requirer": "1.0.0" + } +} + +--RUN-- +update + +--EXPECT-- +Installing foo/original (1.0.0) +Installing foo/provider (1.0.0) +Installing foo/requirer (1.0.0)