From 1e68555e0ae55c01300a9accc1bd5e4321522f57 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 30 Jan 2020 15:50:46 +0100 Subject: [PATCH] Sanitize URLs in getRepoName and centralize the Url sanitization process --- src/Composer/Downloader/GitDownloader.php | 3 ++- .../Repository/ArtifactRepository.php | 2 +- .../Repository/ComposerRepository.php | 3 ++- .../Repository/CompositeRepository.php | 2 +- src/Composer/Repository/PathRepository.php | 3 ++- src/Composer/Repository/VcsRepository.php | 3 ++- src/Composer/Util/AuthHelper.php | 11 ---------- src/Composer/Util/Git.php | 15 ++----------- src/Composer/Util/Hg.php | 15 ++----------- src/Composer/Util/Http/CurlDownloader.php | 8 +++---- src/Composer/Util/RemoteFilesystem.php | 4 ++-- src/Composer/Util/Url.php | 17 +++++++++++++++ tests/Composer/Test/Util/UrlTest.php | 21 +++++++++++++++++++ 13 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index ba026d2e2..1d4ebe4b6 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -17,6 +17,7 @@ use Composer\IO\IOInterface; use Composer\Package\PackageInterface; use Composer\Util\Filesystem; use Composer\Util\Git as GitUtil; +use Composer\Util\Url; use Composer\Util\Platform; use Composer\Util\ProcessExecutor; use Composer\Cache; @@ -434,7 +435,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $this->io->writeError(' '.$reference.' is gone (history was rewritten?)'); } - throw new \RuntimeException(GitUtil::sanitizeUrl('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput())); + throw new \RuntimeException(Url::sanitize('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput())); } protected function updateOriginUrl($path, $url) diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index d317b1404..a0acb7a61 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -45,7 +45,7 @@ class ArtifactRepository extends ArrayRepository implements ConfigurableReposito public function getRepoName() { - return 'platform repo ('.$this->lookup.')'; + return 'artifact repo ('.$this->lookup.')'; } public function getRepoConfig() diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index a03974c9a..0d5b67fa6 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -34,6 +34,7 @@ use Composer\Semver\Constraint\Constraint; use Composer\Semver\Constraint\EmptyConstraint; use Composer\Util\Http\Response; use Composer\Util\MetadataMinifier; +use Composer\Util\Url; use React\Promise\Util as PromiseUtil; /** @@ -129,7 +130,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito public function getRepoName() { - return 'composer repo ('.$this->url.')'; + return 'composer repo ('.Url::sanitize($this->url).')'; } public function getRepoConfig() diff --git a/src/Composer/Repository/CompositeRepository.php b/src/Composer/Repository/CompositeRepository.php index 9565a9b86..806934b7d 100644 --- a/src/Composer/Repository/CompositeRepository.php +++ b/src/Composer/Repository/CompositeRepository.php @@ -41,7 +41,7 @@ class CompositeRepository extends BaseRepository public function getRepoName() { - return 'composite repo ('.count($this->repositories).' repos)'; + return 'composite repo ('.implode(', ', array_map(function ($repo) { return $repo->getRepoName(); }, $this->repositories)).')'; } /** diff --git a/src/Composer/Repository/PathRepository.php b/src/Composer/Repository/PathRepository.php index 4ae6d4b1c..699270777 100644 --- a/src/Composer/Repository/PathRepository.php +++ b/src/Composer/Repository/PathRepository.php @@ -20,6 +20,7 @@ use Composer\Package\Version\VersionGuesser; use Composer\Package\Version\VersionParser; use Composer\Util\Platform; use Composer\Util\ProcessExecutor; +use Composer\Util\Url; /** * This repository allows installing local packages that are not necessarily under their own VCS. @@ -113,7 +114,7 @@ class PathRepository extends ArrayRepository implements ConfigurableRepositoryIn public function getRepoName() { - return 'path repo ('.$this->repoConfig['url'].')'; + return 'path repo ('.Url::sanitize($this->repoConfig['url']).')'; } public function getRepoConfig() diff --git a/src/Composer/Repository/VcsRepository.php b/src/Composer/Repository/VcsRepository.php index 585b0e582..5f4578d1e 100644 --- a/src/Composer/Repository/VcsRepository.php +++ b/src/Composer/Repository/VcsRepository.php @@ -22,6 +22,7 @@ use Composer\Package\Loader\LoaderInterface; use Composer\EventDispatcher\EventDispatcher; use Composer\Util\ProcessExecutor; use Composer\Util\HttpDownloader; +use Composer\Util\Url; use Composer\IO\IOInterface; use Composer\Config; @@ -87,7 +88,7 @@ class VcsRepository extends ArrayRepository implements ConfigurableRepositoryInt $driverType = $driverClass; } - return 'vcs repo ('.$driverType.' '.$this->url.')'; + return 'vcs repo ('.$driverType.' '.Url::sanitize($this->url).')'; } public function getRepoConfig() diff --git a/src/Composer/Util/AuthHelper.php b/src/Composer/Util/AuthHelper.php index 7b8d33d68..e4e829ea7 100644 --- a/src/Composer/Util/AuthHelper.php +++ b/src/Composer/Util/AuthHelper.php @@ -255,15 +255,4 @@ class AuthHelper return count($pathParts) >= 4 && $pathParts[3] == 'downloads'; } - - /** - * @param string $url - * @return string - */ - public function stripCredentialsFromUrl($url) - { - // GitHub repository rename result in redirect locations containing the access_token as GET parameter - // e.g. https://api.github.com/repositories/9999999999?access_token=github_token - return preg_replace('{([&?]access_token=)[^&]+}', '$1***', $url); - } } diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 798dd4a20..2239d33ad 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -362,27 +362,16 @@ class Git return '(' . implode('|', array_map('preg_quote', $config->get('gitlab-domains'))) . ')'; } - public static function sanitizeUrl($message) - { - return preg_replace_callback('{://(?P[^@]+?):(?P.+?)@}', function ($m) { - if (preg_match('{^[a-f0-9]{12,}$}', $m[1])) { - return '://***:***@'; - } - - return '://' . $m[1] . ':***@'; - }, $message); - } - private function throwException($message, $url) { // git might delete a directory when it fails and php will not know clearstatcache(); if (0 !== $this->process->execute('git --version', $ignoredOutput)) { - throw new \RuntimeException(self::sanitizeUrl('Failed to clone ' . $url . ', git was not found, check that it is installed and in your PATH env.' . "\n\n" . $this->process->getErrorOutput())); + throw new \RuntimeException(Url::sanitize('Failed to clone ' . $url . ', git was not found, check that it is installed and in your PATH env.' . "\n\n" . $this->process->getErrorOutput())); } - throw new \RuntimeException(self::sanitizeUrl($message)); + throw new \RuntimeException(Url::sanitize($message)); } /** diff --git a/src/Composer/Util/Hg.php b/src/Composer/Util/Hg.php index 3681ad5c7..d0b7fe79f 100644 --- a/src/Composer/Util/Hg.php +++ b/src/Composer/Util/Hg.php @@ -72,23 +72,12 @@ class Hg $this->throwException('Failed to clone ' . $url . ', ' . "\n\n" . $error, $url); } - public static function sanitizeUrl($message) - { - return preg_replace_callback('{://(?P[^@]+?):(?P.+?)@}', function ($m) { - if (preg_match('{^[a-f0-9]{12,}$}', $m[1])) { - return '://***:***@'; - } - - return '://' . $m[1] . ':***@'; - }, $message); - } - private function throwException($message, $url) { if (0 !== $this->process->execute('hg --version', $ignoredOutput)) { - throw new \RuntimeException(self::sanitizeUrl('Failed to clone ' . $url . ', hg was not found, check that it is installed and in your PATH env.' . "\n\n" . $this->process->getErrorOutput())); + throw new \RuntimeException(Url::sanitize('Failed to clone ' . $url . ', hg was not found, check that it is installed and in your PATH env.' . "\n\n" . $this->process->getErrorOutput())); } - throw new \RuntimeException(self::sanitizeUrl($message)); + throw new \RuntimeException(Url::sanitize($message)); } } diff --git a/src/Composer/Util/Http/CurlDownloader.php b/src/Composer/Util/Http/CurlDownloader.php index ee93ca364..017b2d1a2 100644 --- a/src/Composer/Util/Http/CurlDownloader.php +++ b/src/Composer/Util/Http/CurlDownloader.php @@ -195,7 +195,7 @@ class CurlDownloader $usingProxy = !empty($options['http']['proxy']) ? ' using proxy ' . $options['http']['proxy'] : ''; $ifModified = false !== strpos(strtolower(implode(',', $options['http']['header'])), 'if-modified-since:') ? ' if modified' : ''; if ($attributes['redirects'] === 0) { - $this->io->writeError('Downloading ' . $this->authHelper->stripCredentialsFromUrl($url) . $usingProxy . $ifModified, true, IOInterface::DEBUG); + $this->io->writeError('Downloading ' . Url::sanitize($url) . $usingProxy . $ifModified, true, IOInterface::DEBUG); } $this->checkCurlResult(curl_multi_add_handle($this->multiHandle, $curlHandle)); @@ -254,12 +254,12 @@ class CurlDownloader $contents = stream_get_contents($job['bodyHandle']); } $response = new Response(array('url' => $progress['url']), $statusCode, $headers, $contents); - $this->io->writeError('['.$statusCode.'] '.$this->authHelper->stripCredentialsFromUrl($progress['url']), true, IOInterface::DEBUG); + $this->io->writeError('['.$statusCode.'] '.Url::sanitize($progress['url']), true, IOInterface::DEBUG); } else { rewind($job['bodyHandle']); $contents = stream_get_contents($job['bodyHandle']); $response = new Response(array('url' => $progress['url']), $statusCode, $headers, $contents); - $this->io->writeError('['.$statusCode.'] '.$this->authHelper->stripCredentialsFromUrl($progress['url']), true, IOInterface::DEBUG); + $this->io->writeError('['.$statusCode.'] '.Url::sanitize($progress['url']), true, IOInterface::DEBUG); } fclose($job['bodyHandle']); @@ -362,7 +362,7 @@ class CurlDownloader } if (!empty($targetUrl)) { - $this->io->writeError(sprintf('Following redirect (%u) %s', $job['attributes']['redirects'] + 1, $this->authHelper->stripCredentialsFromUrl($targetUrl)), true, IOInterface::DEBUG); + $this->io->writeError(sprintf('Following redirect (%u) %s', $job['attributes']['redirects'] + 1, Url::sanitize($targetUrl)), true, IOInterface::DEBUG); return $targetUrl; } diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index cf39aaf9d..6312deabc 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -246,7 +246,7 @@ class RemoteFilesystem $actualContextOptions = stream_context_get_options($ctx); $usingProxy = !empty($actualContextOptions['http']['proxy']) ? ' using proxy ' . $actualContextOptions['http']['proxy'] : ''; - $this->io->writeError((substr($origFileUrl, 0, 4) === 'http' ? 'Downloading ' : 'Reading ') . $this->authHelper->stripCredentialsFromUrl($origFileUrl) . $usingProxy, true, IOInterface::DEBUG); + $this->io->writeError((substr($origFileUrl, 0, 4) === 'http' ? 'Downloading ' : 'Reading ') . Url::sanitize($origFileUrl) . $usingProxy, true, IOInterface::DEBUG); unset($origFileUrl, $actualContextOptions); // Check for secure HTTP, but allow insecure Packagist calls to $hashed providers as file integrity is verified with sha256 @@ -704,7 +704,7 @@ class RemoteFilesystem $this->redirects++; $this->io->writeError('', true, IOInterface::DEBUG); - $this->io->writeError(sprintf('Following redirect (%u) %s', $this->redirects, $this->authHelper->stripCredentialsFromUrl($targetUrl)), true, IOInterface::DEBUG); + $this->io->writeError(sprintf('Following redirect (%u) %s', $this->redirects, Url::sanitize($targetUrl)), true, IOInterface::DEBUG); $additionalOptions['redirects'] = $this->redirects; diff --git a/src/Composer/Util/Url.php b/src/Composer/Util/Url.php index b12a2d54d..2da171556 100644 --- a/src/Composer/Util/Url.php +++ b/src/Composer/Util/Url.php @@ -102,4 +102,21 @@ class Url return $origin; } + + public static function sanitize($url) + { + // GitHub repository rename result in redirect locations containing the access_token as GET parameter + // e.g. https://api.github.com/repositories/9999999999?access_token=github_token + $url = preg_replace('{([&?]access_token=)[^&]+}', '$1***', $url); + + $url = preg_replace_callback('{://(?P[^:/\s@]+):(?P[^@\s/]+)@}i', function ($m) { + if (preg_match('{^[a-f0-9]{12,}$}', $m['user'])) { + return '://***:***@'; + } + + return '://'.$m['user'].':***@'; + }, $url); + + return $url; + } } diff --git a/tests/Composer/Test/Util/UrlTest.php b/tests/Composer/Test/Util/UrlTest.php index 7772582a5..322bc0241 100644 --- a/tests/Composer/Test/Util/UrlTest.php +++ b/tests/Composer/Test/Util/UrlTest.php @@ -58,4 +58,25 @@ class UrlTest extends TestCase array('https://mygitlab.com/api/v3/projects/foo%2Fbar/repository/archive.tar.bz2?sha=abcd', 'https://mygitlab.com/api/v3/projects/foo%2Fbar/repository/archive.tar.bz2?sha=65', array('gitlab-domains' => array('mygitlab.com')), '65'), ); } + + /** + * @dataProvider sanitizeProvider + */ + public function testSanitize($expected, $url) + { + $this->assertSame($expected, Url::sanitize($url)); + } + + public static function sanitizeProvider() + { + return array( + array('https://foo:***@example.org/', 'https://foo:bar@example.org/'), + array('https://foo@example.org/', 'https://foo@example.org/'), + array('https://example.org/', 'https://example.org/'), + array('http://***:***@example.org', 'http://10a8f08e8d7b7b9:foo@example.org'), + array('https://foo:***@example.org:123/', 'https://foo:bar@example.org:123/'), + array('https://example.org/foo/bar?access_token=***', 'https://example.org/foo/bar?access_token=abcdef'), + array('https://example.org/foo/bar?foo=bar&access_token=***', 'https://example.org/foo/bar?foo=bar&access_token=abcdef'), + ); + } }