Merge pull request #5032 from curry684/proposal-5508

Centralize secure-http checking

src/Composer/Config.php

@@ -13,6 +13,7 @@
 namespace Composer;
 
 use Composer\Config\ConfigSourceInterface;
+use Composer\Downloader\TransportException;
 
 /**
  * @author Jordi Boggiano <j.boggiano@seld.be>
@@ -395,4 +396,28 @@ class Config
 
         return false;
     }
+
+    /**
+     * Validates that the passed URL is allowed to be used by current config, or throws an exception.
+     *
+     * @param string $url
+     */
+    public function prohibitUrlByConfig($url)
+    {
+        if (!$this->get('secure-http')) {
+            return;
+        }
+
+        // Parse the URL into its separate parts
+        $parsed = parse_url($url);
+        if (false === $parsed || !isset($parsed['scheme'])) {
+            // If the URL is malformed or does not contain a usable scheme it's not going to work anyway
+            return;
+        }
+
+        // Throw exception on known insecure protocols
+        if (in_array($parsed['scheme'], array('http', 'git', 'ftp', 'svn'))) {
+            throw new TransportException("Your configuration does not allow connections to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details.");
+        }
+    }
 }

src/Composer/Downloader/HgDownloader.php

@@ -25,7 +25,8 @@ class HgDownloader extends VcsDownloader
      */
     public function doDownload(PackageInterface $package, $path, $url)
     {
-        $this->checkSecureHttp($url);
+        // Ensure we are allowed to use this URL by config
+        $this->config->prohibitUrlByConfig($url);
 
         $url = ProcessExecutor::escape($url);
         $ref = ProcessExecutor::escape($package->getSourceReference());
@@ -45,7 +46,8 @@ class HgDownloader extends VcsDownloader
      */
     public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url)
     {
-        $this->checkSecureHttp($url);
+        // Ensure we are allowed to use this URL by config
+        $this->config->prohibitUrlByConfig($url);
 
         $url = ProcessExecutor::escape($url);
         $ref = ProcessExecutor::escape($target->getSourceReference());
@@ -89,13 +91,6 @@ class HgDownloader extends VcsDownloader
         return $output;
     }
 
-    protected function checkSecureHttp($url)
-    {
-        if (preg_match('{^http:}i', $url) && $this->config->get('secure-http')) {
-            throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details.");
-        }
-    }
-
     /**
      * {@inheritDoc}
      */

src/Composer/Repository/Vcs/HgDriver.php

@@ -48,9 +48,8 @@ class HgDriver extends VcsDriver
                 throw new \RuntimeException('Can not clone '.$this->url.' to access package information. The "'.$cacheDir.'" directory is not writable by the current user.');
             }
 
-            if (preg_match('{^http:}i', $this->url) && $this->config->get('secure-http')) {
-                throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details.");
-            }
+            // Ensure we are allowed to use this URL by config
+            $this->config->prohibitUrlByConfig($this->url);
 
             // update the repo if it is a valid hg repository
             if (is_dir($this->repoDir) && 0 === $this->process->execute('hg summary', $output, $this->repoDir)) {

src/Composer/Util/Git.php

@@ -40,9 +40,8 @@ class Git
 
     public function runCommand($commandCallable, $url, $cwd, $initialClone = false)
     {
-        if (preg_match('{^(http|git):}i', $url) && $this->config->get('secure-http')) {
-            throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details.");
-        }
+        // Ensure we are allowed to use this URL by config
+        $this->config->prohibitUrlByConfig($url);
 
         if ($initialClone) {
             $origCwd = $cwd;

src/Composer/Util/RemoteFilesystem.php

@@ -260,20 +260,9 @@ class RemoteFilesystem
             $this->io->writeError("    Downloading: <comment>Connecting...</comment>", false);
         }
 
-        // Check for secure HTTP
-        if (
-            ($this->scheme === 'http' || substr($fileUrl, 0, 5) === 'http:')
-            && $this->config && $this->config->get('secure-http')
-        ) {
-            // Passthru unsecure Packagist calls to $hashed providers as file integrity is verified with sha256
-            if (substr($fileUrl, 0, 23) !== 'http://packagist.org/p/' || (false === strpos($fileUrl, '$') && false === strpos($fileUrl, '%24'))) {
-                // other URLs must fail hard
-                throw new TransportException(sprintf(
-                    'Your configuration does not allow connection to %s://%s. See https://getcomposer.org/doc/06-config.md#secure-http for details.',
-                    $this->scheme,
-                    $originUrl
-                ));
-            }
+        // Check for secure HTTP, but allow insecure Packagist calls to $hashed providers as file integrity is verified with sha256
+        if ((substr($fileUrl, 0, 23) !== 'http://packagist.org/p/' || (false === strpos($fileUrl, '$') && false === strpos($fileUrl, '%24'))) && $this->config) {
+            $this->config->prohibitUrlByConfig($fileUrl);
         }
 
         $errorMessage = '';

src/Composer/Util/Svn.php

@@ -100,9 +100,8 @@ class Svn
      */
     public function execute($command, $url, $cwd = null, $path = null, $verbose = false)
     {
-        if (preg_match('{^(http|svn):}i', $url) && $this->config->get('secure-http')) {
-            throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details.");
-        }
+        // Ensure we are allowed to use this URL by config
+        $this->config->prohibitUrlByConfig($url);
 
         $svnCommand = $this->getCommand($command, $url, $path);
         $output = null;

tests/Composer/Test/ConfigTest.php

@@ -13,6 +13,7 @@
 namespace Composer\Test;
 
 use Composer\Config;
+use Composer\Downloader\TransportException;
 
 class ConfigTest extends \PHPUnit_Framework_TestCase
 {
@@ -212,6 +213,66 @@ class ConfigTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('https', 'git'), $config->get('github-protocols'));
     }
 
+    /**
+     * @dataProvider allowedUrlProvider
+     *
+     * @param string $url
+     */
+    public function testAllowedUrlsPass($url)
+    {
+        $config = new Config(false);
+        $config->prohibitUrlByConfig($url);
+    }
+
+    /**
+     * @dataProvider prohibitedUrlProvider
+     *
+     * @param string $url
+     */
+    public function testProhibitedUrlsThrowException($url)
+    {
+        $this->setExpectedException(
+            'Composer\Downloader\TransportException',
+            'Your configuration does not allow connections to ' . $url
+        );
+        $config = new Config(false);
+        $config->prohibitUrlByConfig($url);
+    }
+
+    /**
+     * @return array List of test URLs that should pass strict security
+     */
+    public function allowedUrlProvider()
+    {
+        $urls = array(
+            'https://packagist.org',
+            'git@github.com:composer/composer.git',
+            'hg://user:pass@my.satis/satis',
+            '\\myserver\myplace.git',
+            'file://myserver.localhost/mygit.git',
+            'file://example.org/mygit.git',
+        );
+        return array_combine($urls, array_map(function($e) { return array($e); }, $urls));
+    }
+
+    /**
+     * @return array List of test URLs that should not pass strict security
+     */
+    public function prohibitedUrlProvider()
+    {
+        $urls = array(
+            'http://packagist.org',
+            'http://10.1.0.1/satis',
+            'http://127.0.0.1/satis',
+            'svn://localhost/trunk',
+            'svn://will.not.resolve/trunk',
+            'svn://192.168.0.1/trunk',
+            'svn://1.2.3.4/trunk',
+            'git://5.6.7.8/git.git',
+        );
+        return array_combine($urls, array_map(function($e) { return array($e); }, $urls));
+    }
+    
     /**
      * @group TLS
      */