Issue #2121849 by moshe weitzman: Move chmod.directory and chmod.file out of...

Issue #2121849 by moshe weitzman: Move chmod.directory and chmod.file out of config and into settings so that file writes are independant of the config system.

core/includes/file.inc

@@ -42,6 +42,17 @@
  */
 const STREAM_WRAPPERS_VISIBLE = 0x0010;
 
+
+/**
+ * Default mode for new directories. See drupal_chmod().
+ */
+const FILE_CHMOD_DIRECTORY = 0775;
+
+/**
+ * Default mode for new files. See drupal_chmod().
+ */
+const FILE_CHMOD_FILE = 0664;
+
 /**
  * Composite stream wrapper bit flags that are usually used as the types.
  */
@@ -518,15 +529,12 @@ function file_url_transform_relative($file_url) {
  *   A bitmask to indicate if the directory should be created if it does
  *   not exist (FILE_CREATE_DIRECTORY) or made writable if it is read-only
  *   (FILE_MODIFY_PERMISSIONS).
- * @param int $mode
- *   Octal value for the permissions. Consult PHP chmod() documentation for
- *   more information.
  *
  * @return
  *   TRUE if the directory exists (or was created) and is writable. FALSE
  *   otherwise.
  */
-function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS, $mode = NULL) {
+function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS) {
   if (!file_stream_wrapper_valid_scheme(file_uri_scheme($directory))) {
     // Only trim if we're not dealing with a stream.
     $directory = rtrim($directory, '/\\');
@@ -537,14 +545,14 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS,
     // Let mkdir() recursively create directories and use the default directory
     // permissions.
     if ($options & FILE_CREATE_DIRECTORY) {
-      return @drupal_mkdir($directory, $mode, TRUE);
+      return @drupal_mkdir($directory, NULL, TRUE);
     }
     return FALSE;
   }
   // The directory exists, so check to see if it is writable.
   $writable = is_writable($directory);
   if (!$writable && ($options & FILE_MODIFY_PERMISSIONS)) {
-    return drupal_chmod($directory, $mode);
+    return drupal_chmod($directory);
   }
 
   return $writable;
@@ -1284,8 +1292,8 @@ function file_get_mimetype($uri, $mapping = NULL) {
 /**
  * Sets the permissions on a file or directory.
  *
- * This function will use the system.file:chmod.directory and
- * system.file:chmod.file configuration for the default modes for directories
+ * This function will use the file_chmod_directory and
+ * file_chmod_file settings for the default modes for directories
  * and uploaded/generated files. By default these will give everyone read access
  * so that users accessing the files with a user account without the webserver
  * group (e.g. via FTP) can read these files, and give group write permissions
@@ -1298,8 +1306,8 @@ function file_get_mimetype($uri, $mapping = NULL) {
  *
  * @param $uri
  *   A string containing a URI file, or directory path.
- * @param int $mode
- *   Octal value for the permissions. Consult PHP chmod() documentation for
+ * @param $mode
+ *   Integer value for the permissions. Consult PHP chmod() documentation for
  *   more information.
  *
  * @return bool
@@ -1309,20 +1317,11 @@ function file_get_mimetype($uri, $mapping = NULL) {
  */
 function drupal_chmod($uri, $mode = NULL) {
   if (!isset($mode)) {
-    // Configuration system stores default modes as strings. We use octdec() so
-    // that the octal permission numbers can be expressed as integers or strings
-    // and will be converted correctly in both cases.
     if (is_dir($uri)) {
-      $mode = octdec(\Drupal::config('system.file')->get('chmod.directory'));
-      if (!$mode) {
-        $mode = 0775;
-      }
+      $mode = settings()->get('file_chmod_directory', FILE_CHMOD_DIRECTORY);
     }
     else {
-      $mode = octdec(\Drupal::config('system.file')->get('chmod.file'));
-      if (!$mode) {
-        $mode = 0664;
-      }
+      $mode = settings()->get('file_chmod_file', FILE_CHMOD_FILE);
     }
   }
 
@@ -1493,15 +1492,7 @@ function drupal_basename($uri, $suffix = NULL) {
  */
 function drupal_mkdir($uri, $mode = NULL, $recursive = FALSE, $context = NULL) {
   if (!isset($mode)) {
-    // Configuration system stores default mode as strings.
-    $mode = FALSE;
-    // During early update there's no container.
-    if (is_object(\Drupal::getContainer())) {
-      $mode = octdec(\Drupal::config('system.file')->get('chmod.directory'));
-    }
-    if (!$mode) {
-      $mode = 0775;
-    }
+    $mode = settings()->get('file_chmod_directory', FILE_CHMOD_DIRECTORY);
   }
 
   // If the URI has a scheme, don't override the umask - schemes can handle this

core/includes/install.core.inc

@@ -186,8 +186,6 @@ function install_state_defaults() {
     // Whether or not this installation is interactive. By default this will
     // be set to FALSE if settings are passed in to install_drupal().
     'interactive' => TRUE,
-    // The mode for directories that are created during install.
-    'mode' => NULL,
     // An array of parameters for the installation, pre-populated by the URL
     // or by the settings passed in to install_drupal(). This is primarily
     // used to store 'profile' (the name of the chosen installation profile)
@@ -1271,7 +1269,7 @@ function install_settings_form_submit($form, &$form_state) {
   drupal_rewrite_settings($settings);
 
   // Add the config directories to settings.php.
-  drupal_install_config_directories($install_state['mode']);
+  drupal_install_config_directories();
 
   // Indicate that the settings file has been verified, and check the database
   // for the last completed task, now that we have a valid connection. This
@@ -2229,9 +2227,9 @@ function install_check_translations($install_state) {
   $online = FALSE;
 
   // First attempt to create or make writable the files directory.
-  file_prepare_directory($files_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $install_state['mode']);
+  file_prepare_directory($files_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
   // Then, attempt to create or make writable the translations directory.
-  file_prepare_directory($translations_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $install_state['mode']);
+  file_prepare_directory($translations_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
 
   // Get values so the requirements errors can be specific.
   if (drupal_verify_install_file($translations_directory, FILE_EXIST|FILE_WRITABLE, 'dir')) {

core/includes/install.inc

@@ -433,14 +433,10 @@ function _drupal_rewrite_settings_dump_one(\stdClass $variable, $prefix = '', $s
 /**
  * Creates the config directory and ensures it is operational.
  *
- * @param int $mode
- *   Octal value for the permissions. Consult PHP chmod() documentation for
- *   more information.
- *
  * @see install_settings_form_submit()
  * @see update_prepare_d8_bootstrap()
  */
-function drupal_install_config_directories($mode = NULL) {
+function drupal_install_config_directories() {
   global $config_directories;
 
   // Add a randomized config directory name to settings.php, unless it was
@@ -469,7 +465,7 @@ function drupal_install_config_directories($mode = NULL) {
     // public files directory, which has already been verified to be writable
     // itself. But if it somehow fails anyway, the installation cannot proceed.
     // Bail out using a similar error message as in system_requirements().
-    if (!install_ensure_config_directory($config_type, $mode)) {
+    if (!install_ensure_config_directory($config_type)) {
       throw new Exception(t('The directory %directory could not be created or could not be made writable. To proceed with the installation, either create the directory and modify its permissions manually or ensure that the installer has the permissions to create it automatically. For more information, see the <a href="@handbook_url">online handbook</a>.', array(
         '%directory' => config_get_config_directory($config_type),
         '@handbook_url' => 'http://drupal.org/server-permissions',
@@ -535,14 +531,10 @@ function install_verify_config_directory($type) {
  *   Type of config directory to return. Drupal core provides 'active' and
  *   'staging'.
  *
- * @param int $mode
- *   Octal value for the permissions. Consult PHP chmod() documentation for
- *   more information.
- *
  * @return bool
  *   TRUE if the config directory exists and is writable.
  */
-function install_ensure_config_directory($type, $mode = NULL) {
+function install_ensure_config_directory($type) {
   // The config directory must be defined in settings.php.
   global $config_directories;
   if (!isset($config_directories[$type])) {
@@ -552,7 +544,7 @@ function install_ensure_config_directory($type, $mode = NULL) {
   // directories that the installer creates.
   else {
     $config_directory = config_get_config_directory($type);
-    return file_prepare_directory($config_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $mode);
+    return file_prepare_directory($config_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
   }
 }
 

core/lib/Drupal/Core/Config/FileStorage.php

@@ -110,10 +110,14 @@ public function readMultiple(array $names) {
    */
   public function write($name, array $data) {
     $data = $this->encode($data);
-    $status = @file_put_contents($this->getFilePath($name), $data);
+    $target = $this->getFilePath($name);
+    $status = @file_put_contents($target, $data);
     if ($status === FALSE) {
       throw new StorageException('Failed to write configuration file: ' . $this->getFilePath($name));
     }
+    else {
+      drupal_chmod($target);
+    }
     return TRUE;
   }
 

core/modules/image/image.install

@@ -11,8 +11,7 @@
 function image_install() {
   // Create the styles directory and ensure it's writable.
   $directory = file_default_scheme() . '://styles';
-  $mode = isset($GLOBALS['install_state']['mode']) ? $GLOBALS['install_state']['mode'] : NULL;
-  file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $mode);
+  file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
 }
 
 /**

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

 allow_insecure_uploads: '0'
-# chmod variables should be a string in PHP Octal Notation.
-chmod:
-  directory: '0775'
-  file: '0664'
 default_scheme: 'public'
 path:
   private: ''

core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php

@@ -85,11 +85,12 @@ function testFileCheckDirectoryHandling() {
       $this->assertFalse(file_prepare_directory($directory, 0), 'Error reported for a non-writeable directory.', 'File');
 
       // Test directory permission modification.
+      $this->settingsSet('file_chmod_directory', 0777);
       $this->assertTrue(file_prepare_directory($directory, FILE_MODIFY_PERMISSIONS), 'No error reported when making directory writeable.', 'File');
     }
 
     // Test that the directory has the correct permissions.
-    $this->assertDirectoryPermissions($directory, octdec(\Drupal::config('system.file')->get('chmod.directory')));
+    $this->assertDirectoryPermissions($directory, 0777, 'file_chmod_directory setting is respected.');
 
     // Remove .htaccess file to then test that it gets re-created.
     @drupal_unlink(file_default_scheme() . '://.htaccess');

core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php

@@ -23,7 +23,6 @@ public static function getInfo() {
    * Copy a normal file.
    */
   function testNormal() {
-    $config = \Drupal::config('system.file');
     // Create a file for testing
     $uri = $this->createUri();
 
@@ -34,7 +33,7 @@ function testNormal() {
     $this->assertEqual($new_filepath, $desired_filepath, 'Returned expected filepath.');
     $this->assertTrue(file_exists($uri), 'Original file remains.');
     $this->assertTrue(file_exists($new_filepath), 'New file exists.');
-    $this->assertFilePermissions($new_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($new_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
 
     // Copying with rename.
     $desired_filepath = 'public://' . $this->randomName();
@@ -44,7 +43,7 @@ function testNormal() {
     $this->assertNotEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.');
     $this->assertTrue(file_exists($uri), 'Original file remains.');
     $this->assertTrue(file_exists($newer_filepath), 'New file exists.');
-    $this->assertFilePermissions($newer_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($newer_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
 
     // TODO: test copying to a directory (rather than full directory/file path)
     // TODO: test copying normal files using normal paths (rather than only streams)
@@ -65,7 +64,6 @@ function testNonExistent() {
    * Copy a file onto itself.
    */
   function testOverwriteSelf() {
-    $config = \Drupal::config('system.file');
     // Create a file for testing
     $uri = $this->createUri();
 
@@ -75,7 +73,7 @@ function testOverwriteSelf() {
     $this->assertNotEqual($new_filepath, $uri, 'Copied file has a new name.');
     $this->assertTrue(file_exists($uri), 'Original file exists after copying onto itself.');
     $this->assertTrue(file_exists($new_filepath), 'Copied file exists after copying onto itself.');
-    $this->assertFilePermissions($new_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($new_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
 
     // Copy the file onto itself without renaming fails.
     $new_filepath = file_unmanaged_copy($uri, $uri, FILE_EXISTS_ERROR);
@@ -93,6 +91,6 @@ function testOverwriteSelf() {
     $this->assertNotEqual($new_filepath, $uri, 'Copied file has a new name.');
     $this->assertTrue(file_exists($uri), 'Original file exists after copying onto itself.');
     $this->assertTrue(file_exists($new_filepath), 'Copied file exists after copying onto itself.');
-    $this->assertFilePermissions($new_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($new_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
   }
 }

core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php

@@ -23,7 +23,6 @@ public static function getInfo() {
    * Move a normal file.
    */
   function testNormal() {
-    $config = \Drupal::config('system.file');
     // Create a file for testing
     $uri = $this->createUri();
 
@@ -34,7 +33,7 @@ function testNormal() {
     $this->assertEqual($new_filepath, $desired_filepath, 'Returned expected filepath.');
     $this->assertTrue(file_exists($new_filepath), 'File exists at the new location.');
     $this->assertFalse(file_exists($uri), 'No file remains at the old location.');
-    $this->assertFilePermissions($new_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($new_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
 
     // Moving with rename.
     $desired_filepath = 'public://' . $this->randomName();
@@ -45,7 +44,7 @@ function testNormal() {
     $this->assertNotEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.');
     $this->assertTrue(file_exists($newer_filepath), 'File exists at the new location.');
     $this->assertFalse(file_exists($new_filepath), 'No file remains at the old location.');
-    $this->assertFilePermissions($newer_filepath, octdec($config->get('chmod.file')));
+    $this->assertFilePermissions($newer_filepath, settings()->get('file_chmod_file', FILE_CHMOD_FILE));
 
     // TODO: test moving to a directory (rather than full directory/file path)
     // TODO: test creating and moving normal files (rather than streams)

core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php

@@ -24,6 +24,7 @@ public static function getInfo() {
    */
   function testFileSaveData() {
     $contents = $this->randomName(8);
+    $this->settingsSet('file_chmod_file', 0777);
 
     // No filename.
     $filepath = file_unmanaged_save_data($contents);
@@ -36,6 +37,6 @@ function testFileSaveData() {
     $this->assertTrue($filepath, 'Unnamed file saved correctly.');
     $this->assertEqual('asdf.txt', drupal_basename($filepath), 'File was named correctly.');
     $this->assertEqual($contents, file_get_contents($filepath), 'Contents of the file are correct.');
-    $this->assertFilePermissions($filepath, octdec(\Drupal::config('system.file')->get('chmod.file')));
+    $this->assertFilePermissions($filepath, 0777, 'file_chmod_file setting is respected.');
   }
 }

core/modules/system/system.install

@@ -371,8 +371,7 @@ function system_requirements($phase) {
       continue;
     }
     if ($phase == 'install') {
-      $mode = isset($GLOBALS['install_state']['mode']) ? $GLOBALS['install_state']['mode'] : NULL;
-      file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $mode);
+      file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
     }
     $is_writable = is_writable($directory);
     $is_directory = is_dir($directory);
@@ -1961,6 +1960,7 @@ function system_update_8047() {
   update_variables_to_config('system.file', array(
     'allow_insecure_uploads' => 'allow_insecure_uploads',
     'file_default_scheme' => 'default_scheme',
+    // Note: the chmod variables were later moved to the settings system.
     'file_chmod_directory' => 'chmod.directory',
     'file_chmod_file' => 'chmod.file',
     'file_private_path' => 'path.private',

sites/default/default.settings.php

@@ -449,6 +449,14 @@
  */
 # $settings['mixed_mode_sessions'] = TRUE;
 
+/**
+ * Default mode for for directories and files written by Drupal.
+ *
+ * Value should be in PHP Octal Notation, with leading zero.
+ */
+# $settings['file_chmod_directory'] = 0775;
+# $settings['file_chmod_file'] = 0664;
+
 /**
  * Public file path:
  *