From c5300ce07fd5a840a4d7d04ca169f7705ebab344 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Tue, 14 Jan 2014 15:53:08 -0500 Subject: [PATCH 1/2] 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 | 40 ++++++++++------------ core/lib/Drupal/Core/Config/FileStorage.php | 6 +++- core/modules/system/config/system.file.yml | 4 --- .../lib/Drupal/system/Tests/File/DirectoryTest.php | 2 +- .../Drupal/system/Tests/File/UnmanagedCopyTest.php | 10 +++--- .../Drupal/system/Tests/File/UnmanagedMoveTest.php | 5 ++- .../system/Tests/File/UnmanagedSaveDataTest.php | 2 +- core/modules/system/system.install | 1 + sites/default/default.settings.php | 8 +++++ 9 files changed, 40 insertions(+), 38 deletions(-) diff --git a/core/includes/file.inc b/core/includes/file.inc index 545fca3..171d4f0 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -42,6 +42,17 @@ */ const STREAM_WRAPPERS_VISIBLE = 0x0010; + +/** + * Default mode for new directories. See drupal_chmod(). + */ +const CHMOD_DEFAULT_DIRECTORY = '0775'; + +/** + * Default mode for new files. See drupal_chmod(). + */ +const CHMOD_DEFAULT_FILE = '0664'; + /** * Composite stream wrapper bit flags that are usually used as the types. */ @@ -1284,8 +1295,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 @@ -1309,20 +1320,13 @@ 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. + // 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 = octdec(settings()->get('file_chmod_directory', CHMOD_DEFAULT_DIRECTORY)); } else { - $mode = octdec(\Drupal::config('system.file')->get('chmod.file')); - if (!$mode) { - $mode = 0664; - } + $mode = octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_FILE)); } } @@ -1493,15 +1497,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 = octdec(settings()->get('file_chmod_directory', CHMOD_DEFAULT_DIRECTORY)); } // If the URI has a scheme, don't override the umask - schemes can handle this diff --git a/core/lib/Drupal/Core/Config/FileStorage.php b/core/lib/Drupal/Core/Config/FileStorage.php index 52c9aa5..e9d2185 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/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; } diff --git a/core/modules/system/config/system.file.yml b/core/modules/system/config/system.file.yml index b5be48c..eefee01 100644 --- a/core/modules/system/config/system.file.yml +++ b/core/modules/system/config/system.file.yml @@ -1,8 +1,4 @@ allow_insecure_uploads: '0' -# chmod variables should be a string in PHP Octal Notation. -chmod: - directory: '0775' - file: '0664' default_scheme: 'public' path: private: '' diff --git a/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php index 7fd19c5..b5c9cdc 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php @@ -89,7 +89,7 @@ function testFileCheckDirectoryHandling() { } // Test that the directory has the correct permissions. - $this->assertDirectoryPermissions($directory, octdec(\Drupal::config('system.file')->get('chmod.directory'))); + $this->assertDirectoryPermissions($directory, octdec(settings()->get('file_chmod_directory', CHMOD_DEFAULT_DIRECTORY))); // Remove .htaccess file to then test that it gets re-created. @drupal_unlink(file_default_scheme() . '://.htaccess'); diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php index 70f1903..298f1bd 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php +++ b/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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_FILE))); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php index 1503bd1..d85c32d 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php +++ b/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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_FILE))); // TODO: test moving to a directory (rather than full directory/file path) // TODO: test creating and moving normal files (rather than streams) diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php index 043ed6c..88ec592 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php @@ -36,6 +36,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, octdec(settings()->get('file_chmod_file', CHMOD_DEFAULT_FILE))); } } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 4fd03af..6dd3e6e 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1961,6 +1961,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', diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php index 5a46b12..9d6622f 100644 --- a/sites/default/default.settings.php +++ b/sites/default/default.settings.php @@ -450,6 +450,14 @@ # $settings['mixed_mode_sessions'] = TRUE; /** + * Default mode for for directories and files written by Drupal. + * + * Value should be a string in PHP Octal Notation. + */ +# $settings['file_chmod_directory'] = '0775'; +# $settings['file_chmod_file'] = '0664'; + +/** * Public file path: * * A local file system path where public files will be stored. This directory -- 1.8.1.1 From ea915360eafdb700d19fdb809b736d7c5077edaf Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Wed, 15 Jan 2014 11:20:41 -0500 Subject: [PATCH 2/2] Rollback of [#2099467]. This code has been superceded by [#2121849]. --- core/includes/file.inc | 13 +++++-------- core/includes/install.core.inc | 8 +++----- core/includes/install.inc | 16 ++++------------ core/modules/image/image.install | 3 +-- core/modules/system/system.install | 3 +-- 5 files changed, 14 insertions(+), 29 deletions(-) diff --git a/core/includes/file.inc b/core/includes/file.inc index 171d4f0..38e07a8 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -529,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, '/\\'); @@ -548,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; @@ -1309,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 diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 6dfc270..1794a10 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -185,8 +185,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) @@ -1270,7 +1268,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 @@ -2233,9 +2231,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')) { diff --git a/core/includes/install.inc b/core/includes/install.inc index 67c18b7..87783f6 100644 --- a/core/includes/install.inc +++ b/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 online handbook.', 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); } } diff --git a/core/modules/image/image.install b/core/modules/image/image.install index 6a59953..bdd6f36 100644 --- a/core/modules/image/image.install +++ b/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); } /** diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 6dd3e6e..a956416 100644 --- a/core/modules/system/system.install +++ b/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); -- 1.8.1.1