diff --git a/src/uu/base32/Cargo.toml b/src/uu/base32/Cargo.toml index 6d2dd4291a9..53daf64fd2f 100644 --- a/src/uu/base32/Cargo.toml +++ b/src/uu/base32/Cargo.toml @@ -18,7 +18,7 @@ doctest = false [dependencies] clap = { workspace = true } fluent = { workspace = true } -uucore = { workspace = true, features = ["encoding"] } +uucore = { workspace = true, features = ["encoding", "stdio"] } [target.'cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))'.dependencies] rustix = { workspace = true, features = ["fs"] } diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 658debbf85c..f8529059447 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -18,6 +18,7 @@ use uucore::encoding::{ }; use uucore::error::{FromIo, UResult, USimpleError, UUsageError, strip_errno}; use uucore::format_usage; +use uucore::stdio::stdin; use uucore::translate; pub const BASE_CMD_PARSE_ERROR: i32 = 1; @@ -151,7 +152,7 @@ pub fn get_input(config: &Config) -> UResult> { // Stdin is already buffered by the OS; wrap once more to reduce syscalls per read. Ok(Box::new(BufReader::with_capacity( DEFAULT_BUF_SIZE, - io::stdin(), + stdin(), ))) } } diff --git a/src/uu/csplit/Cargo.toml b/src/uu/csplit/Cargo.toml index af9ee641124..b622eaca1f0 100644 --- a/src/uu/csplit/Cargo.toml +++ b/src/uu/csplit/Cargo.toml @@ -20,7 +20,7 @@ clap = { workspace = true } fluent = { workspace = true } regex = { workspace = true } thiserror = { workspace = true } -uucore = { workspace = true, features = ["entries", "fs", "format"] } +uucore = { workspace = true, features = ["entries", "fs", "format", "stdio"] } [[bin]] name = "csplit" diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index 87433e93793..59b02cd27eb 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -19,7 +19,8 @@ use clap::{Arg, ArgAction, ArgMatches, Command}; use regex::bytes::Regex; use uucore::display::Quotable; use uucore::error::{FromIo, UResult}; -use uucore::format_usage; +use uucore::stdio::stdin; +use uucore::{format_usage, show}; mod csplit_error; mod diagnostics; @@ -127,10 +128,8 @@ where | patterns::Pattern::SkipToMatch(_, _, patterns::ExecutePattern::Always) ) }); - let ret = do_csplit(&mut split_writer, patterns_vec, &mut input_iter); - - // consume the rest, unless there was an error - let ret = if ret.is_ok() { + let ret = do_csplit(&mut split_writer, patterns_vec, &mut input_iter).and_then(|_| { + // consume the rest, unless there was an error input_iter.rewind_buffer(); if let Some((_, line)) = input_iter.next() { // There is remaining input: create a final split and copy remainder @@ -148,20 +147,25 @@ where } else { Ok(()) } - } else { - ret - }; - // GNU still reports the bytes written to the split that was in progress - // when the failure happened. - if ret.is_err() && !split_writer.split_finished { - let _ = split_writer.finish_split(); - } + }); - // delete files on error by default - if ret.is_err() && !options.keep_files { - split_writer.delete_all_splits()?; + if let Err(e) = ret { + // Show error first + show!(e); + + // GNU still reports the bytes written to the split that was in progress + // when the failure happened. + if !split_writer.split_finished { + let _ = split_writer.finish_split(); + } + + // delete files on error by default + if !options.keep_files { + split_writer.delete_all_splits()?; + } } - ret + + Ok(()) } fn do_csplit( @@ -669,7 +673,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let options = CsplitOptions::new(&matches).map_err(report)?; if file_name == "-" { - let stdin = io::stdin(); + let stdin = stdin(); csplit(&options, &patterns, stdin.lock()).map_err(report)?; } else { let file = File::open(file_name) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index b1867dae76e..794b06cf1b1 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -154,7 +154,14 @@ default = ["signals"] backup-control = [] colors = [] diagnostics = ["dep:ariadne"] -checksum = ["base64-simd", "quoting-style", "rustix/fs", "sum"] +checksum = [ + "base64-simd", + "quoting-style", + "rustix/fs", + "signals", + "stdio", + "sum", +] encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc", "rustix/fs", "rustix/process"] extendedbigdecimal = ["bigdecimal", "num-traits"] @@ -203,6 +210,7 @@ safe-traversal = ["libc", "nix/fs", "nix/dir", "nix/user", "rustix/fs"] selinux = ["dep:selinux", "libc"] smack = ["xattr"] signals = ["nix/fs", "nix/signal", "nix/poll"] +stdio = [] sum = [ "digest", "hex", diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index 6afc1ab7647..ff49b33e7d5 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -35,6 +35,7 @@ error-invalid-argument = Invalid argument error-is-a-directory-text = Is a directory error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Too many levels of symbolic links +error-bad-file-descriptor = Bad file descriptor # Common actions action-copying = copying diff --git a/src/uucore/locales/fr-FR.ftl b/src/uucore/locales/fr-FR.ftl index 80303701383..bf1bac848e3 100644 --- a/src/uucore/locales/fr-FR.ftl +++ b/src/uucore/locales/fr-FR.ftl @@ -34,6 +34,7 @@ error-invalid-argument = Argument invalide error-is-a-directory-text = Est un répertoire error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Trop de niveaux de liens symboliques +error-bad-file-descriptor = Mauvais descripteur de fichier # Actions communes action-copying = copie diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index 4bce7676bd3..244c951b89d 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -129,6 +129,8 @@ pub mod selinux; pub mod signals; #[cfg(all(feature = "smack", target_os = "linux"))] pub mod smack; +#[cfg(feature = "stdio")] +pub mod stdio; #[cfg(all(feature = "systemd_logind", target_os = "linux"))] pub mod systemd_logind; #[cfg(all( diff --git a/src/uucore/src/lib/features/checksum/compute.rs b/src/uucore/src/lib/features/checksum/compute.rs index 90072308460..808c059acc2 100644 --- a/src/uucore/src/lib/features/checksum/compute.rs +++ b/src/uucore/src/lib/features/checksum/compute.rs @@ -16,6 +16,7 @@ use crate::checksum::{ use crate::error::{FromIo, UResult, USimpleError}; use crate::line_ending::LineEnding; use crate::quoting_style::locale_aware_shell_escape; +use crate::stdio::stdin; use crate::sum::DigestOutput; use crate::{show, translate_text}; @@ -245,7 +246,7 @@ where let mut file = io::BufReader::with_capacity( READ_BUFFER_SIZE, if filename == "-" { - stdin_buf = io::stdin(); + stdin_buf = stdin(); Box::new(stdin_buf) as Box } else { let filepath = Path::new(filename); diff --git a/src/uucore/src/lib/features/stdio.rs b/src/uucore/src/lib/features/stdio.rs new file mode 100644 index 00000000000..d1324ad0859 --- /dev/null +++ b/src/uucore/src/lib/features/stdio.rs @@ -0,0 +1,94 @@ +use std::io; + +#[cfg(all(feature = "signals", unix, not(target_os = "fuchsia")))] +use crate::{signals, translate}; + +#[cfg_attr( + not(all(feature = "signals", unix, not(target_os = "fuchsia"))), + expect(clippy::unnecessary_wraps) +)] +fn err_if_stdin_closed() -> io::Result<()> { + #[cfg(all(feature = "signals", unix, not(target_os = "fuchsia")))] + if signals::stdin_was_closed() { + return Err(io::Error::other(translate!("error-bad-file-descriptor"))); + } + + Ok(()) +} + +/// Wrapper around [`std::io::Stdin`] that checks if stdin was closed +/// before trying to read. +pub struct UStdin { + inner: io::Stdin, +} + +impl UStdin { + pub fn lock(&self) -> UStdinLock<'static> { + UStdinLock { + inner: self.inner.lock(), + } + } +} + +impl io::Read for UStdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + err_if_stdin_closed()?; + self.inner.read(buf) + } + + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + err_if_stdin_closed()?; + self.inner.read_to_end(buf) + } + + fn read_to_string(&mut self, buf: &mut String) -> io::Result { + err_if_stdin_closed()?; + self.inner.read_to_string(buf) + } + + fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { + err_if_stdin_closed()?; + self.inner.read_exact(buf) + } +} + +pub struct UStdinLock<'a> { + inner: io::StdinLock<'a>, +} + +impl io::Read for UStdinLock<'_> { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + err_if_stdin_closed()?; + self.inner.read(buf) + } + + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + err_if_stdin_closed()?; + self.inner.read_to_end(buf) + } + + fn read_to_string(&mut self, buf: &mut String) -> io::Result { + err_if_stdin_closed()?; + self.inner.read_to_string(buf) + } + + fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { + err_if_stdin_closed()?; + self.inner.read_exact(buf) + } +} + +impl io::BufRead for UStdinLock<'_> { + fn fill_buf(&mut self) -> io::Result<&[u8]> { + err_if_stdin_closed()?; + self.inner.fill_buf() + } + + fn consume(&mut self, amount: usize) { + self.inner.consume(amount); + } +} + +pub fn stdin() -> UStdin { + UStdin { inner: io::stdin() } +} diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 2eca5a546f6..bf3d6aa6201 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -142,6 +142,8 @@ pub use crate::features::safe_traversal; ) ))] pub use crate::features::signals; +#[cfg(feature = "stdio")] +pub use crate::features::stdio; #[cfg(all( feature = "utmpx", any( diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index dac6c15509e..0fc324683b6 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -180,3 +180,13 @@ fn test_base32_file_with_trailing_slash() { .fails() .stderr_only("base32: a/: Not a directory\n"); } + +#[test] +#[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")] +fn test_closed_stdin() { + new_ucmd!() + .close_stdin(true) + .fails_with_code(1) + .stderr_is("base32: read error: Bad file descriptor\n"); +} diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index 3172a9930fb..e320f21fee1 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -308,3 +308,13 @@ fn test_base64_file_with_trailing_slash() { .fails() .stderr_only("base64: a/: Not a directory\n"); } + +#[test] +#[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")] +fn test_closed_stdin() { + new_ucmd!() + .close_stdin(true) + .fails_with_code(1) + .stderr_is("base64: read error: Bad file descriptor\n"); +} diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index a6b478cff29..76a39b59719 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -3920,3 +3920,13 @@ fn test_numeric_looking_directory_names_are_kept_verbatim() { .fails_with_code(1) .stderr_is("cksum: 007: Is a directory\ncksum: 1e3: Is a directory\n"); } + +#[test] +#[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")] +fn test_closed_stdin() { + new_ucmd!() + .close_stdin(true) + .fails_with_code(1) + .stderr_is("cksum: -: Bad file descriptor\n"); +} diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 55f21141e2c..b80b4105e91 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1915,3 +1915,15 @@ fn test_empty_skip_to_regex_is_accepted() { ucmd.args(&["letters", "%%"]).succeeds().stdout_only("19\n"); assert_eq!(at.read("xx00"), "delta\necho\nfoxtrot\n"); } + +#[test] +#[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")] +fn test_closed_stdin() { + new_ucmd!() + .args(&["-", "2", "4"]) + .close_stdin(true) + .stderr_to_stdout() + .fails_with_code(1) + .stdout_is("csplit: read error: Bad file descriptor\n0\n"); +} diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 25db32151de..725ab8a97ea 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -1559,6 +1559,8 @@ pub struct UCommand { has_run: bool, ignore_stdin_write_error: bool, stdin: Option, + #[cfg(unix)] + close_stdin: bool, stdout: Option, stderr: Option, bytes_into_stdin: Option>, @@ -1651,6 +1653,12 @@ impl UCommand { self } + #[cfg(unix)] + pub fn close_stdin(&mut self, close: bool) -> &mut Self { + self.close_stdin = close; + self + } + pub fn set_stdout>(&mut self, stdout: T) -> &mut Self { self.stdout = Some(stdout.into()); self @@ -2004,6 +2012,26 @@ impl UCommand { let mut stdin_pty: Option = None; #[cfg(not(all(unix, not(target_os = "redox"))))] let stdin_pty: Option = None; + + #[cfg(unix)] + if self.close_stdin { + let close_stdin = || { + // SAFETY: the unsafe state where STDIN is not a valid file + // descriptor is precisely what we want to achieve, to test + // how the program under test reacts. + if unsafe { libc::close(libc::STDIN_FILENO) } == -1 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + }; + + // SAFETY: pre-exec is run after `fork`ing, and before calling + // `exec`. This is safe, since we indeed want to run `close_stdin` + // on the child process. + unsafe { command.pre_exec(close_stdin) }; + } + if self.stderr_to_stdout { let mut output = CapturedOutput::default();