-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
cat: Fix reporting "input file is output file" error when outputting to an input file #8025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6dbe45b
aaa8f68
ba4d7d8
9d32c16
cb9bce3
e217ef2
b2b9c99
ada46eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // This file is part of the uutils coreutils package. | ||
| // | ||
| // For the full copyright and license information, please view the LICENSE | ||
| // file that was distributed with this source code. | ||
|
|
||
| #[cfg(unix)] | ||
| pub use self::unix::is_unsafe_overwrite; | ||
|
|
||
| #[cfg(windows)] | ||
| pub use self::windows::is_unsafe_overwrite; | ||
|
|
||
| #[cfg(unix)] | ||
| mod unix; | ||
|
|
||
| #[cfg(windows)] | ||
| mod windows; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // This file is part of the uutils coreutils package. | ||
| // | ||
| // For the full copyright and license information, please view the LICENSE | ||
| // file that was distributed with this source code. | ||
|
|
||
| // spell-checker:ignore lseek seekable | ||
|
|
||
| use nix::fcntl::{FcntlArg, OFlag, fcntl}; | ||
| use nix::unistd::{Whence, lseek}; | ||
| use std::os::fd::AsFd; | ||
| use uucore::fs::FileInformation; | ||
|
|
||
| /// An unsafe overwrite occurs when the same nonempty file is used as both stdin and stdout, | ||
| /// and the file offset of stdin is positioned earlier than that of stdout. | ||
| /// In this scenario, bytes read from stdin are written to a later part of the file | ||
| /// via stdout, which can then be read again by stdin and written again by stdout, | ||
| /// causing an infinite loop and potential file corruption. | ||
| pub fn is_unsafe_overwrite<I: AsFd, O: AsFd>(input: &I, output: &O) -> bool { | ||
| // `FileInformation::from_file` returns an error if the file descriptor is closed, invalid, | ||
| // or refers to a non-regular file (e.g., socket, pipe, or special device). | ||
| let Ok(input_info) = FileInformation::from_file(input) else { | ||
| return false; | ||
| }; | ||
| let Ok(output_info) = FileInformation::from_file(output) else { | ||
| return false; | ||
| }; | ||
| if input_info != output_info || output_info.file_size() == 0 { | ||
| return false; | ||
| } | ||
| if is_appending(output) { | ||
| return true; | ||
| } | ||
| // `lseek` returns an error if the file descriptor is closed or it refers to | ||
| // a non-seekable resource (e.g., pipe, socket, or some devices). | ||
| let Ok(input_pos) = lseek(input.as_fd(), 0, Whence::SeekCur) else { | ||
| return false; | ||
| }; | ||
| let Ok(output_pos) = lseek(output.as_fd(), 0, Whence::SeekCur) else { | ||
| return false; | ||
| }; | ||
| input_pos < output_pos | ||
| } | ||
|
|
||
| /// Whether the file is opened with the `O_APPEND` flag | ||
| fn is_appending<F: AsFd>(file: &F) -> bool { | ||
| let flags_raw = fcntl(file.as_fd(), FcntlArg::F_GETFL).unwrap_or_default(); | ||
| let flags = OFlag::from_bits_truncate(flags_raw); | ||
| flags.contains(OFlag::O_APPEND) | ||
| } | ||
frendsick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use crate::platform::unix::{is_appending, is_unsafe_overwrite}; | ||
| use std::fs::OpenOptions; | ||
| use std::io::{Seek, SeekFrom, Write}; | ||
| use tempfile::NamedTempFile; | ||
|
|
||
| #[test] | ||
| fn test_is_appending() { | ||
| let temp_file = NamedTempFile::new().unwrap(); | ||
| assert!(!is_appending(&temp_file)); | ||
|
|
||
| let read_file = OpenOptions::new().read(true).open(&temp_file).unwrap(); | ||
| assert!(!is_appending(&read_file)); | ||
|
|
||
| let write_file = OpenOptions::new().write(true).open(&temp_file).unwrap(); | ||
| assert!(!is_appending(&write_file)); | ||
|
|
||
| let append_file = OpenOptions::new().append(true).open(&temp_file).unwrap(); | ||
| assert!(is_appending(&append_file)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_is_unsafe_overwrite() { | ||
| // Create two temp files one of which is empty | ||
| let empty = NamedTempFile::new().unwrap(); | ||
| let mut nonempty = NamedTempFile::new().unwrap(); | ||
| nonempty.write_all(b"anything").unwrap(); | ||
| nonempty.seek(SeekFrom::Start(0)).unwrap(); | ||
|
|
||
| // Using a different file as input and output does not result in an overwrite | ||
| assert!(!is_unsafe_overwrite(&empty, &nonempty)); | ||
|
|
||
| // Overwriting an empty file is always safe | ||
| assert!(!is_unsafe_overwrite(&empty, &empty)); | ||
|
|
||
| // Overwriting a nonempty file with itself is safe | ||
| assert!(!is_unsafe_overwrite(&nonempty, &nonempty)); | ||
|
|
||
| // Overwriting an empty file opened in append mode is safe | ||
| let empty_append = OpenOptions::new().append(true).open(&empty).unwrap(); | ||
| assert!(!is_unsafe_overwrite(&empty, &empty_append)); | ||
|
|
||
| // Overwriting a nonempty file opened in append mode is unsafe | ||
| let nonempty_append = OpenOptions::new().append(true).open(&nonempty).unwrap(); | ||
| assert!(is_unsafe_overwrite(&nonempty, &nonempty_append)); | ||
|
|
||
| // Overwriting a file opened in write mode is safe | ||
| let mut nonempty_write = OpenOptions::new().write(true).open(&nonempty).unwrap(); | ||
| assert!(!is_unsafe_overwrite(&nonempty, &nonempty_write)); | ||
|
|
||
| // Overwriting a file when the input and output file descriptors are pointing to | ||
| // different offsets is safe if the input offset is further than the output offset | ||
| nonempty_write.seek(SeekFrom::Start(1)).unwrap(); | ||
| assert!(!is_unsafe_overwrite(&nonempty_write, &nonempty)); | ||
| assert!(is_unsafe_overwrite(&nonempty, &nonempty_write)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // This file is part of the uutils coreutils package. | ||
| // | ||
| // For the full copyright and license information, please view the LICENSE | ||
| // file that was distributed with this source code. | ||
|
|
||
| use std::ffi::OsString; | ||
| use std::os::windows::ffi::OsStringExt; | ||
| use std::path::PathBuf; | ||
| use uucore::fs::FileInformation; | ||
| use winapi_util::AsHandleRef; | ||
| use windows_sys::Win32::Storage::FileSystem::{ | ||
| FILE_NAME_NORMALIZED, GetFinalPathNameByHandleW, VOLUME_NAME_NT, | ||
| }; | ||
|
|
||
| /// An unsafe overwrite occurs when the same file is used as both stdin and stdout | ||
| /// and the stdout file is not empty. | ||
| pub fn is_unsafe_overwrite<I: AsHandleRef, O: AsHandleRef>(input: &I, output: &O) -> bool { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function differs from the Unix version in that it does not attempt to determine when a file is safe to be overwritten, namely, when the input handle is pointing to an earlier or the same position of the file as the output handle. I also could not figure out an easy way of determining if the file handle is opened for append, like the |
||
| if !is_same_file_by_path(input, output) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the output file is empty | ||
| FileInformation::from_file(output) | ||
| .map(|info| info.file_size() > 0) | ||
| .unwrap_or(false) | ||
|
Comment on lines
+18
to
+25
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have liked to do the same thing as with Unix, but unfortunately, the The corresponding Unix code for a reference: let Ok(input_info) = FileInformation::from_file(input) else {
return false;
};
let Ok(output_info) = FileInformation::from_file(output) else {
return false;
};
if input_info != output_info || output_info.file_size() == 0 {
return false;
} |
||
| } | ||
|
|
||
| /// Get the file path for a file handle | ||
| fn get_file_path_from_handle<F: AsHandleRef>(file: &F) -> Option<PathBuf> { | ||
| let handle = file.as_raw(); | ||
| let mut path_buf = vec![0u16; 4096]; | ||
|
|
||
| // SAFETY: We should check how many bytes was written to `path_buf` | ||
| // and only read that many bytes from it. | ||
| let len = unsafe { | ||
| GetFinalPathNameByHandleW( | ||
| handle, | ||
| path_buf.as_mut_ptr(), | ||
| path_buf.len() as u32, | ||
| FILE_NAME_NORMALIZED | VOLUME_NAME_NT, | ||
| ) | ||
| }; | ||
| if len == 0 { | ||
| return None; | ||
| } | ||
| let path = OsString::from_wide(&path_buf[..len as usize]); | ||
| Some(PathBuf::from(path)) | ||
| } | ||
|
|
||
| /// Compare two file handles if they correspond to the same file | ||
| fn is_same_file_by_path<A: AsHandleRef, B: AsHandleRef>(a: &A, b: &B) -> bool { | ||
| match (get_file_path_from_handle(a), get_file_path_from_handle(b)) { | ||
| (Some(path1), Some(path2)) => path1 == path2, | ||
| _ => false, | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.