mkdir: do not change mode of directories that we have not created

samueltardieu · samueltardieu · commit 71d7559c2002 · 2024-09-07T20:31:44.000+02:00
diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs
@@ -165,8 +165,10 @@ pub fn mkdir(path: &Path, recursive: bool, mode: u32, verbose: bool) -> UResult<
     let path_buf = dir_strip_dot_for_creation(path);
     let path = path_buf.as_path();
 
-    create_dir(path, recursive, verbose, false)?;
-    chmod(path, mode)
+    if create_dir(path, recursive, verbose, false)? {
+        chmod(path, mode)?;
+    }
+    Ok(())
 }
 
 #[cfg(any(unix, target_os = "redox"))]
@@ -186,22 +188,25 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> {
     Ok(())
 }
 
+// Return true if the directory at `path` has been created by this call.
 // `is_parent` argument is not used on windows
 #[allow(unused_variables)]
-fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<()> {
+fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<bool> {
     if path.exists() && !recursive {
         return Err(USimpleError::new(
             1,
             format!("{}: File exists", path.display()),
         ));
     }
     if path == Path::new("") {
-        return Ok(());
+        return Ok(false);
     }
 
     if recursive {
         match path.parent() {
-            Some(p) => create_dir(p, recursive, verbose, true)?,
+            Some(p) => {
+                create_dir(p, recursive, verbose, true)?;
+            }
             None => {
                 USimpleError::new(1, "failed to create whole tree");
             }
@@ -222,9 +227,9 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> U
                 // which is umask modified by 'u+wx'
                 chmod(path, (!mode::get_umask() & 0o0777) | 0o0300)?;
             }
-            Ok(())
+            Ok(true)
         }
-        Err(_) if path.is_dir() => Ok(()),
+        Err(_) if path.is_dir() => Ok(false),
         Err(e) => Err(e.into()),
     }
 }
diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs
@@ -129,6 +129,27 @@ fn test_mkdir_parent_mode_check_existing_parent() {
     );
 }
 
+#[cfg(not(windows))]
+#[test]
+fn test_mkdir_parent_mode_skip_existing_last_component_chmod() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("a");
+    at.mkdir("a/b");
+    at.set_mode("a/b", 0);
+
+    let default_umask: mode_t = 0o160;
+
+    ucmd.arg("-p")
+        .arg("a/b")
+        .umask(default_umask)
+        .succeeds()
+        .no_stderr()
+        .no_stdout();
+
+    assert_eq!(at.metadata("a/b").permissions().mode() as mode_t, 0o40000);
+}
+
 #[test]
 fn test_mkdir_dup_file() {
     let scene = TestScenario::new(util_name!());

Original file line number	Diff line number	Diff line change
`@@ -165,8 +165,10 @@ pub fn mkdir(path: &Path, recursive: bool, mode: u32, verbose: bool) -> UResult<`
`165`	`165`	`let path_buf = dir_strip_dot_for_creation(path);`
`166`	`166`	`let path = path_buf.as_path();`
`167`	`167`
`168`		`- create_dir(path, recursive, verbose, false)?;`
`169`		`- chmod(path, mode)`
	`168`	`+ if create_dir(path, recursive, verbose, false)? {`
	`169`	`+ chmod(path, mode)?;`
	`170`	`+ }`
	`171`	`+ Ok(())`
`170`	`172`	`}`
`171`	`173`
`172`	`174`	`#[cfg(any(unix, target_os = "redox"))]`
`@@ -186,22 +188,25 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> {`
`186`	`188`	`Ok(())`
`187`	`189`	`}`
`188`	`190`
	`191`	+// Return true if the directory at `path` has been created by this call.
`189`	`192`	// `is_parent` argument is not used on windows
`190`	`193`	`#[allow(unused_variables)]`
`191`		`-fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<()> {`
	`194`	`+fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<bool> {`
`192`	`195`	`if path.exists() && !recursive {`
`193`	`196`	`return Err(USimpleError::new(`
`194`	`197`	`1,`
`195`	`198`	`format!("{}: File exists", path.display()),`
`196`	`199`	`));`
`197`	`200`	`}`
`198`	`201`	`if path == Path::new("") {`
`199`		`- return Ok(());`
	`202`	`+ return Ok(false);`
`200`	`203`	`}`
`201`	`204`
`202`	`205`	`if recursive {`
`203`	`206`	`match path.parent() {`
`204`		`- Some(p) => create_dir(p, recursive, verbose, true)?,`
	`207`	`+ Some(p) => {`
	`208`	`+ create_dir(p, recursive, verbose, true)?;`
	`209`	`+ }`
`205`	`210`	`None => {`
`206`	`211`	`USimpleError::new(1, "failed to create whole tree");`
`207`	`212`	`}`
`@@ -222,9 +227,9 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> U`
`222`	`227`	`// which is umask modified by 'u+wx'`
`223`	`228`	`chmod(path, (!mode::get_umask() & 0o0777) \| 0o0300)?;`
`224`	`229`	`}`
`225`		`- Ok(())`
	`230`	`+ Ok(true)`
`226`	`231`	`}`
`227`		`- Err(_) if path.is_dir() => Ok(()),`
	`232`	`+ Err(_) if path.is_dir() => Ok(false),`
`228`	`233`	`Err(e) => Err(e.into()),`
`229`	`234`	`}`
`230`	`235`	`}`