Merge pull request #6157 from BenWiederhake/dev-tee-fail-open

tee: Correctly handle read-only files, avoid unnecessary wrapping

src/uu/tee/src/tee.rs

@@ -5,7 +5,7 @@
 
 use clap::{builder::PossibleValue, crate_version, Arg, ArgAction, Command};
 use std::fs::OpenOptions;
-use std::io::{copy, sink, stdin, stdout, Error, ErrorKind, Read, Result, Write};
+use std::io::{copy, stdin, stdout, Error, ErrorKind, Read, Result, Write};
 use std::path::PathBuf;
 use uucore::display::Quotable;
 use uucore::error::UResult;
@@ -148,15 +148,10 @@ fn tee(options: &Options) -> Result<()> {
     }
     let mut writers: Vec<NamedWriter> = options
         .files
-        .clone()
+        .iter()
-        .into_iter()
+        .filter_map(|file| open(file, options.append, options.output_error.as_ref()))
-        .map(|file| {
-            Ok(NamedWriter {
-                name: file.clone(),
-                inner: open(file, options.append, options.output_error.as_ref())?,
-            })
-        })
         .collect::<Result<Vec<NamedWriter>>>()?;
+    let had_open_errors = writers.len() != options.files.len();
 
     writers.insert(
         0,
@@ -181,38 +176,41 @@ fn tee(options: &Options) -> Result<()> {
         _ => Ok(()),
     };
 
-    if res.is_err() || output.flush().is_err() || output.error_occurred() {
+    if had_open_errors || res.is_err() || output.flush().is_err() || output.error_occurred() {
         Err(Error::from(ErrorKind::Other))
     } else {
         Ok(())
     }
 }
 
+/// Tries to open the indicated file and return it. Reports an error if that's not possible.
+/// If that error should lead to program termination, this function returns Some(Err()),
+/// otherwise it returns None.
 fn open(
-    name: String,
+    name: &str,
     append: bool,
     output_error: Option<&OutputErrorMode>,
-) -> Result<Box<dyn Write>> {
+) -> Option<Result<NamedWriter>> {
-    let path = PathBuf::from(name.clone());
+    let path = PathBuf::from(name);
-    let inner: Box<dyn Write> = {
+    let mut options = OpenOptions::new();
-        let mut options = OpenOptions::new();
+    let mode = if append {
-        let mode = if append {
+        options.append(true)
-            options.append(true)
+    } else {
-        } else {
+        options.truncate(true)
-            options.truncate(true)
+    };
-        };
+    match mode.write(true).create(true).open(path.as_path()) {
-        match mode.write(true).create(true).open(path.as_path()) {
+        Ok(file) => Some(Ok(NamedWriter {
-            Ok(file) => Box::new(file),
+            inner: Box::new(file),
-            Err(f) => {
+            name: name.to_owned(),
-                show_error!("{}: {}", name.maybe_quote(), f);
+        })),
-                match output_error {
+        Err(f) => {
-                    Some(OutputErrorMode::Exit | OutputErrorMode::ExitNoPipe) => return Err(f),
+            show_error!("{}: {}", name.maybe_quote(), f);
-                    _ => Box::new(sink()),
+            match output_error {
-                }
+                Some(OutputErrorMode::Exit | OutputErrorMode::ExitNoPipe) => Some(Err(f)),
+                _ => None,
             }
         }
-    };
+    }
-    Ok(Box::new(NamedWriter { inner, name }) as Box<dyn Write>)
 }
 
 struct MultiWriter {

tests/by-util/test_tee.rs

@@ -3,6 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 use crate::common::util::TestScenario;
+use regex::Regex;
 #[cfg(target_os = "linux")]
 use std::fmt::Write;
 
@@ -92,6 +93,27 @@ fn test_tee_no_more_writeable_1() {
     assert_eq!(at.read(file_out), content);
 }
 
+#[test]
+fn test_readonly() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let content_tee = "hello";
+    let content_file = "world";
+    let file_out = "tee_file_out";
+    let writable_file = "tee_file_out2";
+    at.write(file_out, content_file);
+    at.set_readonly(file_out);
+    ucmd.arg(file_out)
+        .arg(writable_file)
+        .pipe_in(content_tee)
+        .ignore_stdin_write_error()
+        .fails()
+        .stdout_is(content_tee)
+        // Windows says "Access is denied" for some reason.
+        .stderr_matches(&Regex::new("(Permission|Access is) denied").unwrap());
+    assert_eq!(at.read(file_out), content_file);
+    assert_eq!(at.read(writable_file), content_tee);
+}
+
 #[test]
 #[cfg(target_os = "linux")]
 fn test_tee_no_more_writeable_2() {