Issue #2170235 by Berdir, penyaskito, larowlan, alexpott, sushyl:...

Issue #2170235 by Berdir, penyaskito, larowlan, alexpott, sushyl: file_private_path should be in $settings, like file_public_path

core/core.services.yml

@@ -893,10 +893,6 @@ services:
     class: Drupal\Core\StreamWrapper\PublicStream
     tags:
       - { name: stream_wrapper, scheme: public }
-  stream_wrapper.private:
-    class: Drupal\Core\StreamWrapper\PrivateStream
-    tags:
-      - { name: stream_wrapper, scheme: private }
   stream_wrapper.temporary:
     class: Drupal\Core\StreamWrapper\TemporaryStream
     tags:

core/includes/file.inc

@@ -13,6 +13,7 @@
 use Drupal\Core\Site\Settings;
 use Drupal\Core\StreamWrapper\PublicStream;
 use Drupal\Core\StreamWrapper\StreamWrapperInterface;
+use Drupal\Core\StreamWrapper\PrivateStream;
 
 /**
  * Default mode for new directories. See drupal_chmod().
@@ -436,7 +437,7 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS)
  */
 function file_ensure_htaccess() {
   file_save_htaccess('public://', FALSE);
-  $private_path = \Drupal::config('system.file')->get('path.private');
+  $private_path = PrivateStream::basePath();
   if (!empty($private_path)) {
     file_save_htaccess('private://', TRUE);
   }

core/lib/Drupal/Core/CoreServiceProvider.php

@@ -21,6 +21,7 @@
 use Drupal\Core\DependencyInjection\Compiler\RegisterServicesForDestructionPass;
 use Drupal\Core\Plugin\PluginManagerPass;
 use Drupal\Core\Render\MainContent\MainContentRenderersPass;
+use Drupal\Core\Site\Settings;
 use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 
 /**
@@ -44,6 +45,12 @@ public function register(ContainerBuilder $container) {
     $this->registerUuid($container);
     $this->registerTest($container);
 
+    // Only register the private file stream wrapper if a file path has been set.
+    if (Settings::get('file_private_path')) {
+      $container->register('stream_wrapper.private', 'Drupal\Core\StreamWrapper\PrivateStream')
+        ->addTag('stream_wrapper', ['scheme' => 'private']);
+    }
+
     // Add the compiler pass that lets service providers modify existing
     // service definitions. This pass must come first so that later
     // list-building passes are operating on the post-alter services list.

core/lib/Drupal/Core/StreamWrapper/PrivateStream.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\StreamWrapper;
 
 use Drupal\Core\Routing\UrlGeneratorTrait;
+use Drupal\Core\Site\Settings;
 
 /**
  * Drupal private (private://) stream wrapper class.
@@ -41,20 +42,28 @@ public function getDescription() {
   }
 
   /**
-   * Implements Drupal\Core\StreamWrapper\LocalStream::getDirectoryPath()
+   * {@inheritdoc}
    */
   public function getDirectoryPath() {
-    return \Drupal::config('system.file')->get('path.private');
+    return static::basePath();
   }
 
   /**
-   * Implements Drupal\Core\StreamWrapper\StreamWrapperInterface::getExternalUrl().
-   *
-   * @return string
-   *   Returns the HTML URI of a private file.
+   * {@inheritdoc}
    */
-  function getExternalUrl() {
+  public function getExternalUrl() {
     $path = str_replace('\\', '/', $this->getTarget());
     return $this->url('system.private_file_download', ['filepath' => $path], ['absolute' => TRUE]);
   }
+
+  /**
+   * Returns the base path for private://.
+   *
+   * @return string
+   *   The base path for private://.
+   */
+  public static function basePath() {
+    return Settings::get('file_private_path');
+  }
+
 }

core/modules/migrate_drupal/config/install/migrate.migration.d6_system_file.yml

@@ -5,11 +5,9 @@ migration_groups:
 source:
   plugin: variable
   variables:
-    - file_directory_path
     - file_directory_temp
     - allow_insecure_uploads
 process:
-  'path/private': file_directory_path
   'path/temporary': file_directory_temp
   allow_insecure_uploads:
     plugin: static_map

core/modules/migrate_drupal/src/Tests/d6/MigrateSystemFileTest.php

@@ -39,7 +39,6 @@ public function testSystemFile() {
     $old_state = \Drupal::configFactory()->getOverrideState();
     \Drupal::configFactory()->setOverrideState(FALSE);
     $config = \Drupal::config('system.file');
-    $this->assertIdentical($config->get('path.private'), 'core/modules/simpletest/files');
     $this->assertIdentical($config->get('path.temporary'), 'files/temp');
     $this->assertIdentical($config->get('allow_insecure_uploads'), TRUE);
     \Drupal::configFactory()->setOverrideState($old_state);

core/modules/simpletest/src/TestBase.php

@@ -160,6 +160,15 @@ abstract class TestBase {
    */
   protected $public_files_directory;
 
+  /**
+   * The private file directory for the test environment.
+   *
+   * This is set in TestBase::prepareEnvironment().
+   *
+   * @var string
+   */
+  protected $private_files_directory;
+
   /**
    * Whether to die in case any test assertion fails.
    *

core/modules/simpletest/src/WebTestBase.php

@@ -794,6 +794,10 @@ protected function setUp() {
       'value' => $this->public_files_directory,
       'required' => TRUE,
     );
+    $settings['settings']['file_private_path'] = (object) array(
+      'value' => $this->private_files_directory,
+      'required' => TRUE,
+    );
     // Save the original site directory path, so that extensions in the
     // site-specific directory can still be discovered in the test site
     // environment.
@@ -874,7 +878,6 @@ protected function setUp() {
     file_prepare_directory($this->private_files_directory, FILE_CREATE_DIRECTORY);
     file_prepare_directory($this->temp_files_directory, FILE_CREATE_DIRECTORY);
     $config->get('system.file')
-      ->set('path.private', $this->private_files_directory)
       ->set('path.temporary', $this->temp_files_directory)
       ->save();
 

core/modules/system/config/install/system.file.yml

 allow_insecure_uploads: false
 default_scheme: 'public'
 path:
-  private: ''
   temporary: ''
 temporary_maximum_age: 21600

core/modules/system/config/schema/system.schema.yml

@@ -302,9 +302,6 @@ system.file:
       type: mapping
       label: 'Path settings'
       mapping:
-        private:
-          type: string
-          label: 'Private file system path'
         temporary:
           type: string
           label: 'Temporary directory'

core/modules/system/src/Form/FileSystemForm.php

@@ -11,6 +11,7 @@
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Datetime\DateFormatter;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\StreamWrapper\PrivateStream;
 use Drupal\Core\StreamWrapper\PublicStream;
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\StreamWrapper\StreamWrapperInterface;
@@ -78,18 +79,15 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     $form['file_public_path'] = array(
       '#type' => 'item',
       '#title' => t('Public file system path'),
-      '#default_value' => PublicStream::basePath(),
       '#markup' => PublicStream::basePath(),
       '#description' => t('A local file system path where public files will be stored. This directory must exist and be writable by Drupal. This directory must be relative to the Drupal installation directory and be accessible over the web. This must be changed in settings.php'),
     );
 
     $form['file_private_path'] = array(
-      '#type' => 'textfield',
+      '#type' => 'item',
       '#title' => t('Private file system path'),
-      '#default_value' => $config->get('path.private'),
-      '#maxlength' => 255,
-      '#description' => t('An existing local file system path for storing private files. It should be writable by Drupal and not accessible over the web. See the online handbook for <a href="@handbook">more information about securing private files</a>.', array('@handbook' => 'http://drupal.org/documentation/modules/file')),
-      '#after_build' => array('system_check_directory'),
+      '#markup' => (PrivateStream::basePath() ? PrivateStream::basePath() : t('Not set')),
+      '#description' => t('An existing local file system path for storing private files. It should be writable by Drupal and not accessible over the web. This must be changed in settings.php. See the online handbook for <a href="@handbook">more information about securing private files</a>.', array('@handbook' => 'http://drupal.org/documentation/modules/file')),
     );
 
     $form['file_temporary_path'] = array(
@@ -133,7 +131,6 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    */
   public function submitForm(array &$form, FormStateInterface $form_state) {
     $config = $this->config('system.file')
-      ->set('path.private', $form_state->getValue('file_private_path'))
       ->set('path.temporary', $form_state->getValue('file_temporary_path'))
       ->set('temporary_maximum_age', $form_state->getValue('temporary_maximum_age'));
 

core/modules/system/src/Tests/File/ConfigTest.php

@@ -33,7 +33,6 @@ function testFileConfigurationPage() {
     // upon form submission.
     $file_path = $this->public_files_directory;
     $fields = array(
-      'file_private_path' => $file_path . '/file_config_page_test/private',
       'file_temporary_path' => $file_path . '/file_config_page_test/temporary',
       'file_default_scheme' => 'private',
     );

core/modules/system/system.install

@@ -12,6 +12,7 @@
 use Drupal\Core\Database\Database;
 use Drupal\Core\Language\Language;
 use Drupal\Core\Site\Settings;
+use Drupal\Core\StreamWrapper\PrivateStream;
 use Drupal\Core\StreamWrapper\PublicStream;
 
 /**
@@ -284,7 +285,7 @@ function system_requirements($phase) {
       'title' => t('Public files directory'),
       'directory' => drupal_realpath('public://'),
     );
-    if (\Drupal::config('system.file')->get('path.private')) {
+    if (PrivateStream::basePath()) {
       $htaccess_files['private://.htaccess'] = array(
         'title' => t('Private files directory'),
         'directory' => drupal_realpath('private://'),
@@ -360,7 +361,7 @@ function system_requirements($phase) {
       PublicStream::basePath(),
       // By default no private files directory is configured. For private files
       // to be secure the admin needs to provide a path outside the webroot.
-      $filesystem_config->get('path.private'),
+      PrivateStream::basePath(),
       file_directory_temp(),
     );
   }
@@ -377,8 +378,8 @@ function system_requirements($phase) {
       // in the intended site directory, so don't require it.
       $directories[] = conf_path(FALSE) . '/files';
     }
-    if (!empty($GLOBALS['config']['system.file']['path']['private'])) {
-      $directories[] = $GLOBALS['config']['system.file']['path']['private'];
+    if ($file_private_path = Settings::get('file_private_path')) {
+      $directories[] = $file_private_path;
     }
     if (!empty($GLOBALS['config']['system.file']['path']['temporary'])) {
       $directories[] = $GLOBALS['config']['system.file']['path']['temporary'];

core/modules/system/system.module

@@ -306,16 +306,6 @@ function system_theme_suggestions_field(array $variables) {
   return $suggestions;
 }
 
-/**
- * Implements hook_stream_wrappers_alter().
- */
-function system_stream_wrappers_alter(&$wrappers) {
-  // Only register the private file stream wrapper if a file path has been set.
-  if (!\Drupal::config('system.file')->get('path.private')) {
-    unset($wrappers['private']);
-  }
-}
-
 /**
  * Menu item access callback - only installed themes can be accessed.
  */

sites/default/default.settings.php

@@ -430,6 +430,18 @@
  */
 # $settings['file_public_path'] = 'sites/default/files';
 
+/**
+ * Private file path:
+ *
+ * A local file system path where private files will be stored. This directory
+ * must be absolute, outside of the the Drupal installation directory and not
+ * accessible over the web.
+ *
+ * See http://drupal.org/documentation/modules/file for more information about
+ * securing private files.
+ */
+# $settings['file_private_path'] = '';
+
 /**
  * Session write interval:
  *