#551658 by pwolanin, aaron, drewish: Move private files to an opt-in system, and no longer force private files to live within web-accessible directory.

6 files changed, 42 insertions, 24 deletions

diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index 5aff8f11c..b1ed5ada2 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -19,7 +19,7 @@ define('DRUPAL_CORE_COMPATIBILITY', '7.x');
 /**
  * Minimum supported version of PHP.
  */
-define('DRUPAL_MINIMUM_PHP',    '5.2.0');
+define('DRUPAL_MINIMUM_PHP',    '5.2.1');
 
 /**
  * Minimum recommended value of PHP memory_limit.
diff --git a/includes/file.inc b/includes/file.inc
index 96da7ad4e..da47b3590 100644
--- a/includes/file.inc
+++ b/includes/file.inc
@@ -409,7 +409,9 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS)
  */
 function file_ensure_htaccess() {
   file_create_htaccess('public://', FALSE);
-  file_create_htaccess('private://', TRUE);
+  if (variable_get('file_private_path', FALSE)) {
+    file_create_htaccess('private://', TRUE);
+  }
   file_create_htaccess('temporary://', TRUE);
 }
 
@@ -1586,8 +1588,7 @@ function file_download() {
   $scheme = array_shift($args);
   $target = implode('/', $args);
   $uri = $scheme . '://' . $target;
-
-  if (file_exists($uri)) {
+  if (file_stream_wrapper_valid_scheme($scheme) && file_exists($uri)) {
     // Let other modules provide headers and controls access to the file.
     $headers = module_invoke_all('file_download', $uri);
     if (in_array(-1, $headers)) {
diff --git a/includes/stream_wrappers.inc b/includes/stream_wrappers.inc
index 288c3979d..87acc5440 100644
--- a/includes/stream_wrappers.inc
+++ b/includes/stream_wrappers.inc
@@ -657,7 +657,7 @@ class DrupalPrivateStreamWrapper extends DrupalLocalStreamWrapper {
    * Implements abstract public function getDirectoryPath()
    */
   public function getDirectoryPath() {
-    return variable_get('file_private_path', conf_path() . '/private/files');
+    return variable_get('file_private_path', '');
   }
 
   /**
@@ -684,7 +684,7 @@ class DrupalTemporaryStreamWrapper extends DrupalLocalStreamWrapper {
    * Implements abstract public function getDirectoryPath()
    */
   public function getDirectoryPath() {
-    return variable_get('file_temporary_path', conf_path() . '/private/temp');
+    return variable_get('file_temporary_path', sys_get_temp_dir());
   }
 
   /**
diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc
index 149b0819c..ec20138a1 100644
--- a/modules/system/system.admin.inc
+++ b/modules/system/system.admin.inc
@@ -1725,15 +1725,18 @@ function system_file_system_settings() {
   // Any visible, writeable wrapper can potentially be used for the files
   // directory, including a remote file system that integrates with a CDN.
   foreach(file_get_stream_wrappers(STREAM_WRAPPERS_WRITE_VISIBLE) as $scheme => $info) {
-    $options[$scheme] = $info['description'];
+    $options[$scheme] = check_plain($info['description']);
+  }
+
+  if (!empty($options)) {
+    $form['file_default_scheme'] = array(
+      '#type' => 'radios',
+      '#title' => t('Default download method'),
+      '#default_value' => isset($options['public']) ? 'public' : key($options),
+      '#options' => $options,
+      '#description' => t('This setting is used as the preferred download method. The use of public files is more efficient, but does not provide any access control.'),
+    );
   }
-  $form['file_default_scheme'] = array(
-    '#type' => 'radios',
-    '#title' => t('Default download method'),
-    '#default_value' => 'public',
-    '#options' => $options,
-    '#description' => t('This setting is used as the preferred download method. The use of public files is more efficient, but does not provide any access control.'),
-  );
 
   return system_settings_form($form, TRUE);
 }
diff --git a/modules/system/system.install b/modules/system/system.install
index d26a203f5..8e04d51ff 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -264,8 +264,10 @@ function system_requirements($phase) {
   // Test files directories.
   $directories = array(
     variable_get('file_public_path', conf_path() . '/files'),
-    variable_get('file_private_path', conf_path() . '/private/files'),
-    variable_get('file_temporary_path', conf_path() . '/private/temp'),
+    // By default no private files directory is configured. For private files
+    // to be secure the admin needs to provide a path outside the webroot.
+    variable_get('file_private_path', FALSE),
+    variable_get('file_temporary_path', sys_get_temp_dir()),
   );
   $requirements['file system'] = array(
     'title' => $t('File system'),
@@ -274,6 +276,9 @@ function system_requirements($phase) {
   $error = '';
   // For installer, create the directories if possible.
   foreach ($directories as $directory) {
+    if (!$directory) {
+      continue;
+    }
     if ($phase == 'install') {
       file_prepare_directory($directory, FILE_CREATE_DIRECTORY);
     }
diff --git a/modules/system/system.module b/modules/system/system.module
index f24c3af9c..21ad835fc 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -1517,24 +1517,30 @@ function system_library() {
  * Implements hook_stream_wrappers().
  */
 function system_stream_wrappers() {
-  return array(
+  $wrappers = array(
     'public' => array(
       'name' => t('Public files'),
       'class' => 'DrupalPublicStreamWrapper',
       'description' => t('Public local files served by the webserver.'),
     ),
-    'private' => array(
-      'name' => t('Private files'),
-      'class' => 'DrupalPrivateStreamWrapper',
-      'description' => t('Private local files served by Drupal.'),
-    ),
     'temporary' => array(
       'name' => t('Temporary files'),
       'class' => 'DrupalTemporaryStreamWrapper',
       'description' => t('Temporary local files for upload and previews.'),
       'type' => STREAM_WRAPPERS_HIDDEN,
-    )
+    ),
   );
+
+  // Only register the private file stream wrapper if a file path has been set.
+  if (variable_get('file_private_path', FALSE)) {
+    $wrappers['private'] = array(
+      'name' => t('Private files'),
+      'class' => 'DrupalPrivateStreamWrapper',
+      'description' => t('Private local files served by Drupal.'),
+    );
+  }
+
+  return $wrappers;
 }
 
 /**
@@ -2046,6 +2052,9 @@ function system_admin_menu_block($item) {
  */
 function system_check_directory($form_element) {
   $directory = $form_element['#value'];
+  if (strlen($directory) == 0) {
+    return $form_element;
+  }
 
   if (!is_dir($directory) && !drupal_mkdir($directory, NULL, TRUE)) {
     // If the directory does not exists and cannot be created.
@@ -2058,7 +2067,7 @@ function system_check_directory($form_element) {
     form_set_error($form_element['#parents'][0], t('The directory %directory exists but is not writable and could not be made writable.', array('%directory' => $directory)));
     watchdog('file system', 'The directory %directory exists but is not writable and could not be made writable.', array('%directory' => $directory), WATCHDOG_ERROR);
   }
-  else {
+  elseif (is_dir($directory)) {
     if ($form_element['#name'] == 'file_public_path') {
       // Create public .htaccess file.
       file_create_htaccess($directory, FALSE);