From 73662c725ab072e6750077d1c4a856221bde2a5e Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 19:15:06 +0000 Subject: [PATCH 1/9] Don't let PHP follow redirects it doesn't validate certificates --- src/Composer/Util/RemoteFilesystem.php | 78 +++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 4754e304b..b317a2399 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -38,6 +38,8 @@ class RemoteFilesystem private $lastHeaders; private $storeAuth; private $degradedMode = false; + private $redirects = 0; + private $maxRedirects = 20; /** * Constructor. @@ -293,6 +295,72 @@ class RemoteFilesystem $statusCode = $this->findStatusCode($http_response_header); } + if (!empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match)) { + // TODO: Only follow if PHP is not set to follow. + foreach ($http_response_header as $header) { + if (preg_match('{^location: *(.+) *$}i', $header, $m)) { + if (parse_url($m[1], PHP_URL_SCHEME)) { + $targetUrl = $m[1]; + + break; + } + + if (parse_url($m[1], PHP_URL_HOST)) { + // Scheme relative; e.g. //example.com/foo + $targetUrl = $this->scheme.':'.$m[1]; + break; + } + + if ('/' === $m[1][0]) { + // Absolute path; e.g. /foo + throw new \Exception('todo'); + } + + throw new \Exception('todo'); + break; + } + } + + if ($targetUrl) { + $this->redirects++; + + if ($this->redirects > $this->maxRedirects) { + $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, too many redirects'); + $e->setHeaders($http_response_header); + $e->setResponse($result); + throw $e; + } + + if ('http' === parse_url($targetUrl, PHP_URL_SCHEME) && 'https' === $this->scheme) { + // Do not allow protocol downgrade. + // TODO: Currently this will fail if a request goes http -> https -> http + $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, not permitting protocol downgrade'); + $e->setHeaders($http_response_header); + $e->setResponse($result); + throw $e; + } + + if ($this->io->isDebug()) { + $this->io->writeError(sprintf('Following redirect (%u)', $this->redirects)); + } + + // TODO: Not so sure about preserving origin here... + $result = $this->get($this->originUrl, $targetUrl, $additionalOptions, $this->fileName, $this->progress); + + $this->redirects--; + + return $result; + } + + if (!$this->retry) { + $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, got redirect without Location ('.$http_response_header[0].')'); + $e->setHeaders($http_response_header); + $e->setResponse($result); + throw $e; + } + $result = false; + } + // fail 4xx and 5xx responses and capture the response if ($statusCode && $statusCode >= 400 && $statusCode <= 599) { if (!$this->retry) { @@ -529,9 +597,9 @@ class RemoteFilesystem $host = parse_url($this->fileUrl, PHP_URL_HOST); } - if ($host === 'github.com' || $host === 'api.github.com') { - $host = '*.github.com'; - } + // if ($host === 'github.com' || $host === 'api.github.com') { + // $host = '*.github.com'; + // } $tlsOptions['ssl']['CN_match'] = $host; $tlsOptions['ssl']['SNI_server_name'] = $host; @@ -551,6 +619,10 @@ class RemoteFilesystem $headers[] = 'Connection: close'; } + if (PHP_VERSION_ID >= 50304 && $this->disableTls === false) { + $options['http']['follow_location'] = 0; + } + if ($this->io->hasAuthentication($originUrl)) { $auth = $this->io->getAuthentication($originUrl); if ('github.com' === $originUrl && 'x-oauth-basic' === $auth['password']) { From ce1eda25f3b7895531a0a7845377fc8f9a17031c Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 19:42:43 +0000 Subject: [PATCH 2/9] Follow redirects inside RFS only when required by PHP version --- src/Composer/Util/RemoteFilesystem.php | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index b317a2399..ba40517c4 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -221,6 +221,7 @@ class RemoteFilesystem } $options = $this->getOptionsForUrl($originUrl, $additionalOptions); + $userlandFollow = isset($options['http']['follow_location']) && !$options['http']['follow_location']; if ($this->io->isDebug()) { $this->io->writeError((substr($fileUrl, 0, 4) === 'http' ? 'Downloading ' : 'Reading ') . $fileUrl); @@ -295,8 +296,7 @@ class RemoteFilesystem $statusCode = $this->findStatusCode($http_response_header); } - if (!empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match)) { - // TODO: Only follow if PHP is not set to follow. + if ($userlandFollow && !empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match)) { foreach ($http_response_header as $header) { if (preg_match('{^location: *(.+) *$}i', $header, $m)) { if (parse_url($m[1], PHP_URL_SCHEME)) { @@ -597,9 +597,20 @@ class RemoteFilesystem $host = parse_url($this->fileUrl, PHP_URL_HOST); } - // if ($host === 'github.com' || $host === 'api.github.com') { - // $host = '*.github.com'; - // } + if (PHP_VERSION_ID >= 50304) { + // Must manually follow when setting CN_match because this causes all + // redirects to be validated against the same CN_match value. + $userlandFollow = true; + } else { + // PHP < 5.3.4 does not support follow_location, for those people + // do some really nasty hard coded transformations. These will + // still breakdown if the site redirects to a domain we don't + // expect. + + if ($host === 'github.com' || $host === 'api.github.com') { + $host = '*.github.com'; + } + } $tlsOptions['ssl']['CN_match'] = $host; $tlsOptions['ssl']['SNI_server_name'] = $host; @@ -619,7 +630,7 @@ class RemoteFilesystem $headers[] = 'Connection: close'; } - if (PHP_VERSION_ID >= 50304 && $this->disableTls === false) { + if (isset($userlandFollow)) { $options['http']['follow_location'] = 0; } From ffab235edd5cc05ba7fe37d0394cd929dc270f65 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 19:43:08 +0000 Subject: [PATCH 3/9] Remove code preventing protocol downgrades --- src/Composer/Util/RemoteFilesystem.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index ba40517c4..7ea71c2b0 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -331,14 +331,15 @@ class RemoteFilesystem throw $e; } - if ('http' === parse_url($targetUrl, PHP_URL_SCHEME) && 'https' === $this->scheme) { - // Do not allow protocol downgrade. - // TODO: Currently this will fail if a request goes http -> https -> http - $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, not permitting protocol downgrade'); - $e->setHeaders($http_response_header); - $e->setResponse($result); - throw $e; - } + // TODO: Disabled because this is (probably) different behaviour to PHP following for us. + // if ('http' === parse_url($targetUrl, PHP_URL_SCHEME) && 'https' === $this->scheme) { + // // Do not allow protocol downgrade. + // // TODO: Currently this will fail if a request goes http -> https -> http + // $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, not permitting protocol downgrade'); + // $e->setHeaders($http_response_header); + // $e->setResponse($result); + // throw $e; + // } if ($this->io->isDebug()) { $this->io->writeError(sprintf('Following redirect (%u)', $this->redirects)); From e830a611ec620237534822991675a3da3018bbf0 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 21:17:52 +0000 Subject: [PATCH 4/9] Handle other path redirects --- src/Composer/Util/RemoteFilesystem.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 7ea71c2b0..ef2a91e26 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -313,10 +313,16 @@ class RemoteFilesystem if ('/' === $m[1][0]) { // Absolute path; e.g. /foo - throw new \Exception('todo'); + $urlHost = parse_url($this->fileUrl, PHP_URL_HOST); + + // Replace path using hostname as an anchor. + $targetUrl = preg_replace('{^(.+(?://|@)'.preg_quote($urlHost).')(?:[/\?].*)?$}', '\1'.$m[1], $this->fileUrl); } - throw new \Exception('todo'); + // Relative path; e.g. foo + // This actually differs from PHP which seems to add duplicate slashes. + $targetUrl = preg_replace('{^(.+/)[^/?]*(?:\?.*)?$}', '\1'.$m[1], $this->fileUrl); + break; } } From 33471e389f126de413619b53afc4520264ccda3a Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 21:33:55 +0000 Subject: [PATCH 5/9] Pass redirect count using options Removing the risk it might be preserved between requests. --- src/Composer/Util/RemoteFilesystem.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index ef2a91e26..1216761ec 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -38,7 +38,7 @@ class RemoteFilesystem private $lastHeaders; private $storeAuth; private $degradedMode = false; - private $redirects = 0; + private $redirects; private $maxRedirects = 20; /** @@ -208,6 +208,7 @@ class RemoteFilesystem $this->lastProgress = null; $this->retryAuthFailure = true; $this->lastHeaders = array(); + $this->redirects = 1; // The first request counts. // capture username/password from URL if there is one if (preg_match('{^https?://(.+):(.+)@([^/]+)}i', $fileUrl, $match)) { @@ -220,6 +221,12 @@ class RemoteFilesystem unset($additionalOptions['retry-auth-failure']); } + if (isset($additionalOptions['redirects'])) { + $this->redirects = $additionalOptions['redirects']; + + unset($additionalOptions['redirects']); + } + $options = $this->getOptionsForUrl($originUrl, $additionalOptions); $userlandFollow = isset($options['http']['follow_location']) && !$options['http']['follow_location']; @@ -351,12 +358,10 @@ class RemoteFilesystem $this->io->writeError(sprintf('Following redirect (%u)', $this->redirects)); } - // TODO: Not so sure about preserving origin here... - $result = $this->get($this->originUrl, $targetUrl, $additionalOptions, $this->fileName, $this->progress); - - $this->redirects--; + $additionalOptions['redirects'] = $this->redirects; - return $result; + // TODO: Not so sure about preserving origin here... + return $this->get($this->originUrl, $targetUrl, $additionalOptions, $this->fileName, $this->progress); } if (!$this->retry) { From 8a8ec6fccc1cdfed76a30080811371966df2129e Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sun, 17 Jan 2016 21:34:34 +0000 Subject: [PATCH 6/9] Too many redirects is not an error in PHP, return the latest response --- src/Composer/Util/RemoteFilesystem.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 1216761ec..82b8f400d 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -303,7 +303,7 @@ class RemoteFilesystem $statusCode = $this->findStatusCode($http_response_header); } - if ($userlandFollow && !empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match)) { + if ($userlandFollow && !empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match) && $this->redirects < $this->maxRedirects) { foreach ($http_response_header as $header) { if (preg_match('{^location: *(.+) *$}i', $header, $m)) { if (parse_url($m[1], PHP_URL_SCHEME)) { @@ -337,13 +337,6 @@ class RemoteFilesystem if ($targetUrl) { $this->redirects++; - if ($this->redirects > $this->maxRedirects) { - $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, too many redirects'); - $e->setHeaders($http_response_header); - $e->setResponse($result); - throw $e; - } - // TODO: Disabled because this is (probably) different behaviour to PHP following for us. // if ('http' === parse_url($targetUrl, PHP_URL_SCHEME) && 'https' === $this->scheme) { // // Do not allow protocol downgrade. From dd3216e93d43e48a0bd82c82fd629c6c761b8d66 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 19 Jan 2016 22:19:17 +0000 Subject: [PATCH 7/9] Refactor to use new helper methods for headers --- src/Composer/Util/RemoteFilesystem.php | 44 ++++++++++---------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 82b8f400d..6ecc349db 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -303,38 +303,28 @@ class RemoteFilesystem $statusCode = $this->findStatusCode($http_response_header); } - if ($userlandFollow && !empty($http_response_header[0]) && preg_match('{^HTTP/\S+ (3\d\d)}i', $http_response_header[0], $match) && $this->redirects < $this->maxRedirects) { - foreach ($http_response_header as $header) { - if (preg_match('{^location: *(.+) *$}i', $header, $m)) { - if (parse_url($m[1], PHP_URL_SCHEME)) { - $targetUrl = $m[1]; - - break; - } - - if (parse_url($m[1], PHP_URL_HOST)) { - // Scheme relative; e.g. //example.com/foo - $targetUrl = $this->scheme.':'.$m[1]; - break; - } - - if ('/' === $m[1][0]) { - // Absolute path; e.g. /foo - $urlHost = parse_url($this->fileUrl, PHP_URL_HOST); - - // Replace path using hostname as an anchor. - $targetUrl = preg_replace('{^(.+(?://|@)'.preg_quote($urlHost).')(?:[/\?].*)?$}', '\1'.$m[1], $this->fileUrl); - } - + if ($userlandFollow && $statusCode >= 300 && $statusCode <= 399 && $this->redirects < $this->maxRedirects) { + if ($locationHeader = $this->findHeaderValue($http_response_header, 'location')) { + if (parse_url($locationHeader, PHP_URL_SCHEME)) { + // Absolute URL; e.g. https://example.com/composer + $targetUrl = $locationHeader; + } elseif (parse_url($locationHeader, PHP_URL_HOST)) { + // Scheme relative; e.g. //example.com/foo + $targetUrl = $this->scheme.':'.$locationHeader; + } elseif ('/' === $locationHeader[0]) { + // Absolute path; e.g. /foo + $urlHost = parse_url($this->fileUrl, PHP_URL_HOST); + + // Replace path using hostname as an anchor. + $targetUrl = preg_replace('{^(.+(?://|@)'.preg_quote($urlHost).')(?:[/\?].*)?$}', '\1'.$locationHeader, $this->fileUrl); + } else { // Relative path; e.g. foo // This actually differs from PHP which seems to add duplicate slashes. - $targetUrl = preg_replace('{^(.+/)[^/?]*(?:\?.*)?$}', '\1'.$m[1], $this->fileUrl); - - break; + $targetUrl = preg_replace('{^(.+/)[^/?]*(?:\?.*)?$}', '\1'.$locationHeader, $this->fileUrl); } } - if ($targetUrl) { + if (!empty($targetUrl)) { $this->redirects++; // TODO: Disabled because this is (probably) different behaviour to PHP following for us. From 34f1fcbdcb7316b356e41a6dc71dbd016c6a0b3f Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 22 Jan 2016 01:47:05 +0000 Subject: [PATCH 8/9] Drop downgrade warning --- src/Composer/Util/RemoteFilesystem.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 6ecc349db..e420fa3b7 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -327,16 +327,6 @@ class RemoteFilesystem if (!empty($targetUrl)) { $this->redirects++; - // TODO: Disabled because this is (probably) different behaviour to PHP following for us. - // if ('http' === parse_url($targetUrl, PHP_URL_SCHEME) && 'https' === $this->scheme) { - // // Do not allow protocol downgrade. - // // TODO: Currently this will fail if a request goes http -> https -> http - // $e = new TransportException('The "'.$this->fileUrl.'" file could not be downloaded, not permitting protocol downgrade'); - // $e->setHeaders($http_response_header); - // $e->setResponse($result); - // throw $e; - // } - if ($this->io->isDebug()) { $this->io->writeError(sprintf('Following redirect (%u)', $this->redirects)); } From 33f823146b52d5fe9b3826a1e7fad1e10f2037cf Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 22 Jan 2016 01:48:16 +0000 Subject: [PATCH 9/9] Account for ports in URL --- src/Composer/Util/RemoteFilesystem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index e420fa3b7..adba6c46e 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -316,7 +316,7 @@ class RemoteFilesystem $urlHost = parse_url($this->fileUrl, PHP_URL_HOST); // Replace path using hostname as an anchor. - $targetUrl = preg_replace('{^(.+(?://|@)'.preg_quote($urlHost).')(?:[/\?].*)?$}', '\1'.$locationHeader, $this->fileUrl); + $targetUrl = preg_replace('{^(.+(?://|@)'.preg_quote($urlHost).'(?::\d+)?)(?:[/\?].*)?$}', '\1'.$locationHeader, $this->fileUrl); } else { // Relative path; e.g. foo // This actually differs from PHP which seems to add duplicate slashes.