yes,timeout: fix SIGPIPE handling when trapped (fixes #7252)

Author: ChrisDryden

src/uu/timeout/src/timeout.rs

@@ -19,9 +19,6 @@ use uucore::parser::parse_time;
 use uucore::process::ChildExt;
 use uucore::translate;
 
-#[cfg(unix)]
-use uucore::signals::enable_pipe_errors;
-
 use uucore::{
     format_usage, show_error,
     signals::{signal_by_name_or_value, signal_name_by_value},
@@ -105,8 +102,16 @@ impl Config {
     }
 }
 
+#[cfg(unix)]
+uucore::init_sigpipe_capture!();
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
+    #[cfg(unix)]
+    if !uucore::signals::sigpipe_was_ignored() {
+        uucore::signals::enable_pipe_errors()?;
+    }
+
     let matches =
         uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), args, 125)?;
 
@@ -320,26 +325,28 @@ fn timeout(
     if !foreground {
         let _ = setpgid(Pid::from_raw(0), Pid::from_raw(0));
     }
-    #[cfg(unix)]
-    enable_pipe_errors()?;
 
-    let process = &mut process::Command::new(&cmd[0])
+    let mut cmd_builder = process::Command::new(&cmd[0]);
+    cmd_builder
         .args(&cmd[1..])
         .stdin(Stdio::inherit())
         .stdout(Stdio::inherit())
-        .stderr(Stdio::inherit())
-        .spawn()
-        .map_err(|err| {
-            let status_code = match err.kind() {
-                ErrorKind::NotFound => ExitStatus::CommandNotFound.into(),
-                ErrorKind::PermissionDenied => ExitStatus::CannotInvoke.into(),
-                _ => ExitStatus::CannotInvoke.into(),
-            };
-            USimpleError::new(
-                status_code,
-                translate!("timeout-error-failed-to-execute-process", "error" => err),
-            )
-        })?;
+        .stderr(Stdio::inherit());
+
+    #[cfg(unix)]
+    uucore::signals::preserve_sigpipe_for_child(&mut cmd_builder);
+
+    let process = &mut cmd_builder.spawn().map_err(|err| {
+        let status_code = match err.kind() {
+            ErrorKind::NotFound => ExitStatus::CommandNotFound.into(),
+            ErrorKind::PermissionDenied => ExitStatus::CannotInvoke.into(),
+            _ => ExitStatus::CannotInvoke.into(),
+        };
+        USimpleError::new(
+            status_code,
+            translate!("timeout-error-failed-to-execute-process", "error" => err),
+        )
+    })?;
     unblock_sigchld();
     catch_sigterm();
     // Wait for the child process for the specified time period.

src/uu/yes/src/yes.rs

@@ -11,16 +11,22 @@ use std::ffi::OsString;
 use std::io::{self, Write};
 use uucore::error::{UResult, USimpleError};
 use uucore::format_usage;
-#[cfg(unix)]
-use uucore::signals::enable_pipe_errors;
 use uucore::translate;
 
 // it's possible that using a smaller or larger buffer might provide better performance on some
 // systems, but honestly this is good enough
 const BUF_SIZE: usize = 16 * 1024;
 
+#[cfg(unix)]
+uucore::init_sigpipe_capture!();
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
+    #[cfg(unix)]
+    if !uucore::signals::sigpipe_was_ignored() {
+        let _ = uucore::signals::enable_pipe_errors();
+    }
+
     let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
 
     let mut buffer = Vec::with_capacity(BUF_SIZE);
@@ -29,7 +35,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     match exec(&buffer) {
         Ok(()) => Ok(()),
-        Err(err) if err.kind() == io::ErrorKind::BrokenPipe => Ok(()),
+        Err(err) if err.kind() == io::ErrorKind::BrokenPipe => {
+            #[cfg(unix)]
+            if uucore::signals::sigpipe_was_ignored() {
+                uucore::show_error!(
+                    "{}",
+                    translate!("yes-error-standard-output", "error" => err)
+                );
+            }
+            Ok(())
+        }
         Err(err) => Err(USimpleError::new(
             1,
             translate!("yes-error-standard-output", "error" => err),
@@ -113,8 +128,6 @@ fn prepare_buffer(buf: &mut Vec<u8>) {
 pub fn exec(bytes: &[u8]) -> io::Result<()> {
     let stdout = io::stdout();
     let mut stdout = stdout.lock();
-    #[cfg(unix)]
-    enable_pipe_errors()?;
 
     loop {
         stdout.write_all(bytes)?;

src/uucore/src/lib/features/signals.rs

@@ -488,6 +488,25 @@ pub const fn sigpipe_was_ignored() -> bool {
     false
 }
 
+/// Configures a Command to preserve SIGPIPE disposition for child processes.
+/// When the parent had SIGPIPE ignored, this ensures the child also has it ignored.
+#[cfg(unix)]
+pub fn preserve_sigpipe_for_child(cmd: &mut std::process::Command) {
+    use std::os::unix::process::CommandExt;
+    if sigpipe_was_ignored() {
+        unsafe {
+            cmd.pre_exec(|| {
+                nix::sys::signal::signal(
+                    nix::sys::signal::Signal::SIGPIPE,
+                    nix::sys::signal::SigHandler::SigIgn,
+                )
+                .map_err(std::io::Error::other)?;
+                Ok(())
+            });
+        }
+    }
+}
+
 #[cfg(target_os = "linux")]
 pub fn ensure_stdout_not_broken() -> std::io::Result<bool> {
     use nix::{

tests/by-util/test_timeout.rs

@@ -9,6 +9,10 @@ use rstest::rstest;
 
 use uucore::display::Quotable;
 use uutests::new_ucmd;
+#[cfg(all(unix, feature = "yes"))]
+use uutests::util::TestScenario;
+#[cfg(all(unix, feature = "yes"))]
+use uutests::util_name;
 
 #[test]
 fn test_invalid_arg() {
@@ -235,3 +239,17 @@ fn test_command_cannot_invoke() {
     // Try to execute a directory (should give permission denied or similar)
     new_ucmd!().args(&["1", "/"]).fails_with_code(126);
 }
+
+/// Test for https://github.com/uutils/coreutils/issues/7252
+#[test]
+#[cfg(unix)]
+#[cfg(feature = "yes")]
+fn test_sigpipe_ignored_preserves_for_child() {
+    let ts = TestScenario::new(util_name!());
+    let bin = ts.bin_path.clone().into_os_string();
+    let result = ts
+        .cmd_shell("trap '' PIPE; \"$BIN\" timeout 10 \"$BIN\" yes 2>err |:; cat err")
+        .env("BIN", &bin)
+        .succeeds();
+    assert!(result.stdout_str().contains("Broken pipe"));
+}

tests/by-util/test_yes.rs

@@ -9,6 +9,10 @@ use std::process::ExitStatus;
 use std::os::unix::process::ExitStatusExt;
 
 use uutests::new_ucmd;
+#[cfg(unix)]
+use uutests::util::TestScenario;
+#[cfg(unix)]
+use uutests::util_name;
 
 #[cfg(unix)]
 fn check_termination(result: ExitStatus) {
@@ -111,3 +115,16 @@ fn test_non_utf8() {
         &b"\xbf\xff\xee bar\n".repeat(5000),
     );
 }
+
+/// Test for https://github.com/uutils/coreutils/issues/7252
+#[test]
+#[cfg(unix)]
+fn test_sigpipe_ignored_prints_error() {
+    let ts = TestScenario::new(util_name!());
+    let bin = ts.bin_path.clone().into_os_string();
+    let result = ts
+        .cmd_shell("trap '' PIPE; \"$BIN\" yes 2>err |:; cat err")
+        .env("BIN", &bin)
+        .succeeds();
+    assert!(result.stdout_str().contains("Broken pipe"));
+}