- Patch #395472 by JacobSingh, chx, cwgordon7: more improvements to the file transfer system to pave the path for a plugin manager. Heroic effort.

3 files changed, 202 insertions, 75 deletions

diff --git a/includes/filetransfer/filetransfer.inc b/includes/filetransfer/filetransfer.inc
index 2fb769913..66e0f5787 100644
--- a/includes/filetransfer/filetransfer.inc
+++ b/includes/filetransfer/filetransfer.inc
@@ -2,28 +2,34 @@
 // $Id$
 
 /*
- * Connection class.
+ * Base FileTransfer class.
  *
- * This class does file operations on directories not writeable by the
- * webserver. It connects back to the server using some backend (for example
- * FTP or SSH). To keep security the password should always be asked from the
- * user and never stored.
+ * Classes extending this class perform file operations on directories not
+ * writeable by the webserver. To achieve this, the class should connect back
+ * to the server using some backend (for example FTP or SSH). To keep security,
+ * the password should always be asked from the user and never stored. For
+ * safety, all methods operate only inside a "jail", by default the Drupal root.
  */
 abstract class FileTransfer {
+  protected $username;
+  protected $password;
+  protected $hostname = 'localhost';
+  protected $port;
 
   /**
    * The constructer for the UpdateConnection class. This method is also called
    * from the classes that extend this class and override this method.
    */
-  function __construct($settings) {
-    $this->username = $settings['username'];
-    $this->password = $settings['password'];
-    $this->hostname = isset($settings['hostname']) ? $settings['hostname'] : 'localhost';
-    if (isset($settings['port'])) {
-      $this->port = $settings['port'];
-    }
+  function __construct($jail, $username, $password, $hostname, $port) {
+    $this->username = $username;
+    $this->password = $password;
+    $this->hostname = $hostname;
+    $this->port = $port;
+    $this->jail = $jail;
   }
 
+  abstract static function factory($jail, $settings);
+
   /**
    * Implementation of the magic __get() method. If the connection isn't set to
    * anything, this will call the connect() method and set it to and return the
@@ -31,25 +37,105 @@ abstract class FileTransfer {
    * this method.
    */
   function __get($name) {
-    static $connection;
     if ($name == 'connection') {
-      $this->connection = $this->connect();
+      $this->connect();
       return $this->connection;
     }
   }
 
   /**
+   * Connect to the server.
+   */
+  abstract protected function connect();
+
+  /**
+   * Copies a directory.
+   *
+   * @param $source
+   *   The source path.
+   * @param $destination
+   *   The destination path.
+   */
+  public final function copyDirectory($source, $destination) {
+    $this->checkPath($destination);
+    $this->copyDirectoryJailed($source, $destination);
+  }
+
+  /**
+   * Creates a directory.
+   *
+   * @param $directory
+   *   The directory to be created.
+   */
+  public final function createDirectory($directory) {
+    $this->checkPath($directory);
+    $this->createDirectoryJailed($directory);
+  }
+
+  /**
+   * Removes a directory.
+   *
+   * @param $directory
+   *   The directory to be removed.
+   */
+  public final function removeDirectory($directory) {
+    $this->checkPath($directory);
+    $this->removeDirectoryJailed($directory);
+  }
+
+  /**
+   * Copies a file.
+   *
+   * @param $source
+   *   The source file.
+   * @param $destination
+   *   The destination file.
+   */
+  public final function copyFile($source, $destination) {
+    $this->checkPath($destination);
+    $this->copyFileJailed($source, $destination);
+  }
+
+  /**
+   * Removes a file.
+   *
+   * @param $destination
+   *   The destination file to be removed.
+   */
+  public final function removeFile($destination) {
+    $this->checkPath($destination);
+    $this->removeFileJailed($destination);
+  }
+
+  /**
+   * Checks that the path is inside the jail and throws an exception if not.
+   *
+   * @param $path
+   *   A path to check against the jail.
+   */
+  protected final function checkPath($path) {
+    if (realpath(substr($path, 0, strlen($this->jail))) !== $this->jail) {
+      throw new FileTransferException('@directory is outside of the @jail', NULL, array('@directory' => $path, '@jail' => $this->jail));
+    }
+  }
+
+  /**
    * Copies a directory.
    *
+   * We need a separate method to make the $destination is in the jail.
+   *
    * @param $source
    *   The source path.
    * @param $destination
    *   The destination path.
    */
-  protected function copyDirectory($source, $destination) {
-    $this->createDirectory($destination . basename($source));
+  protected function copyDirectoryJailed($source, $destination) {
+    if ($this->isDirectory($destination)) {
+      $destination = $destination . '/' . basename($source);
+    }
+    $this->createDirectory($destination);
     foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($source), RecursiveIteratorIterator::SELF_FIRST) as $filename => $file) {
-      $relative_path = basename($source) . substr($filename, strlen($source));
+      $relative_path = substr($filename, strlen($source));
       if ($file->isDir()) {
         $this->createDirectory($destination . $relative_path);
       }
@@ -65,7 +151,7 @@ abstract class FileTransfer {
    * @param $directory
    *   The directory to be created.
    */
-  abstract function createDirectory($directory);
+  abstract protected function createDirectoryJailed($directory);
 
   /**
    * Removes a directory.
@@ -73,7 +159,7 @@ abstract class FileTransfer {
    * @param $directory
    *   The directory to be removed.
    */
-  abstract function removeDirectory($directory);
+  abstract protected function removeDirectoryJailed($directory);
 
   /**
    * Copies a file.
@@ -83,8 +169,7 @@ abstract class FileTransfer {
    * @param $destination
    *   The destination file.
    */
-  abstract function copyFile($source, $destination);
-
+  abstract protected function copyFileJailed($source, $destination);
 
   /**
    * Removes a file.
@@ -92,11 +177,27 @@ abstract class FileTransfer {
    * @param $destination
    *   The destination file to be removed.
    */
-  abstract function removeFile($destination);
+  abstract protected function removeFileJailed($destination);
+  
+  /**
+   * Checks if a particular path is a directory
+   *
+   * @param $path
+   *   The path to check
+   *
+   * @return boolean
+   */
+  abstract public function isDirectory($path);
 }
 
 /**
  * FileTransferException class.
  */
 class FileTransferException extends Exception {
+  public $arguments;
+
+  function __construct($message, $code = 0, $arguments = array()) {
+    parent::__construct($message, $code);
+    $this->arguments = $arguments;
+  }
 }
diff --git a/includes/filetransfer/ftp.inc b/includes/filetransfer/ftp.inc
index d21e88bec..ecb164a17 100644
--- a/includes/filetransfer/ftp.inc
+++ b/includes/filetransfer/ftp.inc
@@ -2,17 +2,6 @@
 // $Id$
 
 /**
- * Common code for the FTP connections.
- */
-abstract class FileTransferFTP extends FileTransfer {
-  function __construct($settings) {
-    // This is the default, if $settings contains a port, this will be overridden.
-    $this->port = 21;
-    parent::__construct($settings);
-  }
-}
-
-/**
  * Connection class using the FTP URL wrapper.
  */
 class FileTransferFTPWrapper extends FileTransfer {
@@ -22,18 +11,21 @@ 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 createDirectory($directory) {
-    if (!@createDirectory($directory)) {
+  function createDirectoryJailed($directory) {
+    if (!@mkdir($directory)) {
       $exception = new FileTransferException('Cannot create directory @directory.', NULL, array('@directory' => $directory));
       throw $exception;
     }
   }
 
-  function removeDirectory($directory) {
-    if (realpath(substr($directory, 0, strlen(DRUPAL_ROOT))) !== DRUPAL_ROOT) {
-      throw new FileTransferException('@directory is outside of the Drupal root.', NULL, array('@directory' => $directory));
-    }
+  function removeDirectoryJailed($directory) {
     if (is_dir($directory)) {
       $dh = opendir($directory);
       while (($resource = readdir($dh)) !== FALSE) {
@@ -49,28 +41,32 @@ class FileTransferFTPWrapper extends FileTransfer {
         }
       }
       closedir($dh);
-      if (!removeDirectory($directory)) {
+      if (!rmdir($directory)) {
         $exception = new FileTransferException('Cannot remove @directory.', NULL, array('@directory' => $directory));
         throw $exception;
       }
     }
   }
 
-  function copyFile($source, $destination) {
+  function copyFileJailed($source, $destination) {
     if (!@copy($this->connection . '/' . $source, $this->connection . '/' . $destination)) {
       throw new FileTransferException('Cannot copy @source_file to @destination_file.', NULL, array('@source' => $source, '@destination' => $destination));
     }
   }
 
-  function removeFile($destination) {
+  function removeFileJailed($destination) {
     if (!@unlink($destination)) {
       throw new FileTransferException('Cannot remove @destination', NULL, array('@destination' => $destination));
     }
   }
+  
+  function isDirectory($path) {
+    return is_dir($this->connection . '/' . $path);
+  }
 }
 
 class FileTransferFTPExtension extends FileTransfer {
-  function connect() {
+  public function connect() {
     $this->connection = ftp_connect($this->hostname, $this->port);
 
     if (!$this->connection) {
@@ -80,23 +76,26 @@ class FileTransferFTPExtension extends FileTransfer {
       throw new FileTransferException("Cannot login to FTP server, please check username and password");
     }
   }
+  
+  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']);
+  }
 
-  function copyFile($source, $destination) {
+  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));
     }
   }
 
-  function createDirectory($directory) {
-    if (!@ftp_createDirectory($this->connection, $directory)) {
+  protected function createDirectoryJailed($directory) {
+    if (!@ftp_mkdir($this->connection, $directory)) {
       throw new FileTransferException("Cannot create directory @directory", NULL, array("@directory" => $directory));
     }
   }
 
-  function removeDirectory($directory) {
-    if (realpath(substr($directory, 0, strlen(DRUPAL_ROOT))) !== DRUPAL_ROOT) {
-      throw new FileTransferException('@directory is outside of the Drupal root.', NULL, array('@directory' => $directory));
-    }
+  protected function removeDirectoryJailed($directory) {
     $pwd = ftp_pwd($this->connection);
     if (!@ftp_chdir($this->connection, $directory)) {
       throw new FileTransferException("Unable to change to directory @directory", NULL, array('@directory' => $directory));
@@ -107,22 +106,32 @@ class FileTransferFTPExtension extends FileTransfer {
         continue;
       }
       if (@ftp_chdir($this->connection, $item)){
-        ftp_chdir($this->connection, '..');
-        $this->removeDirectory($item);
+        ftp_cdup($this->connection);
+        $this->removeDirectory(ftp_pwd($this->connection) . '/' . $item);
       }
       else {
-        $this->removeFile($item);
+        $this->removeFile(ftp_pwd($this->connection) . '/' . $item);
       }
     }
     ftp_chdir($this->connection, $pwd);
-    if (!ftp_removeDirectory($this->connection, $directory)) {
+    if (!ftp_rmdir($this->connection, $directory)) {
       throw new FileTransferException("Unable to remove to directory @directory", NULL, array('@directory' => $directory));
     }
   }
 
-  function removeFile($destination) {
-    if (!ftp_delete($this->connection, $item)) {
-      throw new FileTransferException("Unable to remove to file @file", NULL, array('@file' => $item));
+  protected function removeFileJailed($destination) {
+    if (!ftp_delete($this->connection, $destination)) {
+      throw new FileTransferException("Unable to remove to file @file", NULL, array('@file' => $destination));
+    }
+  }
+  
+  public function isDirectory($path) {
+    $result = FALSE;
+    $curr = ftp_pwd($this->connection);
+    if (ftp_chdir($this->connection, $path)) {
+      $result = TRUE;
     }
+    ftp_chdir($this->connection, $curr);
+    return $result;
   }
 }
diff --git a/includes/filetransfer/ssh.inc b/includes/filetransfer/ssh.inc
index 846b976dd..e6148c8f0 100644
--- a/includes/filetransfer/ssh.inc
+++ b/includes/filetransfer/ssh.inc
@@ -6,52 +6,69 @@
  */
 class FileTransferSSH extends FileTransfer {
 
-  function __construct($settings) {
-    // This is the default, if $settings contains a port, this will be overridden.
-    $this->port = 22;
-    parent::__construct($settings);
+  function __construct($jail, $username, $password, $hostname = "localhost", $port = 22) {
+    parent::__construct($jail, $username, $password, $hostname, $port);
   }
 
   function connect() {
-    $this->connection = @ssh2_connect($setings['hostname'], $this->port);
+    $this->connection = @ssh2_connect($this->hostname, $this->port);
     if (!$this->connection) {
-      throw new FileTransferException('SSH Connection failed.');
+      throw new FileTransferException('SSH Connection failed to @host:@port', NULL, array('@host' => $this->hostname, '@port' => 21));
     }
     if (!@ssh2_auth_password($this->connection, $this->username, $this->password)) {
       throw new FileTransferException('The supplied username/password combination was not accepted.');
     }
   }
 
-  function copyFile($source, $destination) {
+  static function factory($jail, $settings) {
+    $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname'];
+    $settings['port'] = empty($settings['port']) ? 22 : $settings['port'];
+    return new FileTransferSSH($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']);
+  }
+
+  protected function copyFileJailed($source, $destination) {
     if (!@ssh2_scp_send($this->connection, $source, $destination)) {
       throw new FileTransferException('Cannot copy @source_file to @destination_file.', NULL, array('@source' => $source, '@destination' => $destination));
     }
   }
 
-  function copyDirectory($source, $destination) {
-    if (!@ssh2_exec($this->connection, 'cp -Rp ' . escapeshellarg($source) . ' ' . escapeshellarg($destination))) {
+  protected function copyDirectoryJailed($source, $destination) {
+    if (@!ssh2_exec($this->connection, 'cp -Rp ' . escapeshellarg($source) . ' ' . escapeshellarg($destination))) {
       throw new FileTransferException('Cannot copy directory @directory.', NULL, array('@directory' => $source));
     }
   }
 
-  function createDirectory($directory) {
-    if (!@ssh2_exec($this->connection, 'mkdir ' . escapeshellarg($directory))) {
+  protected function createDirectoryJailed($directory) {
+    if (@!ssh2_exec($this->connection, 'mkdir ' . escapeshellarg($directory))) {
       throw new FileTransferException('Cannot create directory @directory.', NULL, array('@directory' => $directory));
     }
   }
 
-  function removeDirectory($directory) {
-    if (realpath(substr($directory, 0, strlen(DRUPAL_ROOT))) !== DRUPAL_ROOT) {
-      throw new FileTransferException('@directory is outside of the Drupal root.', NULL, array('@directory' => $directory));
-    }
-    if (!@ssh2_exec($this->connection, 'rm -Rf ' . escapeshellarg($directory))) {
+  protected function removeDirectoryJailed($directory) {
+    if (@!ssh2_exec($this->connection, 'rm -Rf ' . escapeshellarg($directory))) {
       throw new FileTransferException('Cannot remove @directory.', NULL, array('@directory' => $directory));
     }
   }
-  
-  function removeFile($destination) {
+
+  protected function removeFileJailed($destination) {
     if (!@ssh2_exec($this->connection, 'rm ' . escapeshellarg($destination))) {
       throw new FileTransferException('Cannot remove @directory.', NULL, array('@directory' => $destination));
     }
   }
+  
+  /**
+   * WARNING: This is untested.  It is not currently used, but should do the trick.
+   */
+  public function isDirectory($path) {
+    $directory = escapeshellarg($path);
+    $cmd = "[ -d {$directory} ] && echo 'yes'";
+    if ($output = @ssh2_exec($this->connection, $cmd)) {
+      if ($output == 'yes') {
+        return TRUE;
+      }
+      return FALSE;
+    } else {
+      throw new FileTransferException('Cannot check @path.', NULL, array('@path' => $path));
+    }
+  }
 }