Skip to content

Commit ad0b20d

Browse files
committed
head: Fix bug printing large non-seekable files
Fixes issue #7288. Rewrite logic for print-all-but-last-n-bytes in non-seekable files.
1 parent ee0d178 commit ad0b20d

File tree

2 files changed

+54
-44
lines changed

2 files changed

+54
-44
lines changed

src/uu/head/src/head.rs

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
use clap::{crate_version, Arg, ArgAction, ArgMatches, Command};
99
use std::ffi::OsString;
10-
use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write};
10+
use std::io::{self, Read, Seek, SeekFrom, Write};
1111
use std::num::TryFromIntError;
1212
use thiserror::Error;
1313
use uucore::display::Quotable;
@@ -239,10 +239,7 @@ impl HeadOptions {
239239
}
240240
}
241241

242-
fn read_n_bytes<R>(input: R, n: u64) -> std::io::Result<()>
243-
where
244-
R: Read,
245-
{
242+
fn read_n_bytes(input: impl Read, n: u64) -> std::io::Result<()> {
246243
// Read the first `n` bytes from the `input` reader.
247244
let mut reader = input.take(n);
248245

@@ -287,48 +284,18 @@ fn catch_too_large_numbers_in_backwards_bytes_or_lines(n: u64) -> Option<usize>
287284
}
288285
}
289286

290-
/// Print to stdout all but the last `n` bytes from the given reader.
291-
fn read_but_last_n_bytes(input: &mut impl std::io::BufRead, n: u64) -> std::io::Result<()> {
292-
if n == 0 {
293-
//prints everything
294-
return read_n_bytes(input, u64::MAX);
295-
}
296-
287+
fn read_but_last_n_bytes(input: impl std::io::BufRead, n: u64) -> std::io::Result<()> {
297288
if let Some(n) = catch_too_large_numbers_in_backwards_bytes_or_lines(n) {
298289
let stdout = std::io::stdout();
299290
let mut stdout = stdout.lock();
300-
301-
let mut ring_buffer = Vec::new();
302-
303-
let mut buffer = [0u8; BUF_SIZE];
304-
let mut total_read = 0;
305-
306-
loop {
307-
let read = match input.read(&mut buffer) {
308-
Ok(0) => break,
309-
Ok(read) => read,
310-
Err(e) => match e.kind() {
311-
ErrorKind::Interrupted => continue,
312-
_ => return Err(e),
313-
},
314-
};
315-
316-
total_read += read;
317-
318-
if total_read <= n {
319-
// Fill the ring buffer without exceeding n bytes
320-
let overflow = n - total_read;
321-
ring_buffer.extend_from_slice(&buffer[..read - overflow]);
322-
} else {
323-
// Write the ring buffer and the part of the buffer that exceeds n
324-
stdout.write_all(&ring_buffer)?;
325-
stdout.write_all(&buffer[..read - n + ring_buffer.len()])?;
326-
ring_buffer.clear();
327-
ring_buffer.extend_from_slice(&buffer[read - n + ring_buffer.len()..read]);
328-
}
291+
for bytes in take_all_but(input.bytes(), n) {
292+
stdout.write_all(&[bytes?])?;
329293
}
294+
// Make sure we finish writing everything to the target before
295+
// exiting. Otherwise, when Rust is implicitly flushing, any
296+
// error will be silently ignored.
297+
stdout.flush()?;
330298
}
331-
332299
Ok(())
333300
}
334301

@@ -343,6 +310,10 @@ fn read_but_last_n_lines(
343310
for bytes in take_all_but(lines(input, separator), n) {
344311
stdout.write_all(&bytes?)?;
345312
}
313+
// Make sure we finish writing everything to the target before
314+
// exiting. Otherwise, when Rust is implicitly flushing, any
315+
// error will be silently ignored.
316+
stdout.flush()?;
346317
}
347318
Ok(())
348319
}
@@ -440,8 +411,7 @@ fn head_backwards_without_seek_file(
440411
input: &mut std::fs::File,
441412
options: &HeadOptions,
442413
) -> std::io::Result<()> {
443-
let reader = &mut std::io::BufReader::with_capacity(BUF_SIZE, &*input);
444-
414+
let reader = std::io::BufReader::with_capacity(BUF_SIZE, &*input);
445415
match options.mode {
446416
Mode::AllButLastBytes(n) => read_but_last_n_bytes(reader, n)?,
447417
Mode::AllButLastLines(n) => read_but_last_n_lines(reader, n, options.line_ending.into())?,

tests/by-util/test_head.rs

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

66
// spell-checker:ignore (words) bogusfile emptyfile abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstu
7+
// spell-checker:ignore (words) seekable
78

89
use crate::common::util::TestScenario;
910

@@ -392,6 +393,45 @@ fn test_presume_input_pipe_5_chars() {
392393
.stdout_is_fixture("lorem_ipsum_5_chars.expected");
393394
}
394395

396+
#[test]
397+
fn test_all_but_last_bytes_large_file_piped() {
398+
// Validate print-all-but-last-n-bytes with a large piped-in (i.e. non-seekable) file.
399+
let scene = TestScenario::new(util_name!());
400+
let fixtures = &scene.fixtures;
401+
402+
// First, create all our fixtures.
403+
let seq_30000_file_name = "seq_30000";
404+
let seq_29000_file_name = "seq_29000";
405+
let seq_29001_30000_file_name = "seq_29001_30000";
406+
scene
407+
.cmd("seq")
408+
.arg("30000")
409+
.set_stdout(fixtures.make_file(seq_30000_file_name))
410+
.succeeds();
411+
scene
412+
.cmd("seq")
413+
.arg("29000")
414+
.set_stdout(fixtures.make_file(seq_29000_file_name))
415+
.succeeds();
416+
scene
417+
.cmd("seq")
418+
.args(&["29001", "30000"])
419+
.set_stdout(fixtures.make_file(seq_29001_30000_file_name))
420+
.succeeds();
421+
422+
let seq_29001_30000_file_length = fixtures
423+
.open(seq_29001_30000_file_name)
424+
.metadata()
425+
.unwrap()
426+
.len();
427+
scene
428+
.ucmd()
429+
.args(&["-c", &format!("-{}", seq_29001_30000_file_length)])
430+
.pipe_in_fixture(seq_30000_file_name)
431+
.succeeds()
432+
.stdout_only_fixture(seq_29000_file_name);
433+
}
434+
395435
#[test]
396436
fn test_read_backwards_lines_large_file() {
397437
// Create our fixtures on the fly. We need the input file to be at least double

0 commit comments

Comments
 (0)