#609772 by dww, JacobSingh, ksenzee: Fixed Impossible to extend the FileTransfer class system in contrib

Author: webchick

authorize.php

@@ -125,7 +125,7 @@ function authorize_access_allowed() {
     // Clear the session out.
     unset($_SESSION['authorize_results']);
     unset($_SESSION['authorize_operation']);
-    unset($_SESSION['authorize_filetransfer_backends']);
+    unset($_SESSION['authorize_filetransfer_info']);
 
     if (!empty($results['page_title'])) {
       drupal_set_title(check_plain($results['page_title']));
@@ -154,7 +154,7 @@ function authorize_access_allowed() {
     $output = _batch_page();
   }
   else {
-    if (empty($_SESSION['authorize_operation']) || empty($_SESSION['authorize_filetransfer_backends'])) {
+    if (empty($_SESSION['authorize_operation']) || empty($_SESSION['authorize_filetransfer_info'])) {
       $output = t('It appears you have reached this page in error.');
     }
     elseif (!$batch = batch_get()) {

includes/authorize.inc

@@ -21,12 +21,11 @@ function authorize_filetransfer_form($form_state) {
   $form['#attached']['js'][] = $base_url . '/misc/authorize.js';
 
   // Get all the available ways to transfer files.
-  if (empty($_SESSION['authorize_filetransfer_backends'])) {
+  if (empty($_SESSION['authorize_filetransfer_info'])) {
     drupal_set_message(t('Unable to continue, no available methods of file transfer'), 'error');
     return array();
   }
-  $available_backends = $_SESSION['authorize_filetransfer_backends'];
-  uasort($available_backends, 'drupal_sort_weight');
+  $available_backends = $_SESSION['authorize_filetransfer_info'];
 
   if (!$is_https) {
     drupal_set_message(t('WARNING: You are not using an encrypted connection, so your password will be sent in plain text. <a href="@https-link">Learn more</a>.', array('@https-link' => 'http://drupal.org/https-information')), 'error');
@@ -87,8 +86,7 @@ function authorize_filetransfer_form($form_state) {
       '#title' => t('@backend connection settings', array('@backend' => $backend['title'])),
     );
 
-    $current_settings = variable_get('authorize_filetransfer_connection_settings_' . $name, array());
-    $form['connection_settings'][$name] += system_get_filetransfer_settings_form($name, $current_settings);
+    $form['connection_settings'][$name] += _authorize_filetransfer_connection_settings($name);
 
     // Start non-JS code.
     if (isset($form_state['values']['connection_settings']['authorize_filetransfer_default']) && $form_state['values']['connection_settings']['authorize_filetransfer_default'] == $name) {
@@ -121,6 +119,65 @@ function authorize_filetransfer_form($form_state) {
   return $form;
 }
 
+/**
+ * Generate the Form API array for the settings for a given connection backend.
+ *
+ * @param $backend
+ *   The name of the backend (e.g. 'ftp', 'ssh', etc).
+ * @return
+ *   Form API array of connection settings for the given backend.
+ *
+ * @see hook_filetransfer_backends()
+ */
+function _authorize_filetransfer_connection_settings($backend) {
+  $defaults = variable_get('authorize_filetransfer_connection_settings_' . $backend, array());
+  $form = array();
+
+  // Create an instance of the file transfer class to get its settings form.
+  $filetransfer = authorize_get_filetransfer($backend);
+  if ($filetransfer) {
+    $form = $filetransfer->getSettingsForm();
+  }
+  // Fill in the defaults based on the saved settings, if any.
+  _authorize_filetransfer_connection_settings_set_defaults($form, NULL, $defaults);
+  return $form;
+}
+
+/**
+ * Recursively fill in the default settings on a file transfer connection form.
+ *
+ * The default settings for the file transfer connection forms are saved in
+ * the database. The settings are stored as a nested array in the case of a
+ * settings form that has fieldsets or otherwise uses a nested structure.
+ * Therefore, to properly add defaults, we need to walk through all the
+ * children form elements and process those defaults recursively.
+ *
+ * @param &$element
+ *   Reference to the Form API form element we're operating on.
+ * @param $key
+ *   The key for our current form element, if any.
+ * @param array $defaults
+ *   The default settings for the file transfer backend we're operating on.
+ * @return
+ *   Nothing, this function just sets $element['#default_value'] if needed.
+ */
+function _authorize_filetransfer_connection_settings_set_defaults(&$element, $key, array $defaults) {
+  // If we're operating on a form element which isn't a fieldset, and we have
+  // a default setting saved, stash it in #default_value.
+  if (!empty($key) && isset($defaults[$key]) && isset($element['#type']) && $element['#type'] != 'fieldset') {
+    $element['#default_value'] = $defaults[$key];
+  }
+  // Now, we walk through all the child elements, and recursively invoke
+  // ourself on each one. Since the $defaults settings array can be nested
+  // (because of #tree, any values inside fieldsets will be nested), if
+  // there's a subarray of settings for the form key we're currently
+  // processing, pass in that subarray to the recursive call. Otherwise, just
+  // pass on the whole $defaults array.
+  foreach (element_children($element) as $child_key) {
+    _authorize_filetransfer_connection_settings_set_defaults($element[$child_key], $child_key, ((isset($defaults[$key]) && is_array($defaults[$key])) ? $defaults[$key] : $defaults));
+  }
+}
+
 /**
  * Validate callback for the filetransfer authorization form.
  *
@@ -235,9 +292,17 @@ function authorize_run_operation($filetransfer) {
  */
 function authorize_get_filetransfer($backend, $settings = array()) {
   $filetransfer = FALSE;
-  if (!empty($_SESSION['authorize_filetransfer_backends'][$backend])) {
-    $filetransfer = call_user_func_array(array($_SESSION['authorize_filetransfer_backends'][$backend]['class'], 'factory'), array(DRUPAL_ROOT, $settings));
+  if (!empty($_SESSION['authorize_filetransfer_info'][$backend])) {
+    $backend_info = $_SESSION['authorize_filetransfer_info'][$backend];
+    if (!empty($backend_info['file'])) {
+      $file = $backend_info['file path'] . '/' . $backend_info['file'];
+      require_once $file;
+    }
+    if (class_exists($backend_info['class'])) {
+      // PHP 5.2 doesn't support $class::factory() syntax, so we have to
+      // use call_user_func_array() until we can require PHP 5.3.
+      $filetransfer = call_user_func_array(array($backend_info['class'], 'factory'), array(DRUPAL_ROOT, $settings));
+    }
   }
   return $filetransfer;
 }
-

includes/common.inc

@@ -7679,3 +7679,39 @@ function drupal_get_updaters() {
   }
   return $updaters;
 }
+
+/**
+ * Drupal FileTransfer registry.
+ *
+ * @return
+ *   Returns the Drupal FileTransfer class registry.
+ *
+ * @see FileTransfer
+ * @see hook_filetransfer_info()
+ * @see hook_filetransfer_info_alter()
+ */
+function drupal_get_filetransfer_info() {
+  $info = &drupal_static(__FUNCTION__);
+  if (!isset($info)) {
+    // Since we have to manually set the 'file path' default for each
+    // module separately, we can't use module_invoke_all().
+    $info = array();
+    foreach (module_implements('filetransfer_info') as $module) {
+      $function = $module . '_filetransfer_info';
+      if (function_exists($function)) {
+        $result = $function();
+        if (isset($result) && is_array($result)) {
+          foreach ($result as &$values) {
+            if (empty($values['file path'])) {
+              $values['file path'] = drupal_get_path('module', $module);
+            }
+          }
+          $info = array_merge_recursive($info, $result);
+        }
+      }
+    }
+    drupal_alter('filetransfer_info', $info);
+    uasort($info, 'drupal_sort_weight');
+  }
+  return $info;
+}

includes/filetransfer/filetransfer.inc

@@ -313,6 +313,42 @@ abstract class FileTransfer {
     $this->chroot = $this->findChroot();
     $this->jail = $this->fixRemotePath($this->jail);
   }
+
+  /**
+   * Returns a form to collect connection settings credentials.
+   *
+   * Implementing classes can either extend this form with fields collecting the
+   * specific information they need, or override it entirely.
+   */
+  public function getSettingsForm() {
+    $form['username'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Username'),
+    );
+    $form['password'] = array(
+      '#type' => 'password',
+      '#title' => t('Password'),
+      '#description' => t('Your password is not saved in the database and is only used to establish a connection.'),
+    );
+    $form['advanced'] = array(
+      '#type' => 'fieldset',
+      '#title' => t('Advanced settings'),
+      '#collapsible' => TRUE,
+      '#collapsed' => TRUE,
+    );
+    $form['advanced']['hostname'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Host'),
+      '#default_value' => 'localhost',
+      '#description' => t('The connection will be created between your web server and the machine hosting the web server files. In the vast majority of cases, this will be the same machine, and "localhost" is correct.'),
+    );
+    $form['advanced']['port'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Port'),
+      '#default_value' => NULL,
+    );
+    return $form;
+  }
 }
 
 /**

includes/filetransfer/ftp.inc

@@ -24,6 +24,8 @@ abstract class FileTransferFTP extends FileTransfer {
    *   options. If the FTP PHP extension is available, use it.
    */
   static function factory($jail, $settings) {
+    $settings['username'] = empty($settings['username']) ? '' : $settings['username'];
+    $settings['password'] = empty($settings['password']) ? '' : $settings['password'];
     $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname'];
     $settings['port'] = empty($settings['port']) ? 21 : $settings['port'];
 
@@ -36,6 +38,15 @@ abstract class FileTransferFTP extends FileTransfer {
 
     return new $class($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']);
   }
+
+  /**
+   * Returns the form to configure the FileTransfer class for FTP.
+   */
+  public function getSettingsForm() {
+    $form = parent::getSettingsForm();
+    $form['advanced']['port']['#default_value'] = 21;
+    return $form;
+  }
 }
 
 class FileTransferFTPExtension extends FileTransferFTP implements FileTransferChmodInterface {

includes/filetransfer/ssh.inc

@@ -25,6 +25,8 @@ class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface
   }
 
   static function factory($jail, $settings) {
+    $settings['username'] = empty($settings['hostname']) ? '' : $settings['username'];
+    $settings['password'] = empty($settings['password']) ? '' : $settings['password'];
     $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']);
@@ -95,4 +97,13 @@ class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface
       throw new FileTransferException('Cannot change permissions of @path.', NULL, array('@path' => $path));
     }
   }
+
+  /**
+   * Returns the form to configure the FileTransfer class for SSH.
+   */
+  public function getSettingsForm() {
+    $form = parent::getSettingsForm();
+    $form['advanced']['port']['#default_value'] = 22;
+    return $form;
+  }
 }

modules/simpletest/tests/system_test.module

@@ -16,6 +16,12 @@ function system_test_menu() {
     'access callback' => TRUE,
     'type' => MENU_CALLBACK,
   );
+  $items['system-test/authorize-init/%'] = array(
+    'page callback' => 'system_test_authorize_init_page',
+    'page arguments' => array(2),
+    'access arguments' => array('administer software updates'),
+    'type' => MENU_CALLBACK,
+  );
   $items['system-test/redirect/%'] = array(
     'title' => 'Redirect',
     'page callback' => 'system_test_redirect',
@@ -311,3 +317,45 @@ function _system_test_second_shutdown_function($arg1, $arg2) {
   throw new Exception('Drupal is <blink>awesome</blink>.');
 }
 
+/**
+ * Implements hook_filetransfer_info().
+ */
+function system_test_filetransfer_info() {
+  return array(
+    'system_test' => array(
+      'title' => t('System Test FileTransfer'),
+      'file' => 'system_test.module',  // Should be a .inc, but for test, ok.
+      'class' => 'SystemTestFileTransfer',
+      'weight' => -10,
+    ),
+  );
+}
+
+/**
+ * Mock FileTransfer object to test the settings form functionality.
+ */
+class SystemTestFileTransfer {
+  public static function factory() {
+    return new SystemTestFileTransfer;
+  }
+
+  public function getSettingsForm() {
+    $form = array();
+    $form['system_test_username'] = array(
+      '#type' => 'textfield',
+      '#title' => t('System Test Username'),
+    );
+    return $form;
+  }
+}
+
+/**
+ * Page callback to initialize authorize.php during testing.
+ *
+ * @see system_authorized_init().
+ */
+function system_test_authorize_init_page($page_title) {
+  $authorize_url = $GLOBALS['base_url'] . '/authorize.php';
+  system_authorized_init('system_test_authorize_run', drupal_get_path('module', 'system_test') . '/system_test.module', array(), $page_title);
+  drupal_goto($authorize_url);
+}