Make disableTls a core RemoteFilesystem option - per method invites h…

…uman error
padraic · Feb 24, 2014 · a2bf14e · a2bf14e
1 parent 3cd979b
commit a2bf14e
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 18 deletions.
diff --git a/src/Composer/Command/DiagnoseCommand.php b/src/Composer/Command/DiagnoseCommand.php
@@ -151,12 +151,14 @@ private function checkHttp(Config $config)
             $result[] = '<error>Composer is configured to use SSL/TLS protection but the openssl extension is not available.</error>';
         }
 
-        $remoteFilesystemOptions = array();
-        if (!is_null($config->get('cafile'))) {
-            $remoteFilesystemOptions = array('ssl'=>array('cafile'=>$config->get('cafile')));
+        $rfsOptions = array();
+        if ($disableTls) {
+            if (!is_null($config->get('cafile'))) {
+                $rfsOptions = array('ssl'=>array('cafile'=>$config->get('cafile')));
+            }
         }
         try {
-            $this->rfs = new RemoteFilesystem($this->getIO(), $remoteFilesystemOptions, $disableTls);
+            $this->rfs = new RemoteFilesystem($this->getIO(), $rfsOptions, $disableTls);
         } catch (TransportException $e) {
             if (preg_match('|cafile|', $e->getMessage())) {
                 $result[] = '<error>[' . get_class($e) . '] ' . $e->getMessage() . '</error>';
@@ -168,7 +170,7 @@ private function checkHttp(Config $config)
         }
 
         try {
-            $json = $this->rfs->getContents('packagist.org', $protocol . '://packagist.org/packages.json', false, array(), $disableTls);
+            $json = $this->rfs->getContents('packagist.org', $protocol . '://packagist.org/packages.json', false);
         } catch (\Exception $e) {
             array_unshift($result, '[' . get_class($e) . '] ' . $e->getMessage());
         }

diff --git a/src/Composer/Command/SelfUpdateCommand.php b/src/Composer/Command/SelfUpdateCommand.php
@@ -75,11 +75,13 @@ protected function execute(InputInterface $input, OutputInterface $output)
         }
 
         $remoteFilesystemOptions = array();
-        if (!is_null($config->get('cafile'))) {
+        if ($disableTls === false) {
+            if (!is_null($config->get('cafile'))) {
             $remoteFilesystemOptions = array('ssl'=>array('cafile'=>$config->get('cafile')));
-        }
-        if (!is_null($input->get('cafile'))) {
-            $remoteFilesystemOptions = array('ssl'=>array('cafile'=>$input->get('cafile')));
+            }
+            if (!is_null($input->get('cafile'))) {
+                $remoteFilesystemOptions = array('ssl'=>array('cafile'=>$input->get('cafile')));
+            }
         }
         try {
             $remoteFilesystem = new RemoteFilesystem($this->getIO(), $remoteFilesystemOptions, $disableTls);

diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php
@@ -33,6 +33,7 @@ class RemoteFilesystem
     private $progress;
     private $lastProgress;
     private $options;
+    private $disableTls = false;
 
     /**
      * Constructor.
@@ -52,9 +53,11 @@ public function __construct(IOInterface $io, $options = array(), $disableTls = f
             $this->options = $this->getTlsDefaults();
             if (isset($options['ssl']['cafile'])
             && (!is_readable($options['ssl']['cafile'])
-            || !\openssl_x509_parse(file_get_contents($options['ssl']['cafile'])))) { //check return value and test (it's subject to change)
+            || !\openssl_x509_parse(file_get_contents($options['ssl']['cafile'])))) {
                 throw new TransportException('The configured cafile was not valid or could not be read.');
             }
+        } else {
+            $this->disableTls = true;
         }
 
         // handle the other externally set options normally.
@@ -72,9 +75,9 @@ public function __construct(IOInterface $io, $options = array(), $disableTls = f
      *
      * @return bool true
      */
-    public function copy($originUrl, $fileUrl, $fileName, $progress = true, $options = array(), $disableTls = false) //REFACTOR: to constructor for TLS opt
+    public function copy($originUrl, $fileUrl, $fileName, $progress = true, $options = array())
     {
-        return $this->get($originUrl, $fileUrl, $options, $fileName, $progress, $disableTls);
+        return $this->get($originUrl, $fileUrl, $options, $fileName, $progress);
     }
 
     /**
@@ -87,9 +90,9 @@ public function copy($originUrl, $fileUrl, $fileName, $progress = true, $options
      *
      * @return string The content
      */
-    public function getContents($originUrl, $fileUrl, $progress = true, $options = array(), $disableTls = false)
+    public function getContents($originUrl, $fileUrl, $progress = true, $options = array())
     {
-        return $this->get($originUrl, $fileUrl, $options, null, $progress, $disableTls);
+        return $this->get($originUrl, $fileUrl, $options, null, $progress);
     }
 
     /**
@@ -116,7 +119,7 @@ public function getOptions()
      *
      * @return bool|string
      */
-    protected function get($originUrl, $fileUrl, $additionalOptions = array(), $fileName = null, $progress = true, $disableTls = false)
+    protected function get($originUrl, $fileUrl, $additionalOptions = array(), $fileName = null, $progress = true)
     {
         $this->bytesMax = 0;
         $this->originUrl = $originUrl;
@@ -130,7 +133,7 @@ protected function get($originUrl, $fileUrl, $additionalOptions = array(), $file
             $this->io->setAuthentication($originUrl, urldecode($match[1]), urldecode($match[2]));
         }
 
-        $options = $this->getOptionsForUrl($originUrl, $additionalOptions, $disableTls);
+        $options = $this->getOptionsForUrl($originUrl, $additionalOptions);
 
         if ($this->io->isDebug()) {
             $this->io->write((substr($fileUrl, 0, 4) === 'http' ? 'Downloading ' : 'Reading ') . $fileUrl);
@@ -341,7 +344,7 @@ protected function getOptionsForUrl($originUrl, $additionalOptions, $disableTls
         }
 
         // Setup remaining TLS options - the matching may need monitoring, esp. www vs none in CN
-        if ($disableTls === false) {
+        if ($this->disableTls === false) {
             if (!preg_match("|^https?://|", $originUrl)) {
                 $host = $originUrl;
             } else {

diff --git a/tests/Composer/Test/Mock/RemoteFilesystemMock.php b/tests/Composer/Test/Mock/RemoteFilesystemMock.php
@@ -28,7 +28,7 @@ public function __construct(array $contentMap)
         $this->contentMap = $contentMap;
     }
 
-    public function getContents($originUrl, $fileUrl, $progress = true, $options = array(), $disableTls = false)
+    public function getContents($originUrl, $fileUrl, $progress = true, $options = array())
     {
         if (!empty($this->contentMap[$fileUrl])) {
             return $this->contentMap[$fileUrl];