Skip to content

Commit 4d40671

Browse files
authored
cat: Fix reporting "input file is output file" error when outputting to an input file (#8025)
* cat: Check if a file can be overwritten safely in Unix * cat: Check if a file can be overwritten safely in Windows * cat: Test writing read-write file that is input and output * cat: Unit test `is_appending` function * cat: Unit test `is_unsafe_overwrite` function * cat: Comment why a few function calls could return Err * cat: Remove obvious comments from test
1 parent 9e21259 commit 4d40671

7 files changed

Lines changed: 253 additions & 43 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/cat/Cargo.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ uucore = { workspace = true, features = ["fast-inc", "fs", "pipes"] }
2626
[target.'cfg(unix)'.dependencies]
2727
nix = { workspace = true }
2828

29+
[target.'cfg(windows)'.dependencies]
30+
winapi-util = { workspace = true }
31+
windows-sys = { workspace = true, features = ["Win32_Storage_FileSystem"] }
32+
33+
[dev-dependencies]
34+
tempfile = { workspace = true }
35+
2936
[[bin]]
3037
name = "cat"
3138
path = "src/main.rs"

src/uu/cat/src/cat.rs

Lines changed: 11 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
// file that was distributed with this source code.
55

66
// spell-checker:ignore (ToDO) nonprint nonblank nonprinting ELOOP
7+
8+
mod platform;
9+
10+
use crate::platform::is_unsafe_overwrite;
711
use std::fs::{File, metadata};
812
use std::io::{self, BufWriter, IsTerminal, Read, Write};
913
/// Unix domain socket support
@@ -18,12 +22,9 @@ use std::os::unix::net::UnixStream;
1822

1923
use clap::{Arg, ArgAction, Command};
2024
use memchr::memchr2;
21-
#[cfg(unix)]
22-
use nix::fcntl::{FcntlArg, fcntl};
2325
use thiserror::Error;
2426
use uucore::display::Quotable;
2527
use uucore::error::UResult;
26-
use uucore::fs::FileInformation;
2728
use uucore::locale::get_message;
2829
use uucore::{fast_inc::fast_inc_one, format_usage};
2930

@@ -366,42 +367,17 @@ fn cat_handle<R: FdReadable>(
366367
}
367368
}
368369

