From f6f9ca744910e88c947df39d850637f84d9f5eda Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 22 Jul 2025 13:53:48 +0800 Subject: [PATCH 1/5] uucore: Move strip_minus_from_mode to mkdir It's only used there anyway. --- src/uu/mkdir/src/mkdir.rs | 18 +++++++++++++++++- src/uucore/src/lib/features/mode.rs | 20 -------------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 4e46460f21d..4c96f42b51b 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -85,9 +85,25 @@ fn strip_minus_from_mode(_args: &mut [String]) -> bool { 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) -> bool { - mode::strip_minus_from_mode(args) + 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 } #[uucore::main] diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 5a0a517276e..7befc1a6785 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/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) -> 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 { From 98f3aacb16012b679c028155a1c33d26a6b339ee Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 22 Jul 2025 13:49:52 +0800 Subject: [PATCH 2/5] mkdir: Collect OsString, instead of String. collect_lossy incorrectly converts invalid UTF-8 strings, let's keep the OsString everywhere. This also means we need to modify strip_minus_from_mode to make it work with OsString. --- src/uu/mkdir/src/mkdir.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 4c96f42b51b..3e816789eb0 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -10,6 +10,7 @@ use clap::parser::ValuesRef; use clap::{Arg, ArgAction, ArgMatches, Command}; use std::collections::HashMap; use std::ffi::OsString; +use std::os::unix::ffi::OsStrExt; use std::path::{Path, PathBuf}; #[cfg(not(windows))] use uucore::error::FromIo; @@ -81,7 +82,7 @@ fn get_mode(matches: &ArgMatches, mode_had_minus_prefix: bool) -> Result bool { +fn strip_minus_from_mode(_args: &mut [OsString]) -> bool { false } @@ -89,16 +90,18 @@ fn strip_minus_from_mode(_args: &mut [String]) -> bool { // 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) -> bool { +fn strip_minus_from_mode(args: &mut Vec) -> 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) + let bytes = arg.as_bytes(); + 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 = arg_stripped.to_string(); + *arg = std::ffi::OsStr::from_bytes(&bytes[1..]).to_owned(); return true; } } @@ -108,7 +111,7 @@ fn strip_minus_from_mode(args: &mut Vec) -> bool { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let mut args = args.collect_lossy(); + let mut args: Vec = 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"). From 7a9b88011ca9510e3e335e0b63e5719c2bc2e8f2 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 22 Jul 2025 17:05:45 +0800 Subject: [PATCH 3/5] mkdir: strip_minus_from_mode: Use uucore functions Let's not reinvent stuff. --- src/uu/mkdir/src/mkdir.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 3e816789eb0..89fc634949e 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -10,7 +10,6 @@ use clap::parser::ValuesRef; use clap::{Arg, ArgAction, ArgMatches, Command}; use std::collections::HashMap; use std::ffi::OsString; -use std::os::unix::ffi::OsStrExt; use std::path::{Path, PathBuf}; #[cfg(not(windows))] use uucore::error::FromIo; @@ -82,31 +81,31 @@ fn get_mode(matches: &ArgMatches, mode_had_minus_prefix: bool) -> Result bool { - false +fn strip_minus_from_mode(_args: &mut [OsString]) -> UResult { + 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) -> bool { +fn strip_minus_from_mode(args: &mut Vec) -> UResult { for arg in args { if arg == "--" { break; } - let bytes = arg.as_bytes(); + 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 = std::ffi::OsStr::from_bytes(&bytes[1..]).to_owned(); - return true; + *arg = uucore::os_str_from_bytes(&bytes[1..])?.into_owned(); + return Ok(true); } } } - false + Ok(false) } #[uucore::main] @@ -115,7 +114,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // 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" + From 7e6e9c291e630f0fb4a08ba0d70048c158888a3d Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 22 Jul 2025 17:19:45 +0800 Subject: [PATCH 4/5] uutests: Change dir_exists to take a templated AsRef Similar to what file_exists does, allows us to pass an OsStr to the function. Also change symlink_exists, for consistency. --- tests/by-util/test_cp.rs | 2 +- tests/by-util/test_mv.rs | 12 ++++++------ tests/uutests/src/lib/util.rs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7a3b28d234b..8575d1959e1 100644 --- a/tests/by-util/test_cp.rs +++ b/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)); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 7b984cfdea5..62c12c1d29a 100644 --- a/tests/by-util/test_mv.rs +++ b/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] diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 3fa3ed47755..154a7aa18fa 100644 --- a/tests/uutests/src/lib/util.rs +++ b/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>(&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>(&self, path: P) -> bool { match fs::metadata(self.plus(path)) { Ok(m) => m.is_dir(), Err(_) => false, From 64fb7a746262e890076949cfdd394b47e9b9ac84 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 22 Jul 2025 17:24:13 +0800 Subject: [PATCH 5/5] test_mkdir: Add a test for non-Unicode directory name Similar to the first part of GNU test df/problematic-chars. --- tests/by-util/test_mkdir.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 56b4297caf5..d70d50ce09f 100644 --- a/tests/by-util/test_mkdir.rs +++ b/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";