#602520 by JacobSingh and dww: Make FileTransfer form have one option for FTP (ftp_extension() preferred).

3 files changed, 63 insertions, 59 deletions

diff --git a/includes/filetransfer/filetransfer.inc b/includes/filetransfer/filetransfer.inc
index 921b9fa53..f9182497b 100644
--- a/includes/filetransfer/filetransfer.inc
+++ b/includes/filetransfer/filetransfer.inc
@@ -72,14 +72,15 @@ abstract class FileTransfer {
    * @param bool $recursive
    */
   public final function chmod($path, $mode, $recursive = FALSE) {
+    if (!in_array('FileTransferChmodInterface', class_implements(get_class($this)))) {
+      throw new FileTransferException('Unable to change file permissions');
+    }
     $path = $this->sanitizePath($path);
     $path = $this->fixRemotePath($path);
     $this->checkPath($path);
     $this->chmodJailed($path, $mode, $recursive);
   }
-
-  protected abstract function chmodJailed($path, $mode, $recursive);
-
+  
   /**
    * Creates a directory.
    *
@@ -314,4 +315,23 @@ class FileTransferException extends Exception {
     parent::__construct($message, $code);
     $this->arguments = $arguments;
   }
-}
\ No newline at end of file
+}
+
+
+/**
+ * A FileTransfer Class implementing this interface can be used to chmod files.
+ */
+interface FileTransferChmodInterface {
+
+  /**
+   * Changes the permissions of the file / directory specified in $path
+   *
+   * @param string $path
+   *   Path to change permissions of.
+   * @param long $mode
+   *   @see http://php.net/chmod
+   * @param boolean $recursive
+   *   Pass TRUE to recursively chmod the entire directory specified in $path.
+   */
+  function chmodJailed($path, $mode, $recursive);
+}
diff --git a/includes/filetransfer/ftp.inc b/includes/filetransfer/ftp.inc
index eb10c96b5..e4d83bcc9 100644
--- a/includes/filetransfer/ftp.inc
+++ b/includes/filetransfer/ftp.inc
@@ -2,10 +2,10 @@
 // $Id$
 
 /**
- * Connection class using the FTP URL wrapper.
+ * Base class for FTP implementations.
  */
-class FileTransferFTPWrapper extends FileTransfer {
-
+abstract class FileTransferFTP extends FileTransfer {
+  
   public function __construct($jail, $username, $password, $hostname, $port) {
     $this->username = $username;
     $this->password = $password;
@@ -13,6 +13,39 @@ class FileTransferFTPWrapper extends FileTransfer {
     $this->port = $port;
     parent::__construct($jail);
   }
+
+  /**
+   * Return an object which can implement the FTP protocol.
+   *
+   * @param string $jail
+   * @param array $settings
+   * @return
+   *   The appropriate FileTransferFTP subclass based on the available
+   *   options.  If the FTP PHP extension is available, use it.  Otherwise, we
+   *   try to use the FTP file stream support.
+   */
+  static function factory($jail, $settings) {
+    $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname'];
+    $settings['port'] = empty($settings['port']) ? 21 : $settings['port'];
+    
+    if (function_exists('ftp_connect')) {
+      $class = 'FileTransferFTPExtension';
+    }
+    elseif (ini_get('allow_url_fopen')) {
+      $class = 'FileTransferFTPWrapper';
+    }
+    else {
+      throw new FileTransferException('No FTP backend available.');
+    }
+
+    return new $class($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']);
+  }
+}
+
+/**
+ * Connection class using the FTP URL wrapper.
+ */
+class FileTransferFTPWrapper extends FileTransferFTP {
   
   function connect() {
     $this->connection = 'ftp://' . urlencode($this->username) . ':' . urlencode($this->password) . '@' . $this->hostname . ':' . $this->port . '/';
@@ -20,12 +53,6 @@ class FileTransferFTPWrapper extends FileTransfer {
       throw new FileTransferException('FTP Connection failed.');
     }
   }
-  
-  static function factory($jail, $settings) {
-    $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname'];
-    $settings['port'] = empty($settings['port']) ? 21 : $settings['port'];
-    return new FileTransferFTPWrapper($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']);
-  }
 
   function createDirectoryJailed($directory) {
     if (!@drupal_mkdir($this->connection . $directory)) {
@@ -77,39 +104,9 @@ class FileTransferFTPWrapper extends FileTransfer {
     // This is stupid, but is_file and file_exists don't work! always return true.
     return @fopen($this->connection . '/' . $path,'r');
   }
-
-  /**
-   * This is impossible with the stream wrapper, so an exception is thrown.
-   *
-   * If the ftp extenstion is available, we will cheat and use that instead.
-   *
-   * @param string $path
-   * @param long $mode
-   * @param bool $recursive
-   */
-  function chmodJailed($path, $mode, $recursive) {
-    if (!function_exists('ftp_connect')) {
-      throw new FileTransferException('Unable to set permissions on @path.  Change umask settings on server to be world executable.', array('@path' => $path));
-    }
-    static $ftp_ext_file_transfer;
-
-    if (!$ftp_ext_file_transfer) {
-      $ftp_ext_file_transfer = new FileTransferFTPExtension($this->jail, $this->username, $this->password, $this->hostname, $this->port);
-    }
-    $ftp_ext_file_transfer->chmodJailed($path, $mode, $recursive);
-  }
-
 }
 
-class FileTransferFTPExtension extends FileTransfer {
-
-  public function __construct($jail, $username, $password, $hostname, $port) {
-    $this->username = $username;
-    $this->password = $password;
-    $this->hostname = $hostname;
-    $this->port = $port;
-    parent::__construct($jail);
-  }
+class FileTransferFTPExtension extends FileTransferFTP implements FileTransferChmodInterface {
 
   public function connect() {
     $this->connection = ftp_connect($this->hostname, $this->port);
@@ -122,19 +119,6 @@ class FileTransferFTPExtension extends FileTransfer {
     }
   }
 
-  /**
-   * Returns a copy of itself using common defaults.
-   *
-   * @param string $jail
-   * @param array $settings
-   * @return FileTransferFTPExtension
-   */
-  static function factory($jail, $settings) {
-    $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname'];
-    $settings['port'] = empty($settings['port']) ? 21 : $settings['port'];
-    return new FileTransferFTPExtension($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']);
-  }
-
   protected function copyFileJailed($source, $destination) {
     if (!@ftp_put($this->connection,  $destination, $source, FTP_BINARY)) {
       throw new FileTransferException("Cannot move @source to @destination", NULL, array("@source" => $source, "@destination" => $destination));
@@ -215,4 +199,4 @@ if (!function_exists('ftp_chmod')) {
   function ftp_chmod($ftp_stream, $mode, $filename) {
     return ftp_site($ftp_stream, sprintf('CHMOD %o %s', $mode, $filename));
   }
-}
\ No newline at end of file
+}
diff --git a/includes/filetransfer/ssh.inc b/includes/filetransfer/ssh.inc
index 774e136d6..4ba03c1ae 100644
--- a/includes/filetransfer/ssh.inc
+++ b/includes/filetransfer/ssh.inc
@@ -4,7 +4,7 @@
 /**
  * The SSH connection class for the update module.
  */
-class FileTransferSSH extends FileTransfer {
+class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface {
 
   function __construct($jail, $username, $password, $hostname = "localhost", $port = 22) {
     $this->username = $username;