369-
/// Whether this process is appending to stdout.
370-
#[cfg(unix)]
371-
fn is_appending() -> bool {
372-
let stdout = io::stdout();
373-
let Ok(flags) = fcntl(stdout.as_fd(), FcntlArg::F_GETFL) else {
374-
return false;
375-
};
376-
// TODO Replace `1 << 10` with `nix::fcntl::Oflag::O_APPEND`.
377-
let o_append = 1 << 10;
378-
(flags & o_append) > 0
379-
}
380-
381-
#[cfg(not(unix))]
382-
fn is_appending() -> bool {
383-
false
384-
}
385-
386-
fn cat_path(
387-
path: &str,
388-
options: &OutputOptions,
389-
state: &mut OutputState,
390-
out_info: Option<&FileInformation>,
391-
) -> CatResult<()> {
370+
fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> CatResult<()> {
392371
match get_input_type(path)? {
393372
InputType::StdIn => {
394373
let stdin = io::stdin();
395-
let in_info = FileInformation::from_file(&stdin)?;
374+
if is_unsafe_overwrite(&stdin, &io::stdout()) {
375+
return Err(CatError::OutputIsInput);
376+
}
396377
let mut handle = InputHandle {
397378
reader: stdin,
398379
is_interactive: io::stdin().is_terminal(),
399380
};
400-
if let Some(out_info) = out_info {
401-
if in_info == *out_info && is_appending() {
402-
return Err(CatError::OutputIsInput);
403-
}
404-
}
405381
cat_handle(&mut handle, options, state)
406382
}
407383
InputType::Directory => Err(CatError::IsDirectory),
@@ -417,15 +393,9 @@ fn cat_path(
417393
}
418394
_ => {
419395
let file = File::open(path)?;
420-
421-
if let Some(out_info) = out_info {
422-
if out_info.file_size() != 0
423-
&& FileInformation::from_file(&file).ok().as_ref() == Some(out_info)
424-
{
425-
return Err(CatError::OutputIsInput);
426-
}
396+
if is_unsafe_overwrite(&file, &io::stdout()) {
397+
return Err(CatError::OutputIsInput);
427398
}
428-
429399
let mut handle = InputHandle {
430400
reader: file,
431401
is_interactive: false,
@@ -436,8 +406,6 @@ fn cat_path(
436406
}
437407

438408
fn cat_files(files: &[String], options: &OutputOptions) -> UResult<()> {
439-
let out_info = FileInformation::from_file(&io::stdout()).ok();
440-
441409
let mut state = OutputState {
442410
line_number: LineNumber::new(),
443411
at_line_start: true,
@@ -447,7 +415,7 @@ fn cat_files(files: &[String], options: &OutputOptions) -> UResult<()> {
447415
let mut error_messages: Vec<String> = Vec::new();
448416

449417
for path in files {
450-
if let Err(err) = cat_path(path, options, &mut state, out_info.as_ref()) {
418+
if let Err(err) = cat_path(path, options, &mut state) {
451419
error_messages.push(format!("{}: {err}", path.maybe_quote()));
452420
}
453421
}

src/uu/cat/src/platform/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// This file is part of the uutils coreutils package.
2+
//
3+
// For the full copyright and license information, please view the LICENSE
4+
// file that was distributed with this source code.
5+
6+
#[cfg(unix)]
7+
pub use self::unix::is_unsafe_overwrite;
8+
9+
#[cfg(windows)]
10+
pub use self::windows::is_unsafe_overwrite;
11+
12+
#[cfg(unix)]
13+
mod unix;
14+
15+
#[cfg(windows)]
16+
mod windows;

src/uu/cat/src/platform/unix.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// This file is part of the uutils coreutils package.
2+
//
3+
// For the full copyright and license information, please view the LICENSE
4+
// file that was distributed with this source code.
5+
6+
// spell-checker:ignore lseek seekable
7+
8+
use nix::fcntl::{FcntlArg, OFlag, fcntl};
9+
use nix::unistd::{Whence, lseek};
10+
use std::os::fd::AsFd;
11+
use uucore::fs::FileInformation;
12+
13+
/// An unsafe overwrite occurs when the same nonempty file is used as both stdin and stdout,
14+
/// and the file offset of stdin is positioned earlier than that of stdout.
15+
/// In this scenario, bytes read from stdin are written to a later part of the file
16+
/// via stdout, which can then be read again by stdin and written again by stdout,
17+
/// causing an infinite loop and potential file corruption.
18+
pub fn is_unsafe_overwrite<I: AsFd, O: AsFd>(input: &I, output: &O) -> bool {
19+
// `FileInformation::from_file` returns an error if the file descriptor is closed, invalid,
20+
// or refers to a non-regular file (e.g., socket, pipe, or special device).
21+
let Ok(input_info) = FileInformation::from_file(input) else {
22+
return false;
23+
};
24+
let Ok(output_info) = FileInformation::from_file(output) else {
25+
return false;
26+
};
27+
if input_info != output_info || output_info.file_size() == 0 {
28+
return false;
29+
}
30+
if is_appending(output) {
31+
return true;
32+
}
33+
// `lseek` returns an error if the file descriptor is closed or it refers to
34+
// a non-seekable resource (e.g., pipe, socket, or some devices).
35+
let Ok(input_pos) = lseek(input.as_fd(), 0, Whence::SeekCur) else {
36+
return false;
37+
};
38+
let Ok(output_pos) = lseek(output.as_fd(), 0, Whence::SeekCur) else {
39+
return false;
40+
};
41+
input_pos < output_pos
42+
}
43+
44+
/// Whether the file is opened with the `O_APPEND` flag
45+
fn is_appending<F: AsFd>(file: &F) -> bool {
46+
let flags_raw = fcntl(file.as_fd(), FcntlArg::F_GETFL).unwrap_or_default();
47+
let flags = OFlag::from_bits_truncate(flags_raw);
48+
flags.contains(OFlag::O_APPEND)
49+
}
50+
51+
#[cfg(test)]
52+
mod tests {
53+
use crate::platform::unix::{is_appending, is_unsafe_overwrite};
54+
use std::fs::OpenOptions;
55+
use std::io::{Seek, SeekFrom, Write};
56+
use tempfile::NamedTempFile;
57+
58+
#[test]
59+
fn test_is_appending() {
60+
let temp_file = NamedTempFile::new().unwrap();
61+
assert!(!is_appending(&temp_file));
62+
63+
let read_file = OpenOptions::new().read(true).open(&temp_file).unwrap();
64+
assert!(!is_appending(&read_file));
65+
66+
let write_file = OpenOptions::new().write(true).open(&temp_file).unwrap();
67+
assert!(!is_appending(&write_file));
68+
69+
let append_file = OpenOptions::new().append(true).open(&temp_file).unwrap();
70+
assert!(is_appending(&append_file));
71+
}
72+
73+
#[test]
74+
fn test_is_unsafe_overwrite() {
75+
// Create two temp files one of which is empty
76+
let empty = NamedTempFile::new().unwrap();
77+
let mut nonempty = NamedTempFile::new().unwrap();
78+
nonempty.write_all(b"anything").unwrap();
79+
nonempty.seek(SeekFrom::Start(0)).unwrap();
80+
81+
// Using a different file as input and output does not result in an overwrite
82+
assert!(!is_unsafe_overwrite(&empty, &nonempty));
83+
84+
// Overwriting an empty file is always safe
85+
assert!(!is_unsafe_overwrite(&empty, &empty));
86+
87+
// Overwriting a nonempty file with itself is safe
88+
assert!(!is_unsafe_overwrite(&nonempty, &nonempty));
89+
90+
// Overwriting an empty file opened in append mode is safe
91+
let empty_append = OpenOptions::new().append(true).open(&empty).unwrap();
92+
assert!(!is_unsafe_overwrite(&empty, &empty_append));
93+
94+
// Overwriting a nonempty file opened in append mode is unsafe
95+
let nonempty_append = OpenOptions::new().append(true).open(&nonempty).unwrap();
96+
assert!(is_unsafe_overwrite(&nonempty, &nonempty_append));
97+
98+
// Overwriting a file opened in write mode is safe
99+
let mut nonempty_write = OpenOptions::new().write(true).open(&nonempty).unwrap();
100+
assert!(!is_unsafe_overwrite(&nonempty, &nonempty_write));
101+
102+
// Overwriting a file when the input and output file descriptors are pointing to
103+
// different offsets is safe if the input offset is further than the output offset
104+
nonempty_write.seek(SeekFrom::Start(1)).unwrap();
105+
assert!(!is_unsafe_overwrite(&nonempty_write, &nonempty));
106+
assert!(is_unsafe_overwrite(&nonempty, &nonempty_write));
107+
}
108+
}

src/uu/cat/src/platform/windows.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// This file is part of the uutils coreutils package.
2+
//
3+
// For the full copyright and license information, please view the LICENSE
4+
// file that was distributed with this source code.
5+
6+
use std::ffi::OsString;
7+
use std::os::windows::ffi::OsStringExt;
8+
use std::path::PathBuf;
9+
use uucore::fs::FileInformation;
10+
use winapi_util::AsHandleRef;
11+
use windows_sys::Win32::Storage::FileSystem::{
12+
FILE_NAME_NORMALIZED, GetFinalPathNameByHandleW, VOLUME_NAME_NT,
13+
};
14+
15+
/// An unsafe overwrite occurs when the same file is used as both stdin and stdout
16+
/// and the stdout file is not empty.
17+
pub fn is_unsafe_overwrite<I: AsHandleRef, O: AsHandleRef>(input: &I, output: &O) -> bool {
18+
if !is_same_file_by_path(input, output) {
19+
return false;
20+
}
21+
22+
// Check if the output file is empty
23+
FileInformation::from_file(output)
24+
.map(|info| info.file_size() > 0)
25+
.unwrap_or(false)
26+
}
27+
28+
/// Get the file path for a file handle
29+
fn get_file_path_from_handle<F: AsHandleRef>(file: &F) -> Option<PathBuf> {
30+
let handle = file.as_raw();
31+
let mut path_buf = vec![0u16; 4096];
32+
33+
// SAFETY: We should check how many bytes was written to `path_buf`
34+
// and only read that many bytes from it.
35+
let len = unsafe {
36+
GetFinalPathNameByHandleW(
37+
handle,
38+
path_buf.as_mut_ptr(),
39+
path_buf.len() as u32,
40+
FILE_NAME_NORMALIZED | VOLUME_NAME_NT,
41+
)
42+
};
43+
if len == 0 {
44+
return None;
45+
}
46+
let path = OsString::from_wide(&path_buf[..len as usize]);
47+
Some(PathBuf::from(path))
48+
}
49+
50+
/// Compare two file handles if they correspond to the same file
51+
fn is_same_file_by_path<A: AsHandleRef, B: AsHandleRef>(a: &A, b: &B) -> bool {
52+
match (get_file_path_from_handle(a), get_file_path_from_handle(b)) {
53+
(Some(path1), Some(path2)) => path1 == path2,
54+
_ => false,
55+
}
56+
}

tests/by-util/test_cat.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use rlimit::Resource;
99
#[cfg(unix)]
1010
use std::fs::File;
1111
use std::fs::OpenOptions;
12+
use std::fs::read_to_string;
1213
use std::process::Stdio;
1314
use uutests::at_and_ucmd;
1415
use uutests::new_ucmd;
@@ -637,6 +638,57 @@ fn test_write_to_self() {
637638
);
638639
}
639640

641+
/// Test derived from the following GNU test in `tests/cat/cat-self.sh`:
642+
///
643+
/// `cat fxy2 fy 1<>fxy2`
644+
// TODO: make this work on windows
645+
#[test]
646+
#[cfg(unix)]
647+
fn test_successful_write_to_read_write_self() {
648+
let (at, mut ucmd) = at_and_ucmd!();
649+
at.write("fy", "y");
650+
at.write("fxy2", "x");
651+
652+
// Open `rw_file` as both stdin and stdout (read/write)
653+
let fxy2_file_path = at.plus("fxy2");
654+
let fxy2_file = OpenOptions::new()
655+
.read(true)
656+
.write(true)
657+
.open(&fxy2_file_path)
658+
.unwrap();
659+
ucmd.args(&["fxy2", "fy"]).set_stdout(fxy2_file).succeeds();
660+
661+
// The contents of `fxy2` and `fy` files should be merged
662+
let fxy2_contents = read_to_string(fxy2_file_path).unwrap();
663+
assert_eq!(fxy2_contents, "xy");
664+
}
665+
666+
/// Test derived from the following GNU test in `tests/cat/cat-self.sh`:
667+
///
668+
/// `cat fx fx3 1<>fx3`
669+
#[test]
670+
fn test_failed_write_to_read_write_self() {
671+
let (at, mut ucmd) = at_and_ucmd!();
672+
at.write("fx", "g");
673+
at.write("fx3", "bold");
674+
675+
// Open `rw_file` as both stdin and stdout (read/write)
676+
let fx3_file_path = at.plus("fx3");
677+
let fx3_file = OpenOptions::new()
678+
.read(true)
679+
.write(true)
680+
.open(&fx3_file_path)
681+
.unwrap();
682+
ucmd.args(&["fx", "fx3"])
683+
.set_stdout(fx3_file)
684+
.fails_with_code(1)
685+
.stderr_only("cat: fx3: input file is output file\n");
686+
687+
// The contents of `fx` should have overwritten the beginning of `fx3`
688+
let fx3_contents = read_to_string(fx3_file_path).unwrap();
689+
assert_eq!(fx3_contents, "gold");
690+
}
691+
640692
#[test]
641693
#[cfg(unix)]
642694
#[cfg(not(target_os = "openbsd"))]

0 commit comments

Comments
 (0)