Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkfifo: better handle the mode + umask #6971

Merged
merged 1 commit into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/uu/mkfifo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ path = "src/mkfifo.rs"
[dependencies]
clap = { workspace = true }
libc = { workspace = true }
uucore = { workspace = true }
uucore = { workspace = true, features = ["fs"] }

[[bin]]
name = "mkfifo"
Expand Down
19 changes: 15 additions & 4 deletions src/uu/mkfifo/src/mkfifo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use clap::{crate_version, Arg, ArgAction, Command};
use libc::mkfifo;
use std::ffi::CString;
use std::fs;
use std::os::unix::fs::PermissionsExt;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a unix only program :)

use uucore::display::Quotable;
use uucore::error::{UResult, USimpleError};
use uucore::{format_usage, help_about, help_usage, show};
Expand All @@ -32,11 +34,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
}

let mode = match matches.get_one::<String>(options::MODE) {
// if mode is passed, ignore umask
Some(m) => match usize::from_str_radix(m, 8) {
Ok(m) => m,
Err(e) => return Err(USimpleError::new(1, format!("invalid mode: {e}"))),
},
None => 0o666,
// Default value + umask if present
None => 0o666 & !(uucore::mode::get_umask() as usize),
};

let fifos: Vec<String> = match matches.get_many::<String>(options::FIFO) {
Expand All @@ -47,12 +51,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
for f in fifos {
let err = unsafe {
let name = CString::new(f.as_bytes()).unwrap();
mkfifo(name.as_ptr(), mode as libc::mode_t)
mkfifo(name.as_ptr(), 0o666)
};
if err == -1 {
show!(USimpleError::new(
1,
format!("cannot create fifo {}: File exists", f.quote())
format!("cannot create fifo {}: File exists", f.quote()),
));
}

// Explicitly set the permissions to ignore umask
if let Err(e) = fs::set_permissions(&f, fs::Permissions::from_mode(mode as u32)) {
return Err(USimpleError::new(
1,
format!("cannot set permissions on {}: {}", f.quote(), e),
));
}
}
Expand All @@ -71,7 +83,6 @@ pub fn uu_app() -> Command {
.short('m')
.long(options::MODE)
.help("file permissions for the fifo")
.default_value("0666")
.value_name("MODE"),
)
.arg(
Expand Down
47 changes: 47 additions & 0 deletions tests/by-util/test_mkfifo.rs
sylvestre marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,50 @@ fn test_create_one_fifo_already_exists() {
.fails()
.stderr_is("mkfifo: cannot create fifo 'abcdef': File exists\n");
}

#[test]
fn test_create_fifo_with_mode_and_umask() {
use uucore::fs::display_permissions;
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

let test_fifo_creation = |mode: &str, umask: u16, expected: &str| {
scene
.ucmd()
.arg("-m")
.arg(mode)
.arg(format!("fifo_test_{mode}"))
.umask(libc::mode_t::from(umask))
.succeeds();

let metadata = std::fs::metadata(at.subdir.join(format!("fifo_test_{mode}"))).unwrap();
let permissions = display_permissions(&metadata, true);
assert_eq!(permissions, expected.to_string());
};

test_fifo_creation("734", 0o077, "prwx-wxr--"); // spell-checker:disable-line
test_fifo_creation("706", 0o777, "prwx---rw-"); // spell-checker:disable-line
}

#[test]
fn test_create_fifo_with_umask() {
use uucore::fs::display_permissions;
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

let test_fifo_creation = |umask: u16, expected: &str| {
scene
.ucmd()
.arg("fifo_test")
.umask(libc::mode_t::from(umask))
.succeeds();

let metadata = std::fs::metadata(at.subdir.join("fifo_test")).unwrap();
let permissions = display_permissions(&metadata, true);
assert_eq!(permissions, expected.to_string());
at.remove("fifo_test");
};

test_fifo_creation(0o022, "prw-r--r--"); // spell-checker:disable-line
test_fifo_creation(0o777, "p---------"); // spell-checker:disable-line
}
Loading