Merge pull request #4876 from shinhs0506/mkdir-p

mkdir: set correct permissions on dirs created by -p

src/uu/mkdir/src/mkdir.rs

@@ -20,7 +20,7 @@ use uucore::mode;
 use uucore::{display::Quotable, fs::dir_strip_dot_for_creation};
 use uucore::{format_usage, help_about, help_section, help_usage, show, show_if_err};
 
-static DEFAULT_PERM: u32 = 0o755;
+static DEFAULT_PERM: u32 = 0o777;
 
 const ABOUT: &str = help_about!("mkdir.md");
 const USAGE: &str = help_usage!("mkdir.md");
@@ -41,7 +41,7 @@ fn get_mode(_matches: &ArgMatches, _mode_had_minus_prefix: bool) -> Result<u32,
 #[cfg(not(windows))]
 fn get_mode(matches: &ArgMatches, mode_had_minus_prefix: bool) -> Result<u32, String> {
     let digits: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
-    // Translate a ~str in octal form to u16, default to 755
+    // Translate a ~str in octal form to u16, default to 777
     // Not tested on Windows
     let mut new_mode = DEFAULT_PERM;
     match matches.get_one::<String>(options::MODE) {
@@ -158,7 +158,7 @@ fn exec(dirs: ValuesRef<OsString>, recursive: bool, mode: u32, verbose: bool) ->
 }
 
 fn mkdir(path: &Path, recursive: bool, mode: u32, verbose: bool) -> UResult<()> {
-    create_dir(path, recursive, verbose)?;
+    create_dir(path, recursive, verbose, false)?;
     chmod(path, mode)
 }
 
@@ -179,7 +179,9 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> {
     Ok(())
 }
 
-fn create_dir(path: &Path, recursive: bool, verbose: bool) -> UResult<()> {
+// `is_parent` argument is not used on windows
+#[allow(unused_variables)]
+fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<()> {
     if path.exists() && !recursive {
         return Err(USimpleError::new(
             1,
@@ -192,7 +194,7 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool) -> UResult<()> {
 
     if recursive {
         match path.parent() {
-            Some(p) => create_dir(p, recursive, verbose)?,
+            Some(p) => create_dir(p, recursive, verbose, true)?,
             None => {
                 USimpleError::new(1, "failed to create whole tree");
             }
@@ -207,6 +209,12 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool) -> UResult<()> {
                     path.quote()
                 );
             }
+            #[cfg(not(windows))]
+            if is_parent {
+                // directories created by -p have permission bits set to '=rwx,u+wx',
+                // which is umask modified by 'u+wx'
+                chmod(path, (!mode::get_umask() & 0o0777) | 0o0300)?;
+            }
             Ok(())
         }
         Err(_) if path.is_dir() => Ok(()),

tests/by-util/test_mkdir.rs

@@ -71,6 +71,72 @@ fn test_mkdir_dup_dir_parent() {
     scene.ucmd().arg("-p").arg(TEST_DIR6).succeeds();
 }
 
+#[cfg(not(windows))]
+#[test]
+fn test_mkdir_parent_mode() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    let default_umask: mode_t = 0o160;
+    let original_umask = unsafe { umask(default_umask) };
+
+    ucmd.arg("-p").arg("a/b").succeeds().no_stderr().no_stdout();
+
+    assert!(at.dir_exists("a"));
+    // parents created by -p have permissions set to "=rwx,u+wx"
+    assert_eq!(
+        at.metadata("a").permissions().mode() as mode_t,
+        ((!default_umask & 0o777) | 0o300) + 0o40000
+    );
+    assert!(at.dir_exists("a/b"));
+    // sub directory's permission is determined only by the umask
+    assert_eq!(
+        at.metadata("a/b").permissions().mode() as mode_t,
+        (!default_umask & 0o777) + 0o40000
+    );
+
+    unsafe {
+        umask(original_umask);
+    }
+}
+
+#[cfg(not(windows))]
+#[test]
+fn test_mkdir_parent_mode_check_existing_parent() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("a");
+
+    let default_umask: mode_t = 0o160;
+    let original_umask = unsafe { umask(default_umask) };
+
+    ucmd.arg("-p")
+        .arg("a/b/c")
+        .succeeds()
+        .no_stderr()
+        .no_stdout();
+
+    assert!(at.dir_exists("a"));
+    // parent dirs that already exist do not get their permissions modified
+    assert_eq!(
+        at.metadata("a").permissions().mode() as mode_t,
+        (!original_umask & 0o777) + 0o40000
+    );
+    assert!(at.dir_exists("a/b"));
+    assert_eq!(
+        at.metadata("a/b").permissions().mode() as mode_t,
+        ((!default_umask & 0o777) | 0o300) + 0o40000
+    );
+    assert!(at.dir_exists("a/b/c"));
+    assert_eq!(
+        at.metadata("a/b/c").permissions().mode() as mode_t,
+        (!default_umask & 0o777) + 0o40000
+    );
+
+    unsafe {
+        umask(original_umask);
+    }
+}
+
 #[test]
 fn test_mkdir_dup_file() {
     let scene = TestScenario::new(util_name!());
@@ -98,7 +164,7 @@ fn test_symbolic_alteration() {
 
     ucmd.arg("-m").arg("-w").arg(TEST_DIR1).succeeds();
     let perms = at.metadata(TEST_DIR1).permissions().mode();
-    assert_eq!(perms, 0o40555);
+    assert_eq!(perms, 0o40577);
 }
 
 #[test]