env: fix "--ignore-signal=PIPE"

- revert #9614 because `Command::exec()` resets the default signal handler for SIGPIPE, interfering with the option `--ignore-signal=PIPE`
- add regression-test for --ignore-signal=PIPE
- add FIXME comment about execvp()

Signed-off-by: Etienne Cordonnier <[email protected]>

Author: Ecordonnier

src/uu/env/src/env.rs

@@ -24,16 +24,18 @@ use nix::sys::signal::{
     SigHandler::{SigDfl, SigIgn},
     SigSet, SigmaskHow, Signal, signal, sigprocmask,
 };
+#[cfg(unix)]
+use nix::unistd::execvp;
 use std::borrow::Cow;
 #[cfg(unix)]
 use std::collections::{BTreeMap, BTreeSet};
 use std::env;
+#[cfg(unix)]
+use std::ffi::CString;
 use std::ffi::{OsStr, OsString};
 use std::io;
 #[cfg(unix)]
 use std::os::unix::ffi::OsStrExt;
-#[cfg(unix)]
-use std::os::unix::process::CommandExt;
 
 use uucore::display::{Quotable, print_all_env_vars};
 use uucore::error::{ExitCode, UError, UResult, USimpleError, UUsageError};
@@ -701,6 +703,24 @@ impl EnvAppData {
         #[cfg(unix)]
         {
             let mut signal_action_log = SignalActionLog::default();
+
+            // Rust ignores SIGPIPE (see https://github.com/rust-lang/rust/issues/62569).
+            // We restore its default action here, but only if the user hasn't explicitly
+            // specified signal handling for SIGPIPE via --ignore-signal or --block-signal.
+            let sigpipe_value = nix::libc::SIGPIPE as usize;
+            let user_handled_sigpipe = opts.ignore_signal.signals.contains(&sigpipe_value)
+                || opts.block_signal.signals.contains(&sigpipe_value);
+
+            if !user_handled_sigpipe
+                && !opts.ignore_signal.apply_all
+                && !opts.block_signal.apply_all
+            {
+                // Only restore SIGPIPE to default if user hasn't explicitly handled it
+                unsafe {
+                    libc::signal(libc::SIGPIPE, libc::SIG_DFL);
+                }
+            }
+
             apply_signal_action(
                 &opts.default_signal,
                 &mut signal_action_log,
@@ -782,16 +802,37 @@ impl EnvAppData {
 
         #[cfg(unix)]
         {
-            // Execute the program using exec, which replaces the current process.
-            let err = std::process::Command::new(&*prog)
-                .arg0(&*arg0)
-                .args(args)
-                .exec();
-
-            // exec() only returns if there was an error
-            match err.kind() {
-                io::ErrorKind::NotFound => Err(self.make_error_no_such_file_or_dir(&prog)),
-                io::ErrorKind::PermissionDenied => {
+            // Convert program name to CString.
+            let prog_os: &OsStr = prog.as_ref();
+            let Ok(prog_cstring) = CString::new(prog_os.as_bytes()) else {
+                return Err(self.make_error_no_such_file_or_dir(&prog));
+            };
+
+            // Prepare arguments for execvp.
+            let mut argv = Vec::new();
+
+            // Convert arg0 to CString.
+            let arg0_os: &OsStr = arg0.as_ref();
+            let Ok(arg0_cstring) = CString::new(arg0_os.as_bytes()) else {
+                return Err(self.make_error_no_such_file_or_dir(&prog));
+            };
+            argv.push(arg0_cstring);
+
+            // Convert remaining arguments to CString.
+            for arg in args {
+                let arg_os = arg;
+                let Ok(arg_cstring) = CString::new(arg_os.as_bytes()) else {
+                    return Err(self.make_error_no_such_file_or_dir(&prog));
+                };
+                argv.push(arg_cstring);
+            }
+
+            // Execute the program using execvp. this replaces the current
+            // process. The execvp function takes care of appending a NULL
+            // argument to the argument list so that we don't have to.
+            match execvp(&prog_cstring, &argv) {
+                Err(nix::errno::Errno::ENOENT) => Err(self.make_error_no_such_file_or_dir(&prog)),
+                Err(nix::errno::Errno::EACCES) => {
                     uucore::show_error!(
                         "{}",
                         translate!(
@@ -801,16 +842,19 @@ impl EnvAppData {
                     );
                     Err(126.into())
                 }
-                _ => {
+                Err(_) => {
                     uucore::show_error!(
                         "{}",
                         translate!(
                             "env-error-unknown",
-                            "error" => err
+                            "error" => "execvp failed"
                         )
                     );
                     Err(126.into())
                 }
+                Ok(_) => {
+                    unreachable!("execvp should never return on success")
+                }
             }
         }
 
@@ -1097,12 +1141,6 @@ fn list_signal_handling(log: &SignalActionLog) {
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    // Rust ignores SIGPIPE (see https://github.com/rust-lang/rust/issues/62569).
-    // We restore its default action here.
-    #[cfg(unix)]
-    unsafe {
-        libc::signal(libc::SIGPIPE, libc::SIG_DFL);
-    }
     EnvAppData::default().run_env(args)
 }
 

tests/by-util/test_env.rs

@@ -1962,3 +1962,79 @@ fn test_non_utf8_env_vars() {
         .succeeds()
         .stdout_contains_bytes(b"NON_UTF8_VAR=hello\x80world");
 }
+
+#[test]
+#[cfg(unix)]
+fn test_ignore_signal_pipe_broken_pipe_regression() {
+    // Test that --ignore-signal=PIPE properly ignores SIGPIPE in child processes.
+    // When SIGPIPE is ignored, processes should handle broken pipes gracefully
+    // instead of being terminated by the signal.
+    //
+    // Regression test for: https://github.com/uutils/coreutils/issues/9617
+
+    use std::io::{BufRead, BufReader};
+    use std::process::{Command, Stdio};
+
+    let scene = TestScenario::new(util_name!());
+
+    // Helper function to simulate a broken pipe scenario (like "yes | head -n1")
+    let test_sigpipe_behavior = |use_ignore_signal: bool| -> i32 {
+        let mut cmd = Command::new(&scene.bin_path);
+        cmd.arg("env");
+
+        if use_ignore_signal {
+            cmd.arg("--ignore-signal=PIPE");
+        }
+
+        cmd.arg("yes").stdout(Stdio::piped()).stderr(Stdio::null());
+
+        let mut child = cmd.spawn().expect("Failed to spawn env process");
+
+        // Read exactly one line then close the pipe to trigger SIGPIPE
+        if let Some(stdout) = child.stdout.take() {
+            let mut reader = BufReader::new(stdout);
+            let mut line = String::new();
+            let _ = reader.read_line(&mut line);
+            // Pipe closes when reader is dropped, sending SIGPIPE to writing process
+        }
+
+        match child.wait() {
+            Ok(status) => {
+                // Process terminated by signal (likely SIGPIPE = 13)
+                // Unix convention: signal death = 128 + signal_number
+                status.code().unwrap_or(141) // 128 + 13
+            }
+            Err(_) => 141,
+        }
+    };
+
+    // Test without signal ignoring - should be killed by SIGPIPE
+    let normal_exit_code = test_sigpipe_behavior(false);
+    println!("Normal 'env yes' exit code: {normal_exit_code}");
+
+    // Test with --ignore-signal=PIPE - should handle broken pipe gracefully
+    let ignore_signal_exit_code = test_sigpipe_behavior(true);
+    println!("With --ignore-signal=PIPE exit code: {ignore_signal_exit_code}");
+
+    // Verify the --ignore-signal=PIPE flag changes the behavior
+    assert!(
+        ignore_signal_exit_code != 141,
+        "--ignore-signal=PIPE had no effect! Process was still killed by SIGPIPE (exit code 141). Normal: {normal_exit_code}, --ignore-signal: {ignore_signal_exit_code}"
+    );
+
+    // Expected behavior:
+    assert_eq!(
+        normal_exit_code, 141,
+        "Without --ignore-signal, process should be killed by SIGPIPE"
+    );
+    assert_ne!(
+        ignore_signal_exit_code, 141,
+        "With --ignore-signal=PIPE, process should NOT be killed by SIGPIPE"
+    );
+
+    // Process should exit gracefully when SIGPIPE is ignored
+    assert!(
+        ignore_signal_exit_code == 0 || ignore_signal_exit_code == 1,
+        "With --ignore-signal=PIPE, process should exit gracefully (0 or 1), got: {ignore_signal_exit_code}"
+    );
+}