Issue #1068266 by linclark, andyceo, scottrigby, chx, jbrown, ñull, weboide: Fixed drupal_mkdir() does not set permissions to directories it created recursively .

core/includes/file.inc

@@ -501,8 +501,8 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS)
   if (!is_dir($directory)) {
     // Let mkdir() recursively create directories and use the default directory
     // permissions.
-    if (($options & FILE_CREATE_DIRECTORY) && @drupal_mkdir($directory, NULL, TRUE)) {
-      return drupal_chmod($directory);
+    if ($options & FILE_CREATE_DIRECTORY) {
+      return @drupal_mkdir($directory, NULL, TRUE);
     }
     return FALSE;
   }
@@ -1702,19 +1702,21 @@ function drupal_basename($uri, $suffix = NULL) {
 }
 
 /**
- * Creates a directory using Drupal's default mode.
+ * Creates a directory, optionally creating missing components in the path to
+ * the directory.
  *
- * PHP's mkdir() does not respect Drupal's default permissions mode. If a mode
- * is not provided, this function will make sure that Drupal's is used.
- *
- * Compatibility: normal paths and stream wrappers.
+ * When PHP's mkdir() creates a directory, the requested mode is affected by the
+ * process's umask. This function overrides the umask and sets the mode
+ * explicitly for all directory components created.
  *
  * @param $uri
  *   A URI or pathname.
  * @param $mode
- *   By default the Drupal mode is used.
+ *   Mode given to created directories. Defaults to the directory mode
+ *   configured in the Drupal installation. It must have a leading zero.
  * @param $recursive
- *   Default to FALSE.
+ *   Create directories recursively, defaults to FALSE. Cannot work with a mode
+ *   which denies writing or execution to the owner of the process.
  * @param $context
  *   Refer to http://php.net/manual/ref.stream.php
  *
@@ -1734,7 +1736,52 @@ function drupal_mkdir($uri, $mode = NULL, $recursive = FALSE, $context = NULL) {
     }
   }
 
-  if (!isset($context)) {
+  // If the URI has a scheme, don't override the umask - schemes can handle this
+  // issue in their own implementation.
+  if (file_uri_scheme($uri)) {
+    return _drupal_mkdir_call($uri, $mode, $recursive, $context);
+  }
+
+  // If recursive, create each missing component of the parent directory
+  // individually and set the mode explicitly to override the umask.
+  if ($recursive) {
+    $components = explode(DIRECTORY_SEPARATOR, $uri);
+    array_pop($components);
+    $recursive_path = '';
+    foreach ($components as $component) {
+      $recursive_path .= $component;
+
+      if (!file_exists($recursive_path)) {
+        if (!_drupal_mkdir_call($recursive_path, $mode, FALSE, $context)) {
+          return FALSE;
+        }
+        // Not necessary to use drupal_chmod() as there is no scheme.
+        if (!chmod($recursive_path, $mode)) {
+          return FALSE;
+        }
+      }
+
+      $recursive_path .= DIRECTORY_SEPARATOR;
+    }
+  }
+
+  // Do not check if the top-level directory already exists, as this condition
+  // must cause this function to fail.
+  if (!_drupal_mkdir_call($uri, $mode, FALSE, $context)) {
+    return FALSE;
+  }
+  // Not necessary to use drupal_chmod() as there is no scheme.
+  return chmod($uri, $mode);
+}
+
+/**
+ * Helper function. Ensures we don't pass a NULL as a context resource to
+ * mkdir().
+ *
+ * @see drupal_mkdir()
+ */
+function _drupal_mkdir_call($uri, $mode, $recursive, $context) {
+  if (is_null($context)) {
     return mkdir($uri, $mode, $recursive);
   }
   else {

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

@@ -425,10 +425,10 @@ abstract class LocalStream implements StreamWrapperInterface {
       $localpath = $this->getLocalPath($uri);
     }
     if ($options & STREAM_REPORT_ERRORS) {
-      return mkdir($localpath, $mode, $recursive);
+      return drupal_mkdir($localpath, $mode, $recursive);
     }
     else {
-      return @mkdir($localpath, $mode, $recursive);
+      return @drupal_mkdir($localpath, $mode, $recursive);
     }
   }
 

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

@@ -19,6 +19,39 @@ class DirectoryTest extends FileTestBase {
     );
   }
 
+  /**
+   * Test local directory handling functions.
+   */
+  function testFileCheckLocalDirectoryHandling() {
+    $directory = conf_path() . '/files';
+
+    // Check a new recursively created local directory for correct file system
+    // permissions.
+    $parent = $this->randomName();
+    $child = $this->randomName();
+
+    // Files directory already exists.
+    $this->assertTrue(is_dir($directory), t('Files directory already exists.'), 'File');
+    // Make files directory writable only.
+    $old_mode = fileperms($directory);
+
+    // Create the directories.
+    $parent_path = $directory . DIRECTORY_SEPARATOR . $parent;
+    $child_path = $parent_path . DIRECTORY_SEPARATOR . $child;
+    $this->assertTrue(drupal_mkdir($child_path, 0775, TRUE), t('No error reported when creating new local directories.'), 'File');
+
+    // Ensure new directories also exist.
+    $this->assertTrue(is_dir($parent_path), t('New parent directory actually exists.'), 'File');
+    $this->assertTrue(is_dir($child_path), t('New child directory actually exists.'), 'File');
+
+    // Check that new directory permissions were set properly.
+    $this->assertDirectoryPermissions($parent_path, 0775);
+    $this->assertDirectoryPermissions($child_path, 0775);
+
+    // Check that existing directory permissions were not modified.
+    $this->assertDirectoryPermissions($directory, $old_mode);
+  }
+
   /**
    * Test directory handling functions.
    */

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

@@ -131,6 +131,7 @@ abstract class FileTestBase extends WebTestBase {
 
     // Mask out all but the last three octets.
     $actual_mode = fileperms($directory) & 0777;
+    $expected_mode = $expected_mode & 0777;
 
     // PHP on Windows has limited support for file permissions. Usually each of
     // "user", "group" and "other" use one octal digit (3 bits) to represent the