diff --git a/src/uu/dd/src/bufferedoutput.rs b/src/uu/dd/src/bufferedoutput.rs index 7f4f92b8528..e0e5b371a56 100644 --- a/src/uu/dd/src/bufferedoutput.rs +++ b/src/uu/dd/src/bufferedoutput.rs @@ -74,6 +74,14 @@ impl<'a> BufferedOutput<'a> { // If `buf` does not include enough bytes to form a full block, // just buffer the whole thing and write zero blocks. let n = self.buf.len() + buf.len(); + if n < self.inner.settings.obs { + // Not even one complete block can be formed: buffer everything. + // Falling through would pass the pending bytes to the inner + // writer and emit them as a premature partial write (reachable + // once short reads end a fill early, see issue #13458). + self.buf.extend_from_slice(buf); + return Ok(WriteStat::default()); + } let rem = n % self.inner.settings.obs; let i = buf.len().saturating_sub(rem); let (to_write, to_buffer) = buf.split_at(i); @@ -144,6 +152,28 @@ mod tests { assert_eq!(output.buf, b"ab"); } + #[test] + fn test_buffered_output_write_blocks_lone_partial_accumulates() { + let settings = Settings { + obs: 6, + ..Default::default() + }; + let inner = Output { + dst: Dest::Sink, + settings: &settings, + }; + let mut output = BufferedOutput::new(inner).unwrap(); + // Two writes that together still do not fill one block must both + // be buffered; the second call must not flush the pending partial + // block prematurely. + output.write_blocks(b"ab").unwrap(); + let wstat = output.write_blocks(b"cd").unwrap(); + assert_eq!(wstat.writes_complete, 0); + assert_eq!(wstat.writes_partial, 0); + assert_eq!(wstat.bytes_total, 0); + assert_eq!(output.buf, b"abcd"); + } + #[test] fn test_buffered_output_write_blocks_complete() { let settings = Settings { diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 0599963e6e3..163d10c9423 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -515,7 +515,9 @@ impl Input<'_> { /// Fills a given buffer. /// Reads in increments of 'self.ibs'. - /// The start of each ibs-sized read follows the previous one. + /// The start of each ibs-sized read follows the previous one; a short + /// read ends the fill, so the bytes received so far form one partial + /// record for the copy loop (as in GNU dd). fn fill_consecutive(&mut self, buf: &mut Vec) -> io::Result { let mut reads_complete = 0; let mut reads_partial = 0; @@ -530,6 +532,12 @@ impl Input<'_> { rlen if rlen > 0 => { bytes_total += rlen; reads_partial += 1; + // A short read must end this fill: the next read would + // start at the following ibs-aligned chunk, leaving a + // gap of stale bytes inside `buf` that the `truncate` + // below would keep in the output while dropping the + // same number of real trailing bytes (issue #13458). + break; } _ => break, } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 849c47e227f..6ba3bb82572 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -1761,6 +1761,53 @@ fn test_reading_partial_blocks_from_fifo_unbuffered() { assert!(output.stderr.starts_with(expected)); } +/// Regression test for : +/// two deliberately short reads (ibs=3 sees only 2 bytes each) must be +/// gathered into a single obs=6 output block without pulling stale bytes +/// from the ibs-aligned gap into the output. +/// +/// The writer below runs `printf` inside `sh`, where it is a builtin, so this +/// needs no `printf` feature. +#[test] +#[cfg(all(unix, not(target_os = "macos"), not(target_os = "freebsd")))] +fn test_reading_partial_blocks_from_fifo_gathered_into_larger_obs() { + // Create the FIFO. + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.mkfifo("fifo"); + let fifoname = at.plus_as_string("fifo"); + + // Start a `dd` process that reads from the fifo (so it will wait + // until the writer process starts). + let mut reader_command = Command::new(get_tests_binary()); + let child = reader_command + .args(["dd", "ibs=3", "obs=6", &format!("if={fifoname}")]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .env("LC_ALL", "C") + .env("LANG", "C") + .env("LANGUAGE", "C") + .spawn() + .unwrap(); + + // Start different processes to write to the FIFO, with a small + // pause in between. + let mut writer_command = Command::new("sh"); + let _ = writer_command + .args([ + "-c", + &format!("(printf \"ab\"; sleep 0.1; printf \"cd\") > {fifoname}"), + ]) + .spawn() + .unwrap() + .wait(); + + let output = child.wait_with_output().unwrap(); + assert_eq!(output.stdout, b"abcd"); + let expected = b"0+2 records in\n0+1 records out\n4 bytes copied"; + assert!(output.stderr.starts_with(expected)); +} + #[test] #[cfg(any(target_os = "linux", target_os = "android"))] fn test_iflag_directory_fails_when_file_is_passed_via_std_in() {