dd: keep short reads from corrupting re-blocked output

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 {

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<u8>) -> io::Result<ReadStat> {
         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,
             }

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 <https://github.com/uutils/coreutils/issues/13458>:
+/// 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() {