From d906f09e6ed84c83360b26caff70b1e15cde9406 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 6 May 2022 14:01:23 +0200 Subject: [PATCH] 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 | 81 ++++++++++++++++------------ src/uucore/src/lib/features/fsext.rs | 5 +- tests/by-util/test_stat.rs | 34 +++++++++--- 3 files changed, 78 insertions(+), 42 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index cebc6c108..41c9ae4a9 100644 --- a/src/uu/stat/src/stat.rs +++ b/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, + files: Vec, mount_list: Option>, default_tokens: Vec, default_dev_tokens: Vec, @@ -471,24 +473,10 @@ impl Stater { } fn new(matches: &ArgMatches) -> UResult { - let mut files: Vec = matches - .values_of(ARG_FILES) - .map(|v| v.map(ToString::to_string).collect()) + let files = matches + .values_of_os(ARG_FILES) + .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 { - if !self.show_fs { - let result = if self.follow { - fs::metadata(file) + fn do_stat(&self, file: &OsStr, stdin_is_fifo: bool) -> i32 { + let display_name = file.to_string_lossy(); + let file: OsString = if cfg!(unix) && display_name.eq("-") { + 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), ) } diff --git a/src/uucore/src/lib/features/fsext.rs b/src/uucore/src/lib/features/fsext.rs index 3d7ca1c1f..787684e93 100644 --- a/src/uucore/src/lib/features/fsext.rs +++ b/src/uucore/src/lib/features/fsext.rs @@ -85,6 +85,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; @@ -709,9 +710,9 @@ impl FsMeta for StatFs { } #[cfg(unix)] -pub fn statfs>(path: P) -> Result +pub fn statfs

(path: P) -> Result where - Vec: From

, + P: Into>, { match CString::new(path) { Ok(p) => { diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index f1a687981..14aba96fc 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -346,9 +346,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: - @@ -362,17 +374,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") @@ -380,9 +401,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: - @@ -393,7 +413,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")