From 11e77c72d43f53e4d3f5e2c0da4b3317fd7552d7 Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sat, 6 Dec 2025 10:16:56 +0000 Subject: [PATCH 1/5] uucore: mode parsing: support comma-separated mode Parsing in uucore::mode did not support multiple mode chunks separated by commas, e.g. "ug+rw,o+r" --- src/uucore/src/lib/features/mode.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 50ed8c97c..323830d76 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -139,21 +139,16 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) { #[allow(clippy::unnecessary_cast)] pub fn parse_mode(mode: &str) -> Result { - #[cfg(all( - not(target_os = "freebsd"), - not(target_vendor = "apple"), - not(target_os = "android") - ))] - let fperm = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; - #[cfg(any(target_os = "freebsd", target_vendor = "apple", target_os = "android"))] - let fperm = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; + let mut new_mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; - let result = if mode.chars().any(|c| c.is_ascii_digit()) { - parse_numeric(fperm as u32, mode, true) - } else { - parse_symbolic(fperm as u32, mode, get_umask(), true) - }; - result.map(|mode| mode as mode_t) + for mode_chunk in mode.split(',') { + new_mode = if mode_chunk.chars().any(|c| c.is_ascii_digit()) { + parse_numeric(new_mode, mode_chunk, true)? + } else { + parse_symbolic(new_mode, mode_chunk, get_umask(), true)? + }; + } + Ok(new_mode as mode_t) } pub fn get_umask() -> u32 { @@ -202,4 +197,9 @@ mod test { assert_eq!(super::parse_mode("+100").unwrap(), 0o766); assert_eq!(super::parse_mode("-4").unwrap(), 0o662); } + + #[test] + fn multiple_modes() { + assert_eq!(super::parse_mode("+100,+010").unwrap(), 0o776); + } } From 4e653b5ec0d0e9f91a3a8f62c7e5050b94091edc Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sat, 6 Dec 2025 13:58:37 +0000 Subject: [PATCH 2/5] move mode parsing and tests from install to uucore --- src/uu/install/src/install.rs | 2 +- src/uu/install/src/mode.rs | 149 ---------------------------- src/uucore/src/lib/features/mode.rs | 144 ++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 153 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index ab05c7ca0..582eb91ac 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -338,7 +338,7 @@ fn behavior(matches: &ArgMatches) -> UResult { let specified_mode: Option = if matches.contains_id(OPT_MODE) { let x = matches.get_one::(OPT_MODE).ok_or(1)?; - Some(mode::parse(x, considering_dir, 0).map_err(|err| { + Some(uucore::mode::parse(x, considering_dir, 0).map_err(|err| { show_error!( "{}", translate!("install-error-invalid-mode", "error" => err) diff --git a/src/uu/install/src/mode.rs b/src/uu/install/src/mode.rs index 5c29aaf77..96aae38c4 100644 --- a/src/uu/install/src/mode.rs +++ b/src/uu/install/src/mode.rs @@ -4,32 +4,8 @@ // file that was distributed with this source code. use std::fs; use std::path::Path; -#[cfg(not(windows))] -use uucore::mode; use uucore::translate; -/// Takes a user-supplied string and tries to parse to u16 mode bitmask. -/// Supports comma-separated mode strings like "ug+rwX,o+rX" (same as chmod). -pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result { - // Split by commas and process each mode part sequentially - let mut current_mode: u32 = 0; - - for mode_part in mode_string.split(',') { - let mode_part = mode_part.trim(); - if mode_part.is_empty() { - continue; - } - - current_mode = if mode_part.chars().any(|c| c.is_ascii_digit()) { - mode::parse_numeric(current_mode, mode_part, considering_dir)? - } else { - mode::parse_symbolic(current_mode, mode_part, umask, considering_dir)? - }; - } - - Ok(current_mode) -} - /// chmod a file or directory on UNIX. /// /// Adapted from mkdir.rs. Handles own error printing. @@ -55,128 +31,3 @@ pub fn chmod(path: &Path, mode: u32) -> Result<(), ()> { // chmod on Windows only sets the readonly flag, which isn't even honored on directories Ok(()) } - -#[cfg(test)] -#[cfg(not(windows))] -mod tests { - use super::parse; - - #[test] - fn test_parse_numeric_mode() { - // Simple numeric mode - assert_eq!(parse("644", false, 0).unwrap(), 0o644); - assert_eq!(parse("755", false, 0).unwrap(), 0o755); - assert_eq!(parse("777", false, 0).unwrap(), 0o777); - assert_eq!(parse("600", false, 0).unwrap(), 0o600); - } - - #[test] - fn test_parse_numeric_mode_with_operator() { - // Numeric mode with + operator - assert_eq!(parse("+100", false, 0).unwrap(), 0o100); - assert_eq!(parse("+644", false, 0).unwrap(), 0o644); - - // Numeric mode with - operator (starting from 0, so nothing to remove) - assert_eq!(parse("-4", false, 0).unwrap(), 0); - // But if we first set a mode, then remove bits - assert_eq!(parse("644,-4", false, 0).unwrap(), 0o640); - } - - #[test] - fn test_parse_symbolic_mode() { - // Simple symbolic modes - assert_eq!(parse("u+x", false, 0).unwrap(), 0o100); - assert_eq!(parse("g+w", false, 0).unwrap(), 0o020); - assert_eq!(parse("o+r", false, 0).unwrap(), 0o004); - assert_eq!(parse("a+x", false, 0).unwrap(), 0o111); - } - - #[test] - fn test_parse_symbolic_mode_multiple_permissions() { - // Multiple permissions in one mode - assert_eq!(parse("u+rw", false, 0).unwrap(), 0o600); - assert_eq!(parse("ug+rwx", false, 0).unwrap(), 0o770); - assert_eq!(parse("a+rwx", false, 0).unwrap(), 0o777); - } - - #[test] - fn test_parse_comma_separated_modes() { - // Comma-separated mode strings (as mentioned in the doc comment) - assert_eq!(parse("ug+rwX,o+rX", false, 0).unwrap(), 0o664); - assert_eq!(parse("u+rwx,g+rx,o+r", false, 0).unwrap(), 0o754); - assert_eq!(parse("u+w,g+w,o+w", false, 0).unwrap(), 0o222); - } - - #[test] - fn test_parse_comma_separated_with_spaces() { - // Comma-separated with spaces (should be trimmed) - assert_eq!(parse("u+rw, g+rw, o+r", false, 0).unwrap(), 0o664); - assert_eq!(parse(" u+x , g+x ", false, 0).unwrap(), 0o110); - } - - #[test] - fn test_parse_mixed_numeric_and_symbolic() { - // Mix of numeric and symbolic modes - assert_eq!(parse("644,u+x", false, 0).unwrap(), 0o744); - assert_eq!(parse("u+rw,755", false, 0).unwrap(), 0o755); - } - - #[test] - fn test_parse_empty_string() { - // Empty string should return 0 - assert_eq!(parse("", false, 0).unwrap(), 0); - assert_eq!(parse(" ", false, 0).unwrap(), 0); - assert_eq!(parse(",,", false, 0).unwrap(), 0); - } - - #[test] - fn test_parse_with_umask() { - // Test with umask (affects symbolic modes when no level is specified) - let umask = 0o022; - assert_eq!(parse("+w", false, umask).unwrap(), 0o200); - // The umask should be respected for symbolic modes without explicit level - } - - #[test] - fn test_parse_considering_dir() { - // Test directory vs file mode differences - // For directories, X (capital X) should add execute permission - assert_eq!(parse("a+X", true, 0).unwrap(), 0o111); - // For files without execute, X should not add execute - assert_eq!(parse("a+X", false, 0).unwrap(), 0o000); - - // Numeric modes for directories preserve setuid/setgid bits - assert_eq!(parse("755", true, 0).unwrap(), 0o755); - } - - #[test] - fn test_parse_invalid_modes() { - // Invalid numeric mode (too large) - assert!(parse("10000", false, 0).is_err()); - - // Invalid operator - assert!(parse("u*rw", false, 0).is_err()); - - // Invalid symbolic mode - assert!(parse("invalid", false, 0).is_err()); - } - - #[test] - fn test_parse_complex_combinations() { - // Complex real-world examples - assert_eq!(parse("u=rwx,g=rx,o=r", false, 0).unwrap(), 0o754); - // To test removal, we need to first set permissions, then remove them - assert_eq!(parse("644,a-w", false, 0).unwrap(), 0o444); - assert_eq!(parse("644,g-r", false, 0).unwrap(), 0o604); - } - - #[test] - fn test_parse_sequential_application() { - // Test that comma-separated modes are applied sequentially - // First set to 644, then add execute for user - assert_eq!(parse("644,u+x", false, 0).unwrap(), 0o744); - - // First add user write, then set to 755 (should override) - assert_eq!(parse("u+w,755", false, 0).unwrap(), 0o755); - } -} diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 323830d76..af2494737 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -137,6 +137,28 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) { (srwx, pos) } +/// Takes a user-supplied string and tries to parse to u16 mode bitmask. +/// Supports comma-separated mode strings like "ug+rwX,o+rX" (same as chmod). +pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result { + // Split by commas and process each mode part sequentially + let mut current_mode: u32 = 0; + + for mode_part in mode_string.split(',') { + let mode_part = mode_part.trim(); + if mode_part.is_empty() { + continue; + } + + current_mode = if mode_part.chars().any(|c| c.is_ascii_digit()) { + parse_numeric(current_mode, mode_part, considering_dir)? + } else { + parse_symbolic(current_mode, mode_part, umask, considering_dir)? + }; + } + + Ok(current_mode) +} + #[allow(clippy::unnecessary_cast)] pub fn parse_mode(mode: &str) -> Result { let mut new_mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; @@ -178,7 +200,9 @@ pub fn get_umask() -> u32 { } #[cfg(test)] -mod test { +mod tests { + + use super::parse; #[test] fn symbolic_modes() { @@ -199,7 +223,121 @@ mod test { } #[test] - fn multiple_modes() { - assert_eq!(super::parse_mode("+100,+010").unwrap(), 0o776); + fn test_parse_numeric_mode() { + // Simple numeric mode + assert_eq!(parse("644", false, 0).unwrap(), 0o644); + assert_eq!(parse("755", false, 0).unwrap(), 0o755); + assert_eq!(parse("777", false, 0).unwrap(), 0o777); + assert_eq!(parse("600", false, 0).unwrap(), 0o600); + } + + #[test] + fn test_parse_numeric_mode_with_operator() { + // Numeric mode with + operator + assert_eq!(parse("+100", false, 0).unwrap(), 0o100); + assert_eq!(parse("+644", false, 0).unwrap(), 0o644); + + // Numeric mode with - operator (starting from 0, so nothing to remove) + assert_eq!(parse("-4", false, 0).unwrap(), 0); + // But if we first set a mode, then remove bits + assert_eq!(parse("644,-4", false, 0).unwrap(), 0o640); + } + + #[test] + fn test_parse_symbolic_mode() { + // Simple symbolic modes + assert_eq!(parse("u+x", false, 0).unwrap(), 0o100); + assert_eq!(parse("g+w", false, 0).unwrap(), 0o020); + assert_eq!(parse("o+r", false, 0).unwrap(), 0o004); + assert_eq!(parse("a+x", false, 0).unwrap(), 0o111); + } + + #[test] + fn test_parse_symbolic_mode_multiple_permissions() { + // Multiple permissions in one mode + assert_eq!(parse("u+rw", false, 0).unwrap(), 0o600); + assert_eq!(parse("ug+rwx", false, 0).unwrap(), 0o770); + assert_eq!(parse("a+rwx", false, 0).unwrap(), 0o777); + } + + #[test] + fn test_parse_comma_separated_modes() { + // Comma-separated mode strings (as mentioned in the doc comment) + assert_eq!(parse("ug+rwX,o+rX", false, 0).unwrap(), 0o664); + assert_eq!(parse("u+rwx,g+rx,o+r", false, 0).unwrap(), 0o754); + assert_eq!(parse("u+w,g+w,o+w", false, 0).unwrap(), 0o222); + } + + #[test] + fn test_parse_comma_separated_with_spaces() { + // Comma-separated with spaces (should be trimmed) + assert_eq!(parse("u+rw, g+rw, o+r", false, 0).unwrap(), 0o664); + assert_eq!(parse(" u+x , g+x ", false, 0).unwrap(), 0o110); + } + + #[test] + fn test_parse_mixed_numeric_and_symbolic() { + // Mix of numeric and symbolic modes + assert_eq!(parse("644,u+x", false, 0).unwrap(), 0o744); + assert_eq!(parse("u+rw,755", false, 0).unwrap(), 0o755); + } + + #[test] + fn test_parse_empty_string() { + // Empty string should return 0 + assert_eq!(parse("", false, 0).unwrap(), 0); + assert_eq!(parse(" ", false, 0).unwrap(), 0); + assert_eq!(parse(",,", false, 0).unwrap(), 0); + } + + #[test] + fn test_parse_with_umask() { + // Test with umask (affects symbolic modes when no level is specified) + let umask = 0o022; + assert_eq!(parse("+w", false, umask).unwrap(), 0o200); + // The umask should be respected for symbolic modes without explicit level + } + + #[test] + fn test_parse_considering_dir() { + // Test directory vs file mode differences + // For directories, X (capital X) should add execute permission + assert_eq!(parse("a+X", true, 0).unwrap(), 0o111); + // For files without execute, X should not add execute + assert_eq!(parse("a+X", false, 0).unwrap(), 0o000); + + // Numeric modes for directories preserve setuid/setgid bits + assert_eq!(parse("755", true, 0).unwrap(), 0o755); + } + + #[test] + fn test_parse_invalid_modes() { + // Invalid numeric mode (too large) + assert!(parse("10000", false, 0).is_err()); + + // Invalid operator + assert!(parse("u*rw", false, 0).is_err()); + + // Invalid symbolic mode + assert!(parse("invalid", false, 0).is_err()); + } + + #[test] + fn test_parse_complex_combinations() { + // Complex real-world examples + assert_eq!(parse("u=rwx,g=rx,o=r", false, 0).unwrap(), 0o754); + // To test removal, we need to first set permissions, then remove them + assert_eq!(parse("644,a-w", false, 0).unwrap(), 0o444); + assert_eq!(parse("644,g-r", false, 0).unwrap(), 0o604); + } + + #[test] + fn test_parse_sequential_application() { + // Test that comma-separated modes are applied sequentially + // First set to 644, then add execute for user + assert_eq!(parse("644,u+x", false, 0).unwrap(), 0o744); + + // First add user write, then set to 755 (should override) + assert_eq!(parse("u+w,755", false, 0).unwrap(), 0o755); } } From 22344cea9987949d32e59b8a3b19beb902de313d Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sat, 6 Dec 2025 14:16:25 +0000 Subject: [PATCH 3/5] Use new method from parse_mode as well --- src/uucore/src/lib/features/mode.rs | 70 ++++++++++++++++++----------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index af2494737..a9477fa5f 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -137,39 +137,42 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) { (srwx, pos) } -/// Takes a user-supplied string and tries to parse to u16 mode bitmask. +/// Modify a file mode based on a user-supplied string. /// Supports comma-separated mode strings like "ug+rwX,o+rX" (same as chmod). -pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result { - // Split by commas and process each mode part sequentially - let mut current_mode: u32 = 0; +pub fn parse_chmod( + current_mode: u32, + mode_string: &str, + considering_dir: bool, + umask: u32, +) -> Result { + let mut new_mode: u32 = current_mode; + // Split by commas and process each mode part sequentially for mode_part in mode_string.split(',') { let mode_part = mode_part.trim(); if mode_part.is_empty() { continue; } - current_mode = if mode_part.chars().any(|c| c.is_ascii_digit()) { - parse_numeric(current_mode, mode_part, considering_dir)? + new_mode = if mode_part.chars().any(|c| c.is_ascii_digit()) { + parse_numeric(new_mode, mode_part, considering_dir)? } else { - parse_symbolic(current_mode, mode_part, umask, considering_dir)? + parse_symbolic(new_mode, mode_part, umask, considering_dir)? }; } - Ok(current_mode) + Ok(new_mode) +} + +/// Takes a user-supplied string and tries to parse to u32 mode bitmask. +pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result { + parse_chmod(0, mode_string, considering_dir, umask) } #[allow(clippy::unnecessary_cast)] pub fn parse_mode(mode: &str) -> Result { let mut new_mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; - - for mode_chunk in mode.split(',') { - new_mode = if mode_chunk.chars().any(|c| c.is_ascii_digit()) { - parse_numeric(new_mode, mode_chunk, true)? - } else { - parse_symbolic(new_mode, mode_chunk, get_umask(), true)? - }; - } + new_mode = parse_chmod(new_mode, mode, true, get_umask())?; Ok(new_mode as mode_t) } @@ -203,23 +206,25 @@ pub fn get_umask() -> u32 { mod tests { use super::parse; + use super::parse_chmod; + use super::parse_mode; #[test] - fn symbolic_modes() { - assert_eq!(super::parse_mode("u+x").unwrap(), 0o766); + fn test_symbolic_modes() { + assert_eq!(parse_mode("u+x").unwrap(), 0o766); assert_eq!( - super::parse_mode("+x").unwrap(), + parse_mode("+x").unwrap(), if crate::os::is_wsl_1() { 0o776 } else { 0o777 } ); - assert_eq!(super::parse_mode("a-w").unwrap(), 0o444); - assert_eq!(super::parse_mode("g-r").unwrap(), 0o626); + assert_eq!(parse_mode("a-w").unwrap(), 0o444); + assert_eq!(parse_mode("g-r").unwrap(), 0o626); } #[test] - fn numeric_modes() { - assert_eq!(super::parse_mode("644").unwrap(), 0o644); - assert_eq!(super::parse_mode("+100").unwrap(), 0o766); - assert_eq!(super::parse_mode("-4").unwrap(), 0o662); + fn test_numeric_modes() { + assert_eq!(parse_mode("644").unwrap(), 0o644); + assert_eq!(parse_mode("+100").unwrap(), 0o766); + assert_eq!(parse_mode("-4").unwrap(), 0o662); } #[test] @@ -340,4 +345,19 @@ mod tests { // First add user write, then set to 755 (should override) assert_eq!(parse("u+w,755", false, 0).unwrap(), 0o755); } + + #[test] + fn test_chmod_symbolic_modes() { + assert_eq!(parse_chmod(0o666, "u+x", false, 0).unwrap(), 0o766); + assert_eq!(parse_chmod(0o666, "+x", false, 0).unwrap(), 0o777); + assert_eq!(parse_chmod(0o666, "a-w", false, 0).unwrap(), 0o444); + assert_eq!(parse_chmod(0o666, "g-r", false, 0).unwrap(), 0o626); + } + + #[test] + fn test_chmod_numeric_modes() { + assert_eq!(parse_chmod(0o666, "644", false, 0).unwrap(), 0o644); + assert_eq!(parse_chmod(0o666, "+100", false, 0).unwrap(), 0o766); + assert_eq!(parse_chmod(0o666, "-4", false, 0).unwrap(), 0o662); + } } From 114be93e01158252092275a429df2ce413d8283b Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sat, 6 Dec 2025 14:27:03 +0000 Subject: [PATCH 4/5] Use common mode parsing in mkdirm, mkfifo, mknod --- src/uu/mkdir/src/mkdir.rs | 13 ++------ src/uu/mkfifo/src/mkfifo.rs | 14 ++------- src/uu/mknod/src/mknod.rs | 6 ++-- src/uucore/src/lib/features/mode.rs | 46 +++++++---------------------- 4 files changed, 19 insertions(+), 60 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index a16be0c26..6ee610013 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -57,20 +57,11 @@ fn get_mode(_matches: &ArgMatches) -> Result { #[cfg(not(windows))] fn get_mode(matches: &ArgMatches) -> Result { // Not tested on Windows - let mut new_mode = DEFAULT_PERM; - if let Some(m) = matches.get_one::(options::MODE) { - for mode in m.split(',') { - if mode.chars().any(|c| c.is_ascii_digit()) { - new_mode = mode::parse_numeric(new_mode, m, true)?; - } else { - new_mode = mode::parse_symbolic(new_mode, mode, mode::get_umask(), true)?; - } - } - Ok(new_mode) + mode::parse_chmod(DEFAULT_PERM, m, true, mode::get_umask()) } else { // If no mode argument is specified return the mode derived from umask - Ok(!mode::get_umask() & 0o0777) + Ok(!mode::get_umask() & DEFAULT_PERM) } } diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs index 572ea00b8..c55593dcb 100644 --- a/src/uu/mkfifo/src/mkfifo.rs +++ b/src/uu/mkfifo/src/mkfifo.rs @@ -119,19 +119,11 @@ pub fn uu_app() -> Command { fn calculate_mode(mode_option: Option<&String>) -> Result { let umask = uucore::mode::get_umask(); - let mut mode = 0o666; // Default mode for FIFOs + let mode = 0o666; // Default mode for FIFOs if let Some(m) = mode_option { - if m.chars().any(|c| c.is_ascii_digit()) { - mode = uucore::mode::parse_numeric(mode, m, false)?; - } else { - for item in m.split(',') { - mode = uucore::mode::parse_symbolic(mode, item, umask, false)?; - } - } + uucore::mode::parse_chmod(mode, m, false, umask) } else { - mode &= !umask; // Apply umask if no mode is specified + Ok(mode & !umask) // Apply umask if no mode is specified } - - Ok(mode) } diff --git a/src/uu/mknod/src/mknod.rs b/src/uu/mknod/src/mknod.rs index ca2640b68..cc22aee5f 100644 --- a/src/uu/mknod/src/mknod.rs +++ b/src/uu/mknod/src/mknod.rs @@ -225,8 +225,10 @@ pub fn uu_app() -> Command { ) } +#[allow(clippy::unnecessary_cast)] fn parse_mode(str_mode: &str) -> Result { - uucore::mode::parse_mode(str_mode) + let default_mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; + uucore::mode::parse_chmod(default_mode, str_mode, true, uucore::mode::get_umask()) .map_err(|e| { translate!( "mknod-error-invalid-mode", @@ -237,7 +239,7 @@ fn parse_mode(str_mode: &str) -> Result { if mode > 0o777 { Err(translate!("mknod-error-mode-permission-bits-only")) } else { - Ok(mode) + Ok(mode as mode_t) } }) } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index a9477fa5f..d562f1fe0 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -7,7 +7,7 @@ // spell-checker:ignore (vars) fperm srwx -use libc::{S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR, mode_t, umask}; +use libc::umask; pub fn parse_numeric(fperm: u32, mut mode: &str, considering_dir: bool) -> Result { let (op, pos) = parse_op(mode).map_or_else(|_| (None, 0), |(op, pos)| (Some(op), pos)); @@ -169,13 +169,6 @@ pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result Result { - let mut new_mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32; - new_mode = parse_chmod(new_mode, mode, true, get_umask())?; - Ok(new_mode as mode_t) -} - pub fn get_umask() -> u32 { // There's no portable way to read the umask without changing it. // We have to replace it and then quickly set it back, hopefully before @@ -207,24 +200,20 @@ mod tests { use super::parse; use super::parse_chmod; - use super::parse_mode; #[test] - fn test_symbolic_modes() { - assert_eq!(parse_mode("u+x").unwrap(), 0o766); - assert_eq!( - parse_mode("+x").unwrap(), - if crate::os::is_wsl_1() { 0o776 } else { 0o777 } - ); - assert_eq!(parse_mode("a-w").unwrap(), 0o444); - assert_eq!(parse_mode("g-r").unwrap(), 0o626); + fn test_chmod_symbolic_modes() { + assert_eq!(parse_chmod(0o666, "u+x", false, 0).unwrap(), 0o766); + assert_eq!(parse_chmod(0o666, "+x", false, 0).unwrap(), 0o777); + assert_eq!(parse_chmod(0o666, "a-w", false, 0).unwrap(), 0o444); + assert_eq!(parse_chmod(0o666, "g-r", false, 0).unwrap(), 0o626); } #[test] - fn test_numeric_modes() { - assert_eq!(parse_mode("644").unwrap(), 0o644); - assert_eq!(parse_mode("+100").unwrap(), 0o766); - assert_eq!(parse_mode("-4").unwrap(), 0o662); + fn test_chmod_numeric_modes() { + assert_eq!(parse_chmod(0o666, "644", false, 0).unwrap(), 0o644); + assert_eq!(parse_chmod(0o666, "+100", false, 0).unwrap(), 0o766); + assert_eq!(parse_chmod(0o666, "-4", false, 0).unwrap(), 0o662); } #[test] @@ -345,19 +334,4 @@ mod tests { // First add user write, then set to 755 (should override) assert_eq!(parse("u+w,755", false, 0).unwrap(), 0o755); } - - #[test] - fn test_chmod_symbolic_modes() { - assert_eq!(parse_chmod(0o666, "u+x", false, 0).unwrap(), 0o766); - assert_eq!(parse_chmod(0o666, "+x", false, 0).unwrap(), 0o777); - assert_eq!(parse_chmod(0o666, "a-w", false, 0).unwrap(), 0o444); - assert_eq!(parse_chmod(0o666, "g-r", false, 0).unwrap(), 0o626); - } - - #[test] - fn test_chmod_numeric_modes() { - assert_eq!(parse_chmod(0o666, "644", false, 0).unwrap(), 0o644); - assert_eq!(parse_chmod(0o666, "+100", false, 0).unwrap(), 0o766); - assert_eq!(parse_chmod(0o666, "-4", false, 0).unwrap(), 0o662); - } } From 63dbffa7f3d0c1134da535dec6117ae92d9c9784 Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sat, 6 Dec 2025 15:08:13 +0000 Subject: [PATCH 5/5] Add tests for multiple mode specifications --- tests/by-util/test_mkfifo.rs | 2 ++ tests/by-util/test_mknod.rs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/tests/by-util/test_mkfifo.rs b/tests/by-util/test_mkfifo.rs index 707adf71c..b90fc0c95 100644 --- a/tests/by-util/test_mkfifo.rs +++ b/tests/by-util/test_mkfifo.rs @@ -99,6 +99,8 @@ fn test_create_fifo_with_mode_and_umask() { test_fifo_creation("u-r,g-w,o+x", 0o022, "p-w-r--rwx"); // spell-checker:disable-line test_fifo_creation("a=rwx,o-w", 0o022, "prwxrwxr-x"); // spell-checker:disable-line test_fifo_creation("=rwx,o-w", 0o022, "prwxr-xr-x"); // spell-checker:disable-line + test_fifo_creation("ug+rw,o+r", 0o022, "prw-rw-rw-"); // spell-checker:disable-line + test_fifo_creation("u=rwx,g=rx,o=", 0o022, "prwxr-x---"); // spell-checker:disable-line } #[test] diff --git a/tests/by-util/test_mknod.rs b/tests/by-util/test_mknod.rs index 34136b828..5d2b08aec 100644 --- a/tests/by-util/test_mknod.rs +++ b/tests/by-util/test_mknod.rs @@ -154,6 +154,22 @@ fn test_mknod_mode_permissions() { } } +#[test] +fn test_mknod_mode_comma_separated() { + let ts = TestScenario::new(util_name!()); + ts.ucmd() + .arg("-m") + .arg("u=rwx,g=rx,o=") + .arg("test_file") + .arg("p") + .succeeds(); + assert!(ts.fixtures.is_fifo("test_file")); + assert_eq!( + ts.fixtures.metadata("test_file").permissions().mode() & 0o777, + 0o750 + ); +} + #[test] #[cfg(feature = "feat_selinux")] fn test_mknod_selinux() {