diff --git a/plib/src/modestr.rs b/plib/src/modestr.rs index f612dc114..ecd6fefcc 100644 --- a/plib/src/modestr.rs +++ b/plib/src/modestr.rs @@ -289,9 +289,11 @@ pub fn mutate(init_mode: u32, is_dir: bool, symbolic: &ChmodSymbolic) -> u32 { if who_is_not_specified { let umask = get_umask(); - user &= !(rwx << 6) & !umask; - group &= !(rwx << 3) & !umask; - others &= !rwx & !umask; + // When who is not specified, umask protects bits from being removed + // We remove bits in rwx, except those protected by umask + user &= !(rwx << 6) | (umask & (S_IRWXU as u32)); + group &= !(rwx << 3) | (umask & (S_IRWXG as u32)); + others &= !rwx | (umask & (S_IRWXO as u32)); } } diff --git a/tree/mkfifo.rs b/tree/mkfifo.rs index f76e961f9..4f79a2108 100644 --- a/tree/mkfifo.rs +++ b/tree/mkfifo.rs @@ -17,25 +17,39 @@ use std::io; #[derive(Parser)] #[command(version, about = gettext("mkfifo - make FIFO special files"))] struct Args { - #[arg(short, long, help = gettext("Set the file permission bits of the newly-created FIFO to the specified mode value"))] + #[arg(short, long, allow_hyphen_values = true, help = gettext("Set the file permission bits of the newly-created FIFO to the specified mode value"))] mode: Option, #[arg(help = gettext("A pathname of the FIFO special file to be created"))] files: Vec, } -fn do_mkfifo(filename: &str, mode: &ChmodMode) -> io::Result<()> { +fn do_mkfifo(filename: &str, mode: &ChmodMode, explicit_mode: bool) -> io::Result<()> { let mode_val = match mode { ChmodMode::Absolute(mode, _) => *mode, ChmodMode::Symbolic(sym) => modestr::mutate(0o666, false, sym), }; + // When mode is explicitly specified with -m, bypass umask per POSIX spec + // When no mode is specified, let umask apply normally + let old_umask = if explicit_mode { + Some(unsafe { libc::umask(0) }) + } else { + None + }; + let res = unsafe { libc::mkfifo( filename.as_ptr() as *const libc::c_char, mode_val as libc::mode_t, ) }; + + // Restore the original umask if we changed it + if let Some(umask) = old_umask { + unsafe { libc::umask(umask) }; + } + if res < 0 { return Err(io::Error::last_os_error()); } @@ -53,6 +67,7 @@ fn main() -> Result<(), Box> { let mut exit_code = 0; // parse the mode string + let explicit_mode = args.mode.is_some(); let mode = match args.mode { Some(mode) => modestr::parse(&mode)?, None => ChmodMode::Absolute(0o666, 3), @@ -60,7 +75,7 @@ fn main() -> Result<(), Box> { // apply the mode to each file for filename in &args.files { - if let Err(e) = do_mkfifo(filename, &mode) { + if let Err(e) = do_mkfifo(filename, &mode, explicit_mode) { exit_code = 1; eprintln!("{}: {}", filename, e); } diff --git a/tree/tests/mkfifo/mod.rs b/tree/tests/mkfifo/mod.rs new file mode 100644 index 000000000..c70ab7057 --- /dev/null +++ b/tree/tests/mkfifo/mod.rs @@ -0,0 +1,170 @@ +// +// Copyright (c) 2024 Jeff Garzik +// Copyright (c) 2024 Hemi Labs, Inc. +// +// This file is part of the posixutils-rs project covered under +// the MIT License. For the full license text, please see the LICENSE +// file in the root directory of this project. +// SPDX-License-Identifier: MIT +// + +use plib::testing::{run_test_with_checker, TestPlan}; +use std::fs; +use std::path::Path; +use std::process::Output; + +fn run_mkfifo_test(args: Vec<&str>, expected_exit_code: i32) { + let plan = TestPlan { + cmd: String::from("mkfifo"), + args: args.iter().map(|&s| s.into()).collect(), + stdin_data: String::new(), + expected_out: String::new(), + expected_err: String::new(), + expected_exit_code, + }; + + run_test_with_checker(plan, |_, output: &Output| { + assert_eq!(output.status.code(), Some(expected_exit_code)); + }); +} + +#[test] +fn test_create_single_fifo() { + let fifo_path = "/tmp/posixutils_mkfifo_test_1"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec![fifo_path], 0); + + assert!(Path::new(fifo_path).exists()); + + #[cfg(unix)] + { + use std::os::unix::fs::FileTypeExt; + let metadata = fs::metadata(fifo_path).expect("Unable to get FIFO metadata"); + assert!(metadata.file_type().is_fifo()); + } + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_fifo_already_exists() { + let fifo_path = "/tmp/posixutils_mkfifo_test_2"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec![fifo_path], 0); + assert!(Path::new(fifo_path).exists()); + + run_mkfifo_test(vec![fifo_path], 1); + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_invalid_mode() { + let fifo_path = "/tmp/posixutils_mkfifo_test_3"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec!["-m", "invalid", fifo_path], 1); + + assert!(!Path::new(fifo_path).exists()); +} + +#[test] +fn test_set_fifo_mode_absolute() { + let fifo_path = "/tmp/posixutils_mkfifo_test_4"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec!["-m", "644", fifo_path], 0); + + assert!(Path::new(fifo_path).exists()); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let metadata = fs::metadata(fifo_path).expect("Unable to get FIFO metadata"); + let permissions = metadata.permissions(); + assert_eq!(permissions.mode() & 0o777, 0o644); + } + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_set_fifo_mode_symbolic_plus() { + let fifo_path = "/tmp/posixutils_mkfifo_test_5"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec!["-m", "+x", fifo_path], 0); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let metadata = fs::metadata(fifo_path).expect("Unable to get FIFO metadata"); + let permissions = metadata.permissions(); + assert_eq!( + permissions.mode() & 0o777, + 0o777, + "+x should produce rwxrwxrwx" + ); + } + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_set_fifo_mode_symbolic_minus() { + let fifo_path = "/tmp/posixutils_mkfifo_test_6"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec!["-m", "-w", fifo_path], 0); + + assert!(Path::new(fifo_path).exists()); + + #[cfg(unix)] + { + use std::os::unix::fs::FileTypeExt; + let metadata = fs::metadata(fifo_path).expect("Unable to get FIFO metadata"); + assert!(metadata.file_type().is_fifo()); + } + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_set_fifo_mode_symbolic_who_specified() { + let fifo_path = "/tmp/posixutils_mkfifo_test_7"; + let _ = fs::remove_file(fifo_path); + + run_mkfifo_test(vec!["-m", "a-w", fifo_path], 0); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let metadata = fs::metadata(fifo_path).expect("Unable to get FIFO metadata"); + let permissions = metadata.permissions(); + assert_eq!( + permissions.mode() & 0o777, + 0o444, + "a-w should produce r--r--r--" + ); + } + + fs::remove_file(fifo_path).expect("Unable to remove test FIFO"); +} + +#[test] +fn test_create_multiple_fifos() { + let fifo1 = "/tmp/posixutils_mkfifo_test_8a"; + let fifo2 = "/tmp/posixutils_mkfifo_test_8b"; + let _ = fs::remove_file(fifo1); + let _ = fs::remove_file(fifo2); + + run_mkfifo_test(vec![fifo1, fifo2], 0); + + assert!(Path::new(fifo1).exists()); + assert!(Path::new(fifo2).exists()); + + fs::remove_file(fifo1).expect("Unable to remove test FIFO"); + fs::remove_file(fifo2).expect("Unable to remove test FIFO"); +} diff --git a/tree/tests/tree-tests.rs b/tree/tests/tree-tests.rs index 4f6282853..75738d3d7 100644 --- a/tree/tests/tree-tests.rs +++ b/tree/tests/tree-tests.rs @@ -15,6 +15,7 @@ mod du; mod link; mod ls; mod mkdir; +mod mkfifo; mod mv; mod readlink; mod rm;