Merge pull request #8367 from drinkcat/mkdir-osstr

Fix mkdir with non-unicode parameters

Author: cakebaker

src/uu/mkdir/src/mkdir.rs

@@ -81,22 +81,40 @@ fn get_mode(matches: &ArgMatches, mode_had_minus_prefix: bool) -> Result<u32, St
 }
 
 #[cfg(windows)]
-fn strip_minus_from_mode(_args: &mut [String]) -> bool {
-    false
+fn strip_minus_from_mode(_args: &mut [OsString]) -> UResult<bool> {
+    Ok(false)
 }
 
+// Iterate 'args' and delete the first occurrence
+// of a prefix '-' if it's associated with MODE
+// e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE"
 #[cfg(not(windows))]
-fn strip_minus_from_mode(args: &mut Vec<String>) -> bool {
-    mode::strip_minus_from_mode(args)
+fn strip_minus_from_mode(args: &mut Vec<OsString>) -> UResult<bool> {
+    for arg in args {
+        if arg == "--" {
+            break;
+        }
+        let bytes = uucore::os_str_as_bytes(arg)?;
+        if let Some(b'-') = bytes.first() {
+            if let Some(
+                b'r' | b'w' | b'x' | b'X' | b's' | b't' | b'u' | b'g' | b'o' | b'0'..=b'7',
+            ) = bytes.get(1)
+            {
+                *arg = uucore::os_str_from_bytes(&bytes[1..])?.into_owned();
+                return Ok(true);
+            }
+        }
+    }
+    Ok(false)
 }
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let mut args = args.collect_lossy();
+    let mut args: Vec<OsString> = args.collect();
 
     // Before we can parse 'args' with clap (and previously getopts),
     // a possible MODE prefix '-' needs to be removed (e.g. "chmod -x FILE").
-    let mode_had_minus_prefix = strip_minus_from_mode(&mut args);
+    let mode_had_minus_prefix = strip_minus_from_mode(&mut args)?;
 
     // Linux-specific options, not implemented
     // opts.optflag("Z", "context", "set SELinux security context" +

src/uucore/src/lib/features/mode.rs

@@ -182,26 +182,6 @@ pub fn get_umask() -> u32 {
     return mask as u32;
 }
 
-// Iterate 'args' and delete the first occurrence
-// of a prefix '-' if it's associated with MODE
-// e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE"
-pub fn strip_minus_from_mode(args: &mut Vec<String>) -> bool {
-    for arg in args {
-        if arg == "--" {
-            break;
-        }
-        if let Some(arg_stripped) = arg.strip_prefix('-') {
-            if let Some('r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7') =
-                arg.chars().nth(1)
-            {
-                *arg = arg_stripped.to_string();
-                return true;
-            }
-        }
-    }
-    false
-}
-
 #[cfg(test)]
 mod test {
 

tests/by-util/test_cp.rs

@@ -5837,7 +5837,7 @@ fn test_dir_perm_race_with_preserve_mode_and_ownership() {
                 start_time.elapsed() < timeout,
                 "timed out: cp took too long to create destination directory"
             );
-            if at.dir_exists(&format!("{DEST_DIR}/{SRC_DIR}")) {
+            if at.dir_exists(format!("{DEST_DIR}/{SRC_DIR}")) {
                 break;
             }
             sleep(Duration::from_millis(100));

tests/by-util/test_mkdir.rs

@@ -34,6 +34,18 @@ fn test_mkdir_mkdir() {
     new_ucmd!().arg("test_dir").succeeds();
 }
 
+#[cfg(feature = "test_risky_names")]
+#[test]
+fn test_mkdir_non_unicode() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    let target = uucore::os_str_from_bytes(b"some-\xc0-dir-\xf3")
+        .expect("Only unix platforms can test non-unicode names");
+    ucmd.arg(&target).succeeds();
+
+    assert!(at.dir_exists(target));
+}
+
 #[test]
 fn test_mkdir_verbose() {
     let expected = "mkdir: created directory 'test_dir'\n";

tests/by-util/test_mv.rs

@@ -228,8 +228,8 @@ fn test_mv_multiple_folders() {
         .succeeds()
         .no_stderr();
 
-    assert!(at.dir_exists(&format!("{target_dir}/{dir_a}")));
-    assert!(at.dir_exists(&format!("{target_dir}/{dir_b}")));
+    assert!(at.dir_exists(format!("{target_dir}/{dir_a}")));
+    assert!(at.dir_exists(format!("{target_dir}/{dir_b}")));
 }
 
 #[test]
@@ -555,7 +555,7 @@ fn test_mv_hardlink_to_symlink() {
         .arg(hardlink_to_symlink_file)
         .succeeds();
     assert!(!at2.symlink_exists(symlink_file));
-    assert!(at2.symlink_exists(&format!("{hardlink_to_symlink_file}~")));
+    assert!(at2.symlink_exists(format!("{hardlink_to_symlink_file}~")));
 }
 
 #[test]
@@ -649,7 +649,7 @@ fn test_mv_simple_backup_for_directory() {
 
     assert!(!at.dir_exists(dir_a));
     assert!(at.dir_exists(dir_b));
-    assert!(at.dir_exists(&format!("{dir_b}~")));
+    assert!(at.dir_exists(format!("{dir_b}~")));
     assert!(at.file_exists(format!("{dir_b}/file_a")));
     assert!(at.file_exists(format!("{dir_b}~/file_b")));
 }
@@ -1353,7 +1353,7 @@ fn test_mv_backup_dir() {
 
     assert!(!at.dir_exists(dir_a));
     assert!(at.dir_exists(dir_b));
-    assert!(at.dir_exists(&format!("{dir_b}~")));
+    assert!(at.dir_exists(format!("{dir_b}~")));
 }
 
 #[test]
@@ -1572,7 +1572,7 @@ fn test_mv_dir_into_dir_with_source_name_a_prefix_of_target_name() {
 
     ucmd.arg(source).arg(target).succeeds().no_output();
 
-    assert!(at.dir_exists(&format!("{target}/{source}")));
+    assert!(at.dir_exists(format!("{target}/{source}")));
 }
 
 #[test]

tests/uutests/src/lib/util.rs

@@ -1243,14 +1243,14 @@ impl AtPath {
     }
 
     /// Decide whether the named symbolic link exists in the test directory.
-    pub fn symlink_exists(&self, path: &str) -> bool {
+    pub fn symlink_exists<P: AsRef<Path>>(&self, path: P) -> bool {
         match fs::symlink_metadata(self.plus(path)) {
             Ok(m) => m.file_type().is_symlink(),
             Err(_) => false,
         }
     }
 
-    pub fn dir_exists(&self, path: &str) -> bool {
+    pub fn dir_exists<P: AsRef<Path>>(&self, path: P) -> bool {
         match fs::metadata(self.plus(path)) {
             Ok(m) => m.is_dir(),
             Err(_) => false,