From 917a9dc30fffe06b5cf938038a340ffb4ee0b147 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Wed, 15 Jan 2014 11:20:41 -0500 Subject: [PATCH 1/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 545fca3..fbb64ba 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -518,15 +518,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 +534,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; @@ -1298,8 +1295,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 4fd03af..e0f9ddf 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 From 44d4e1e332a2011401b1923515f83d3f277d34c8 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Tue, 14 Jan 2014 15:53:08 -0500 Subject: [PATCH 2/2] Move chmod.directory and chmod.file out of config and into settings so that file writes are independent of the config system. --- core/includes/file.inc | 38 +++++++++------------- core/lib/Drupal/Core/Config/FileStorage.php | 6 +++- core/modules/system/config/system.file.yml | 4 --- .../lib/Drupal/system/Tests/File/DirectoryTest.php | 3 +- .../Drupal/system/Tests/File/UnmanagedCopyTest.php | 10 +++--- .../Drupal/system/Tests/File/UnmanagedMoveTest.php | 5 ++- .../system/Tests/File/UnmanagedSaveDataTest.php | 3 +- 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 fbb64ba..e3e9326 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 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. */ @@ -1281,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 @@ -1306,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); } } @@ -1490,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 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..8c5634e 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php +++ b/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'); 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..acae92b 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, 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)); } } 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..7fac3b3 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, 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) 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..65755af 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php +++ b/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.'); } } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index e0f9ddf..a956416 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1960,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', diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php index 5a46b12..616f260 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 in PHP Octal Notation, with leading zero. + */ +# $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