From 92c3f6044064fb1797f9c981a8f21fa0d8e9579e Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Thu, 18 Aug 2022 22:04:57 +0200 Subject: [PATCH 1/4] tail: fix stdin redirect (#3842) This fixes a bug where calling `tail - < file.txt` would result in invoking `unbounded_tail()`. However, it is a stdin redirect to a seekable regular file and therefore `bounded_tail` should be invoked as if `tail file.txt` had been called. --- src/uu/tail/src/tail.rs | 8 ++++---- tests/by-util/test_tail.rs | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 6f169d3f4..a20c1db09 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -436,9 +436,9 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { let metadata = path.metadata().ok(); - if display_name.is_stdin() && path_is_tailable { + if display_name.is_stdin() && path_is_tailable && !path.is_file() { if settings.verbose { - files.print_header(Path::new(text::STDIN_HEADER), !first_header); + files.print_header(&display_name, !first_header); first_header = false; } @@ -452,7 +452,7 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { PathData { reader: Some(Box::new(reader)), metadata: None, - display_name: PathBuf::from(text::STDIN_HEADER), + display_name, }, true, ); @@ -476,7 +476,7 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { match File::open(&path) { Ok(mut file) => { if settings.verbose { - files.print_header(&path, !first_header); + files.print_header(&display_name, !first_header); first_header = false; } let mut reader; diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 6afd55f51..54cc7bbf1 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -76,6 +76,7 @@ fn test_stdin_redirect_file() { .set_stdin(std::fs::File::open(at.plus("f")).unwrap()) .arg("-v") .run() + .no_stderr() .stdout_is("==> standard input <==\nfoo") .succeeded(); From 942928b0ea466f3a2bcaa79fa539a1b2ebabb3bc Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 29 Aug 2022 01:41:06 +0200 Subject: [PATCH 2/4] tail: fix stdin redirect when file is not at its beginning Previously, if stdin redirect pointed to a regular file, tailing started at the beginning of the file. However, tailing needs to start at the current position because this is expected by tests/tail-2/start-middle.sh. This fixes the issue by taking the current offset into account while going backwards through the stdin redirected file. --- src/uu/tail/src/chunks.rs | 3 ++- src/uu/tail/src/tail.rs | 48 ++++++++++++++++++++++++++++++-------- tests/by-util/test_tail.rs | 20 ++++++++++++++++ 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/uu/tail/src/chunks.rs b/src/uu/tail/src/chunks.rs index 270493093..c5fb73082 100644 --- a/src/uu/tail/src/chunks.rs +++ b/src/uu/tail/src/chunks.rs @@ -30,7 +30,8 @@ pub struct ReverseChunks<'a> { impl<'a> ReverseChunks<'a> { pub fn new(file: &'a mut File) -> ReverseChunks<'a> { - let size = file.seek(SeekFrom::End(0)).unwrap(); + let current = file.seek(SeekFrom::Current(0)).unwrap(); + let size = file.seek(SeekFrom::End(0)).unwrap() - current; let max_blocks_to_read = (size as f64 / BLOCK_SIZE as f64).ceil() as usize; let block_idx = 0; ReverseChunks { diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index a20c1db09..fd14d5d3b 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -128,6 +128,8 @@ pub struct Settings { use_polling: bool, verbose: bool, stdin_is_pipe_or_fifo: bool, + stdin_offset: u64, + stdin_redirect: PathBuf, } impl Settings { @@ -323,6 +325,15 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { settings.paths.push_front(dash); } + if cfg!(unix) && settings.stdin_is_pipe_or_fifo { + settings.stdin_redirect = PathBuf::from(text::STDIN_HEADER).handle_redirect(); + use std::os::unix::io::FromRawFd; + let mut stdin_handle = unsafe { std::fs::File::from_raw_fd(0) }; + if let Ok(offset) = stdin_handle.seek(SeekFrom::Current(0)) { + settings.stdin_offset = offset; + } + } + // TODO: is there a better way to check for a readable stdin? let mut buf = [0; 0]; // empty buffer to check if stdin().read().is_err() let stdin_read_possible = settings.stdin_is_pipe_or_fifo && stdin().read(&mut buf).is_ok(); @@ -341,7 +352,11 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { // Iterate user provided `paths` and add them to Watcher. if let Some((ref mut watcher, _)) = watcher_rx { for path in &settings.paths { - let mut path = path.handle_redirect(); + let mut path = if path.is_stdin() { + settings.stdin_redirect.to_owned() + } else { + path.to_owned() + }; if path.is_stdin() { continue; } @@ -376,7 +391,11 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { } else { path.to_owned() }; - let mut path = path.handle_redirect(); + let path = if path.is_stdin() { + settings.stdin_redirect.to_owned() + } else { + path.to_owned() + }; let path_is_tailable = path.is_tailable(); if !path.is_stdin() && !path_is_tailable { @@ -479,9 +498,14 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { files.print_header(&display_name, !first_header); first_header = false; } - let mut reader; - if file.is_seekable() && metadata.as_ref().unwrap().get_block_size() > 0 { + let mut reader; + if file.is_seekable(if display_name.is_stdin() { + settings.stdin_offset + } else { + 0 + }) && metadata.as_ref().unwrap().get_block_size() > 0 + { bounded_tail(&mut file, &settings); reader = BufReader::new(file); } else { @@ -513,9 +537,11 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { } } } else if settings.retry && settings.follow.is_some() { - if path.is_relative() { - path = std::env::current_dir()?.join(&path); - } + let path = if path.is_relative() { + std::env::current_dir()?.join(&path) + } else { + path.to_owned() + }; // Insert non-is_tailable() paths into `files.map` files.insert( &path, @@ -1544,14 +1570,16 @@ pub fn stdin_is_bad_fd() -> bool { trait FileExtTail { #[allow(clippy::wrong_self_convention)] - fn is_seekable(&mut self) -> bool; + fn is_seekable(&mut self, current_offset: u64) -> bool; } impl FileExtTail for File { - fn is_seekable(&mut self) -> bool { + /// Test if File is seekable. + /// Set the current position offset to `current_offset`. + fn is_seekable(&mut self, current_offset: u64) -> bool { self.seek(SeekFrom::Current(0)).is_ok() && self.seek(SeekFrom::End(0)).is_ok() - && self.seek(SeekFrom::Start(0)).is_ok() + && self.seek(SeekFrom::Start(current_offset)).is_ok() } } diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 54cc7bbf1..e9797b7b4 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -94,6 +94,26 @@ fn test_stdin_redirect_file() { assert!(buf_stderr.is_empty()); } +#[test] +#[cfg(all(unix, not(any(target_os = "android", target_vendor = "apple"))))] // FIXME: make this work not just on Linux +fn test_stdin_redirect_offset() { + // inspired by: "gnu/tests/tail-2/start-middle.sh" + use std::io::{Seek, SeekFrom}; + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.write("k", "1\n2\n"); + let mut fh = std::fs::File::open(at.plus("k")).unwrap(); + fh.seek(SeekFrom::Start(2)).unwrap(); + + ts.ucmd() + .set_stdin(fh) + .run() + .stdout_is("2\n") + .succeeded(); +} + #[test] fn test_nc_0_wo_follow() { // verify that -[nc]0 without -f, exit without reading From 74f359bd76d4085ebe5b0a30c70c0c49a80dd99b Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Tue, 30 Aug 2022 21:42:14 +0200 Subject: [PATCH 3/4] tail: use same-file crate to get a handle of stdin redirected file --- Cargo.lock | 1 + src/uu/tail/Cargo.toml | 1 + src/uu/tail/src/chunks.rs | 6 +++++- src/uu/tail/src/tail.rs | 30 +++++++++++++++--------------- tests/by-util/test_tail.rs | 6 +----- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 394b9b8cd..5139bd3dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2816,6 +2816,7 @@ dependencies = [ "libc", "nix", "notify", + "same-file", "uucore", "winapi", "winapi-util", diff --git a/src/uu/tail/Cargo.toml b/src/uu/tail/Cargo.toml index 98ab9314d..8e12beafa 100644 --- a/src/uu/tail/Cargo.toml +++ b/src/uu/tail/Cargo.toml @@ -20,6 +20,7 @@ clap = { version = "3.2", features = ["wrap_help", "cargo"] } libc = "0.2.132" notify = { version = "=5.0.0-pre.16", features=["macos_kqueue"]} uucore = { version=">=0.0.15", package="uucore", path="../../uucore", features=["ringbuffer", "lines"] } +same-file = "1.0.6" [target.'cfg(windows)'.dependencies] winapi = { version="0.3", features=["fileapi", "handleapi", "processthreadsapi", "synchapi", "winbase"] } diff --git a/src/uu/tail/src/chunks.rs b/src/uu/tail/src/chunks.rs index c5fb73082..0ba64540a 100644 --- a/src/uu/tail/src/chunks.rs +++ b/src/uu/tail/src/chunks.rs @@ -30,7 +30,11 @@ pub struct ReverseChunks<'a> { impl<'a> ReverseChunks<'a> { pub fn new(file: &'a mut File) -> ReverseChunks<'a> { - let current = file.seek(SeekFrom::Current(0)).unwrap(); + let current = if cfg!(unix) { + file.seek(SeekFrom::Current(0)).unwrap() + } else { + 0 + }; let size = file.seek(SeekFrom::End(0)).unwrap() - current; let max_blocks_to_read = (size as f64 / BLOCK_SIZE as f64).ceil() as usize; let block_idx = 0; diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index fd14d5d3b..717eac6d8 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -318,6 +318,18 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { )); } + settings.stdin_redirect = dash.handle_redirect(); + if cfg!(unix) && settings.stdin_is_pipe_or_fifo { + // Save the current seek position/offset of a stdin redirected file. + // This is needed to pass "gnu/tests/tail-2/start-middle.sh" + use same_file::Handle; + if let Ok(mut stdin_handle) = Handle::stdin() { + if let Ok(offset) = stdin_handle.as_file_mut().seek(SeekFrom::Current(0)) { + settings.stdin_offset = offset; + } + } + } + // add '-' to paths if !settings.paths.contains(&dash) && settings.stdin_is_pipe_or_fifo || settings.paths.is_empty() && !settings.stdin_is_pipe_or_fifo @@ -325,15 +337,6 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { settings.paths.push_front(dash); } - if cfg!(unix) && settings.stdin_is_pipe_or_fifo { - settings.stdin_redirect = PathBuf::from(text::STDIN_HEADER).handle_redirect(); - use std::os::unix::io::FromRawFd; - let mut stdin_handle = unsafe { std::fs::File::from_raw_fd(0) }; - if let Ok(offset) = stdin_handle.seek(SeekFrom::Current(0)) { - settings.stdin_offset = offset; - } - } - // TODO: is there a better way to check for a readable stdin? let mut buf = [0; 0]; // empty buffer to check if stdin().read().is_err() let stdin_read_possible = settings.stdin_is_pipe_or_fifo && stdin().read(&mut buf).is_ok(); @@ -455,7 +458,7 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { let metadata = path.metadata().ok(); - if display_name.is_stdin() && path_is_tailable && !path.is_file() { + if display_name.is_stdin() && !path.is_file() { if settings.verbose { files.print_header(&display_name, !first_header); first_header = false; @@ -500,11 +503,8 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { } let mut reader; - if file.is_seekable(if display_name.is_stdin() { - settings.stdin_offset - } else { - 0 - }) && metadata.as_ref().unwrap().get_block_size() > 0 + if file.is_seekable(settings.stdin_offset) + && metadata.as_ref().unwrap().get_block_size() > 0 { bounded_tail(&mut file, &settings); reader = BufReader::new(file); diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index e9797b7b4..16bdf77ee 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -107,11 +107,7 @@ fn test_stdin_redirect_offset() { let mut fh = std::fs::File::open(at.plus("k")).unwrap(); fh.seek(SeekFrom::Start(2)).unwrap(); - ts.ucmd() - .set_stdin(fh) - .run() - .stdout_is("2\n") - .succeeded(); + ts.ucmd().set_stdin(fh).run().stdout_is("2\n").succeeded(); } #[test] From 88d3aee71cb9f2138f5a683699507be29286012f Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 31 Aug 2022 15:47:01 +0200 Subject: [PATCH 4/4] tail: fix offset for stdin redirect if multiple input files --- src/uu/tail/src/tail.rs | 7 +++++-- tests/by-util/test_tail.rs | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 717eac6d8..28a65093d 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -503,8 +503,11 @@ fn uu_tail(mut settings: Settings) -> UResult<()> { } let mut reader; - if file.is_seekable(settings.stdin_offset) - && metadata.as_ref().unwrap().get_block_size() > 0 + if file.is_seekable(if display_name.is_stdin() { + settings.stdin_offset + } else { + 0 + }) && metadata.as_ref().unwrap().get_block_size() > 0 { bounded_tail(&mut file, &settings); reader = BufReader::new(file); diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 16bdf77ee..1a48cebfe 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -107,7 +107,38 @@ fn test_stdin_redirect_offset() { let mut fh = std::fs::File::open(at.plus("k")).unwrap(); fh.seek(SeekFrom::Start(2)).unwrap(); - ts.ucmd().set_stdin(fh).run().stdout_is("2\n").succeeded(); + ts.ucmd() + .set_stdin(fh) + .run() + .no_stderr() + .stdout_is("2\n") + .succeeded(); +} + +#[test] +#[cfg(all(unix, not(any(target_os = "android", target_vendor = "apple"))))] // FIXME: make this work not just on Linux +fn test_stdin_redirect_offset2() { + // like test_stdin_redirect_offset but with multiple files + use std::io::{Seek, SeekFrom}; + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.write("k", "1\n2\n"); + at.write("l", "3\n4\n"); + at.write("m", "5\n6\n"); + let mut fh = std::fs::File::open(at.plus("k")).unwrap(); + fh.seek(SeekFrom::Start(2)).unwrap(); + + ts.ucmd() + .set_stdin(fh) + .args(&["k", "-", "l", "m"]) + .run() + .no_stderr() + .stdout_is( + "==> k <==\n1\n2\n\n==> standard input <==\n2\n\n==> l <==\n3\n4\n\n==> m <==\n5\n6\n", + ) + .succeeded(); } #[test]