stat: improve handling of stdin/fifo (fix #3485)

* fix https://github.com/uutils/coreutils/issues/3485
* improve the workaround from #3280
* add tests

src/uu/stat/src/stat.rs

@@ -19,7 +19,9 @@ use uucore::{entries, format_usage};
 use clap::{crate_version, Arg, ArgMatches, Command};
 use std::borrow::Cow;
 use std::convert::AsRef;
+use std::ffi::{OsStr, OsString};
 use std::os::unix::fs::{FileTypeExt, MetadataExt};
+use std::os::unix::prelude::OsStrExt;
 use std::path::Path;
 use std::{cmp, fs, iter};
 
@@ -221,7 +223,7 @@ pub struct Stater {
     follow: bool,
     show_fs: bool,
     from_user: bool,
-    files: Vec<String>,
+    files: Vec<OsString>,
     mount_list: Option<Vec<String>>,
     default_tokens: Vec<Token>,
     default_dev_tokens: Vec<Token>,
@@ -471,24 +473,10 @@ impl Stater {
     }
 
     fn new(matches: &ArgMatches) -> UResult<Self> {
-        let mut files: Vec<String> = matches
+        let files = matches
-            .values_of(ARG_FILES)
+            .values_of_os(ARG_FILES)
-            .map(|v| v.map(ToString::to_string).collect())
+            .map(|v| v.map(OsString::from).collect())
             .unwrap_or_default();
-        #[cfg(unix)]
-        if files.contains(&String::from("-")) {
-            let redirected_path = Path::new("/dev/stdin")
-                .canonicalize()
-                .expect("unable to canonicalize /dev/stdin")
-                .into_os_string()
-                .into_string()
-                .unwrap();
-            for file in &mut files {
-                if file == "-" {
-                    *file = redirected_path.clone();
-                }
-            }
-        }
         let format_str = if matches.is_present(options::PRINTF) {
             matches
                 .value_of(options::PRINTF)
@@ -550,19 +538,37 @@ impl Stater {
     }
 
     fn exec(&self) -> i32 {
+        let mut stdin_is_fifo = false;
+        if cfg!(unix) {
+            if let Ok(md) = fs::metadata("/dev/stdin") {
+                stdin_is_fifo = md.file_type().is_fifo();
+            }
+        }
+
         let mut ret = 0;
         for f in &self.files {
-            ret |= self.do_stat(f.as_str());
+            ret |= self.do_stat(f, stdin_is_fifo);
         }
         ret
     }
 
-    fn do_stat(&self, file: &str) -> i32 {
+    fn do_stat(&self, file: &OsStr, stdin_is_fifo: bool) -> i32 {
-        if !self.show_fs {
+        let display_name = file.to_string_lossy();
-            let result = if self.follow {
+        let file: OsString = if cfg!(unix) && display_name.eq("-") {
-                fs::metadata(file)
+            if let Ok(p) = Path::new("/dev/stdin").canonicalize() {
+                p.into_os_string()
             } else {
-                fs::symlink_metadata(file)
+                OsString::from("/dev/stdin")
+            }
+        } else {
+            OsString::from(file)
+        };
+
+        if !self.show_fs {
+            let result = if self.follow || stdin_is_fifo && display_name.eq("-") {
+                fs::metadata(&file)
+            } else {
+                fs::symlink_metadata(&file)
             };
             match result {
                 Ok(meta) => {
@@ -658,28 +664,32 @@ impl Stater {
 
                                     // mount point
                                     'm' => {
-                                        arg = self.find_mount_point(file).unwrap();
+                                        arg = self.find_mount_point(&file).unwrap();
                                         output_type = OutputType::Str;
                                     }
 
                                     // file name
                                     'n' => {
-                                        arg = file.to_owned();
+                                        arg = display_name.to_string();
                                         output_type = OutputType::Str;
                                     }
                                     // quoted file name with dereference if symbolic link
                                     'N' => {
                                         if file_type.is_symlink() {
-                                            let dst = match fs::read_link(file) {
+                                            let dst = match fs::read_link(&file) {
                                                 Ok(path) => path,
                                                 Err(e) => {
                                                     println!("{}", e);
                                                     return 1;
                                                 }
                                             };
-                                            arg = format!("{} -> {}", file.quote(), dst.quote());
+                                            arg = format!(
+                                                "{} -> {}",
+                                                display_name.quote(),
+                                                dst.quote()
+                                            );
                                         } else {
-                                            arg = file.to_string();
+                                            arg = display_name.to_string();
                                         }
                                         output_type = OutputType::Str;
                                     }
@@ -771,12 +781,16 @@ impl Stater {
                     }
                 }
                 Err(e) => {
-                    show_error!("cannot stat {}: {}", file.quote(), e);
+                    show_error!("cannot stat {}: {}", display_name.quote(), e);
                     return 1;
                 }
             }
         } else {
-            match statfs(file) {
+            #[cfg(unix)]
+            let p = file.as_bytes();
+            #[cfg(not(unix))]
+            let p = file.into_string().unwrap();
+            match statfs(p) {
                 Ok(meta) => {
                     let tokens = &self.default_tokens;
 
@@ -829,7 +843,7 @@ impl Stater {
                                     }
                                     // file name
                                     'n' => {
-                                        arg = file.to_owned();
+                                        arg = display_name.to_string();
                                         output_type = OutputType::Str;
                                     }
                                     // block size (for faster transfers)
@@ -866,7 +880,7 @@ impl Stater {
                 Err(e) => {
                     show_error!(
                         "cannot read file system information for {}: {}",
-                        file.quote(),
+                        display_name.quote(),
                         e
                     );
                     return 1;
@@ -1028,6 +1042,7 @@ pub fn uu_app<'a>() -> Command<'a> {
             Arg::new(ARG_FILES)
                 .multiple_occurrences(true)
                 .takes_value(true)
+                .allow_invalid_utf8(true)
                 .min_values(1),
         )
 }

src/uucore/src/lib/features/fsext.rs

@@ -84,6 +84,7 @@ use std::ffi::CString;
 use std::io::Error as IOError;
 #[cfg(unix)]
 use std::mem;
+#[cfg(not(unix))]
 use std::path::Path;
 use std::time::UNIX_EPOCH;
 
@@ -708,9 +709,9 @@ impl FsMeta for StatFs {
 }
 
 #[cfg(unix)]
-pub fn statfs<P: AsRef<Path>>(path: P) -> Result<StatFs, String>
+pub fn statfs<P>(path: P) -> Result<StatFs, String>
 where
-    Vec<u8>: From<P>,
+    P: Into<Vec<u8>>,
 {
     match CString::new(path) {
         Ok(p) => {

tests/by-util/test_stat.rs

@@ -312,9 +312,21 @@ fn test_printf() {
     ts.ucmd().args(&args).succeeds().stdout_is(expected_stdout);
 }
 
-#[cfg(unix)]
 #[test]
-#[cfg(disable_until_fixed)]
+#[cfg(unix)]
+fn test_pipe_fifo() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkfifo("FIFO");
+    ucmd.arg("FIFO")
+        .run()
+        .no_stderr()
+        .stdout_contains("fifo")
+        .stdout_contains("File: FIFO")
+        .succeeded();
+}
+
+#[test]
+#[cfg(target_os = "linux")]
 fn test_stdin_pipe_fifo1() {
     // $ echo | stat -
     // File: -
@@ -328,17 +340,26 @@ fn test_stdin_pipe_fifo1() {
         .stdout_contains("fifo")
         .stdout_contains("File: -")
         .succeeded();
+
+    new_ucmd!()
+        .args(&["-L", "-"])
+        .set_stdin(std::process::Stdio::piped())
+        .run()
+        .no_stderr()
+        .stdout_contains("fifo")
+        .stdout_contains("File: -")
+        .succeeded();
 }
 
-#[cfg(unix)]
 #[test]
-#[cfg(disable_until_fixed)]
+#[cfg(target_os = "linux")]
 fn test_stdin_pipe_fifo2() {
     // $ stat -
     // File: -
     // Size: 0               Blocks: 0          IO Block: 1024   character special file
     new_ucmd!()
         .arg("-")
+        .set_stdin(std::process::Stdio::null())
         .run()
         .no_stderr()
         .stdout_contains("character special file")
@@ -346,9 +367,8 @@ fn test_stdin_pipe_fifo2() {
         .succeeded();
 }
 
-#[cfg(unix)]
 #[test]
-#[cfg(disable_until_fixed)]
+#[cfg(target_os = "linux")]
 fn test_stdin_redirect() {
     // $ touch f && stat - < f
     // File: -
@@ -359,7 +379,7 @@ fn test_stdin_redirect() {
     at.touch("f");
     new_ucmd!()
         .arg("-")
-        .set_stdin(std::fs::File::open("f").unwrap())
+        .set_stdin(std::fs::File::open(at.plus("f")).unwrap())
         .run()
         .no_stderr()
         .stdout_contains("regular empty file")