Merge pull request #6971 from sylvestre/mkfifo

cakebaker · web-flow · commit a152687d4829 · 2024-12-21T15:31:15.000+01:00
mkfifo: better handle the mode + umask
diff --git a/src/uu/mkfifo/Cargo.toml b/src/uu/mkfifo/Cargo.toml
@@ -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"
diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs
@@ -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;
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError};
 use uucore::{format_usage, help_about, help_usage, show};
@@ -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) {
@@ -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),
             ));
         }
     }
@@ -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(
diff --git a/tests/by-util/test_mkfifo.rs b/tests/by-util/test_mkfifo.rs
@@ -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
+}
