Skip to content

Commit f2508fd

Browse files
committed
cat: Suppress Broken Pipe errors.
At present, the `cat` command unexpectedly prints an error message when it receives a broken pipe error. As an example, there are many workflows that make use of `cat` and `head` together to process only part of the data. The `head` command will stop reading after a configured number of bytes or lines, subsequently exposing `cat` to a broken pipe condition. Said workflows may fail when they unexpectedly get error messages in their output. Suppress broken pipe errors. Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
1 parent a73c0ea commit f2508fd

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed

src/uu/cat/src/cat.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,44 @@ fn cat_files(files: &[String], options: &OutputOptions) -> UResult<()> {
450450

451451
for path in files {
452452
if let Err(err) = cat_path(path, options, &mut state, out_info.as_ref()) {
453+
// At this point we know that `cat_path` returned an error.
454+
//
455+
// Most errors should be logged, but for purpose of compatibility
456+
// with GNU cat, we do not want to log errors that occur on the
457+
// back of broken pipe.
458+
//
459+
// A common pattern is to use cat piped together with other tools.
460+
// As an example `cat | head -1` could induce a broken pipe
461+
// condition.
462+
//
463+
// Existing workflows choke on there unexpectedly being errors
464+
// printed for this condition.
465+
//
466+
// Different types of errors are wrapped by the `CatError` type,
467+
// and the broken pipe error either come from std::io::Error or
468+
// nix::Error.
469+
//
470+
// Make use of pattern matching to know how to check inside the
471+
// different types of errors.
472+
//
473+
// We need to explicitly borrow std::io::Error because it does not
474+
// implement the Copy trait and consequently it would be partially
475+
// moved, we are also not modifying it and as such would benefit
476+
// from not having to do the copy.
477+
if let CatError::Io(ref err_io) = err {
478+
if err_io.kind() == io::ErrorKind::BrokenPipe {
479+
continue;
480+
}
481+
}
482+
// While nix::Error does implement the Copy trait, we explicitly
483+
// borrow it to avoid the unnecessary copy.
484+
#[cfg(any(target_os = "linux", target_os = "android"))]
485+
if let CatError::Nix(ref err_nix) = err {
486+
// spell-checker:disable-next-line
487+
if *err_nix == nix::errno::Errno::EPIPE {
488+
continue;
489+
}
490+
}
453491
error_messages.push(format!("{}: {err}", path.maybe_quote()));
454492
}
455493
}

tests/by-util/test_cat.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,3 +716,86 @@ fn test_child_when_pipe_in() {
716716

717717
ts.ucmd().pipe_in("content").run().stdout_is("content");
718718
}
719+
720+
#[cfg(target_os = "linux")]
721+
mod linux_only {
722+
use uutests::util::{CmdResult, TestScenario, UCommand};
723+
724+
use std::fmt::Write;
725+
use std::fs::File;
726+
use std::process::Stdio;
727+
use uutests::new_ucmd;
728+
use uutests::util_name;
729+
730+
fn make_broken_pipe() -> File {
731+
use libc::c_int;
732+
use std::os::unix::io::FromRawFd;
733+
734+
let mut fds: [c_int; 2] = [0, 0];
735+
assert!(
736+
(unsafe { libc::pipe(std::ptr::from_mut::<c_int>(&mut fds[0])) } == 0),
737+
"Failed to create pipe"
738+
);
739+
740+
// Drop the read end of the pipe
741+
let _ = unsafe { File::from_raw_fd(fds[0]) };
742+
743+
// Make the write end of the pipe into a Rust File
744+
unsafe { File::from_raw_fd(fds[1]) }
745+
}
746+
747+
fn run_cat(proc: &mut UCommand) -> (String, CmdResult) {
748+
let content = (1..=100_000).fold(String::new(), |mut output, x| {
749+
let _ = writeln!(output, "{x}");
750+
output
751+
});
752+
753+
let result = proc
754+
.ignore_stdin_write_error()
755+
.set_stdin(Stdio::piped())
756+
.run_no_wait()
757+
.pipe_in_and_wait(content.as_bytes());
758+
759+
(content, result)
760+
}
761+
762+
fn expect_silent_success(result: &CmdResult) {
763+
assert!(
764+
result.succeeded(),
765+
"Command was expected to succeed.\nstdout = {}\n stderr = {}",
766+
std::str::from_utf8(result.stdout()).unwrap(),
767+
std::str::from_utf8(result.stderr()).unwrap(),
768+
);
769+
assert!(
770+
result.stderr_str().is_empty(),
771+
"Unexpected data on stderr.\n stderr = {}",
772+
std::str::from_utf8(result.stderr()).unwrap(),
773+
);
774+
}
775+
776+
fn expect_short(result: &CmdResult, contents: &str) {
777+
let compare = result.stdout_str();
778+
assert!(
779+
compare.len() < contents.len(),
780+
"Too many bytes ({}) written to stdout (should be a short count from {})",
781+
compare.len(),
782+
contents.len()
783+
);
784+
assert!(
785+
contents.starts_with(compare),
786+
"Expected truncated output to be a prefix of the correct output, but it isn't.\n Correct: {contents}\n Compare: {compare}"
787+
);
788+
}
789+
790+
#[test]
791+
fn test_pipe_error_default() {
792+
let mut ucmd = new_ucmd!();
793+
794+
let proc = ucmd.set_stdout(make_broken_pipe());
795+
796+
let (content, output) = run_cat(proc);
797+
798+
expect_silent_success(&output);
799+
expect_short(&output, &content);
800+
}
801+
}

0 commit comments

Comments
 (0)