diff --git a/Cargo.lock b/Cargo.lock index a65cfbb6e7f..05ddacdf6f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4188,6 +4188,7 @@ dependencies = [ "notify", "rstest", "rustix", + "tempfile", "uucore", "windows-sys", ] diff --git a/src/uu/tail/Cargo.toml b/src/uu/tail/Cargo.toml index f1a1fdbf3ee..e4a17f89a73 100644 --- a/src/uu/tail/Cargo.toml +++ b/src/uu/tail/Cargo.toml @@ -46,6 +46,7 @@ windows-sys = { workspace = true, features = [ [dev-dependencies] rstest = { workspace = true } +tempfile = { workspace = true } [features] diagnostics = ["uucore/diagnostics"] diff --git a/src/uu/tail/src/chunks.rs b/src/uu/tail/src/chunks.rs index 27c9625f8da..5c6f99f3fb6 100644 --- a/src/uu/tail/src/chunks.rs +++ b/src/uu/tail/src/chunks.rs @@ -12,7 +12,7 @@ use std::collections::VecDeque; use std::fs::File; -use std::io::{BufRead, Read, Seek, SeekFrom, Write}; +use std::io::{self, BufRead, ErrorKind, Read, Seek, SeekFrom, Write}; use uucore::error::UResult; /// When reading files in reverse in `bounded_tail`, this is the size of each @@ -46,31 +46,27 @@ pub struct ReverseChunks<'a> { } impl<'a> ReverseChunks<'a> { - pub fn new(file: &'a mut File) -> Self { + pub fn new(file: &'a mut File) -> io::Result { let current = if cfg!(unix) { - file.stream_position().unwrap() + file.stream_position()? } else { 0 }; - let size = file.seek(SeekFrom::End(0)).unwrap() - current; + let size = file.seek(SeekFrom::End(0))? - current; let max_blocks_to_read = (size as f64 / BLOCK_SIZE as f64).ceil() as usize; let block_idx = 0; - ReverseChunks { + Ok(ReverseChunks { file, size, max_blocks_to_read, block_idx, - } + }) } -} -impl Iterator for ReverseChunks<'_> { - type Item = Vec; - - fn next(&mut self) -> Option { + fn next_chunk(&mut self) -> io::Result>> { // If there are no more chunks to read, terminate the iterator. if self.block_idx >= self.max_blocks_to_read { - return None; + return Ok(None); } // The chunk size is `BLOCK_SIZE` for all but the last chunk @@ -86,20 +82,34 @@ impl Iterator for ReverseChunks<'_> { // Seek backwards by the next chunk, read the full chunk into // `buf`, and then seek back to the start of the chunk again. let mut buf = vec![0; block_size as usize]; - let pos = self - .file - .seek(SeekFrom::Current(-(block_size as i64))) - .unwrap(); - self.file.read_exact(&mut buf).unwrap(); - let pos2 = self - .file - .seek(SeekFrom::Current(-(block_size as i64))) - .unwrap(); + let pos = self.file.seek(SeekFrom::Current(-(block_size as i64)))?; + match self.file.read_exact(&mut buf) { + Ok(()) => {} + // The file may have been truncated after its size was measured. + // Treat the remaining bytes as the start of the file instead of + // panicking; GNU tail exits successfully in the same situation. + // Rewind so the caller prints whatever is left of the file. + Err(err) if err.kind() == ErrorKind::UnexpectedEof => { + self.file.seek(SeekFrom::Start(0))?; + return Ok(None); + } + // Propagate real I/O errors (EIO, etc.) to the caller. + Err(err) => return Err(err), + } + let pos2 = self.file.seek(SeekFrom::Current(-(block_size as i64)))?; assert_eq!(pos, pos2); self.block_idx += 1; - Some(buf) + Ok(Some(buf)) + } +} + +impl Iterator for ReverseChunks<'_> { + type Item = io::Result>; + + fn next(&mut self) -> Option { + self.next_chunk().transpose() } } @@ -623,7 +633,43 @@ impl LinesChunkBuffer { #[cfg(test)] mod tests { - use crate::chunks::{BUFFER_SIZE, BytesChunk}; + use crate::chunks::{BLOCK_SIZE, BUFFER_SIZE, BytesChunk, ReverseChunks}; + use std::io::{Seek, Write}; + use tempfile::tempdir; + + // WASI has no temporary directory, so this file-backed case is skipped there. + #[cfg(not(target_os = "wasi"))] + #[test] + fn test_reverse_chunks_stops_gracefully_when_file_is_truncated() { + let dir = tempdir().unwrap(); + let path = dir.path().join("truncated"); + { + let mut file = std::fs::File::create(&path).unwrap(); + file.write_all(&vec![b'a'; BLOCK_SIZE as usize + 1]) + .unwrap(); + } + + { + let mut file = std::fs::File::open(&path).unwrap(); + let mut chunks = ReverseChunks::new(&mut file).unwrap(); + assert!(matches!(chunks.next(), Some(Ok(_)))); + + // Truncate through a second handle while the iterator still holds its + // previously measured size, then make sure the next read stops instead + // of panicking. + std::fs::OpenOptions::new() + .write(true) + .open(&path) + .unwrap() + .set_len(0) + .unwrap(); + assert!(chunks.next().is_none()); + + // The rewind must leave the file at the start so the caller can + // print whatever is left of the truncated file. + assert_eq!(file.stream_position().unwrap(), 0); + } + } #[test] fn test_bytes_chunk_from_when_offset_is_zero() { diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index ad794bfd392..6a6d96b9b11 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -406,16 +406,17 @@ fn forwards_thru_file( /// Iterate over bytes in the file, in reverse, until we find the /// `num_delimiters` instance of `delimiter`. The `file` is left seek'd to the /// position just after that delimiter. -fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) { +fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) -> UResult<()> { if num_delimiters == 0 { - file.seek(SeekFrom::End(0)).unwrap(); - return; + file.seek(SeekFrom::End(0))?; + return Ok(()); } // This variable counts the number of delimiters found in the file // so far (reading from the end of the file toward the beginning). let mut counter = 0; let mut first_slice = true; - for slice in ReverseChunks::new(file) { + for slice in ReverseChunks::new(file)? { + let slice = slice?; // Iterate over each byte in the slice in reverse order. let mut iter = memrchr_iter(delimiter, &slice); @@ -440,11 +441,12 @@ fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) { // cursor in the file is at the *beginning* of the // block, so seeking forward by `i + 1` bytes puts // us right after the found delimiter. - file.seek(SeekFrom::Current((i + 1) as i64)).unwrap(); - return; + file.seek(SeekFrom::Current((i + 1) as i64))?; + return Ok(()); } } } + Ok(()) } /// When tail'ing a file, we do not need to read the whole file from start to @@ -459,7 +461,7 @@ fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> { // Find the position in the file to start printing from. match &settings.mode { FilterMode::Lines(Signum::Negative(count), delimiter) => { - backwards_thru_file(file, *count, *delimiter); + backwards_thru_file(file, *count, *delimiter)?; } FilterMode::Lines(Signum::Positive(count), delimiter) if count > &1 => { let i = forwards_thru_file(file, *count - 1, *delimiter).unwrap();