From 6f5dc1f6134c36a5f114215ceb9a02b36682d425 Mon Sep 17 00:00:00 2001 From: mavrodikomsd-eng Date: Mon, 26 Oct 2026 20:33:13 +0300 Subject: [PATCH 1/3] dd: keep unblock state across reads so records are not split at read boundaries unblock() was stateless and ran buf.chunks(cbs) on each read buffer separately, so every buffer was assumed to start at a record boundary: a read ending partway through a record turned the leftover into a short line and cut every later record in that buffer at the wrong column, for example with cbs=80 and the default 512-byte block, or with short reads from a pipe. The Converter now carries the partial record between calls, like the conv=block path already does, and pads the final incomplete record at EOF the way GNU dd does. Fixes #15818 --- src/uu/dd/src/blocks.rs | 141 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 131 insertions(+), 10 deletions(-) diff --git a/src/uu/dd/src/blocks.rs b/src/uu/dd/src/blocks.rs index 7137c89251b..2dafb5b8270 100644 --- a/src/uu/dd/src/blocks.rs +++ b/src/uu/dd/src/blocks.rs @@ -47,6 +47,9 @@ pub(crate) struct Converter { truncated: bool, /// The blocked output not yet written. out: Vec, + /// The partial record left over from the previous read for `conv=unblock`, + /// at most `cbs - 1` bytes. + record: Vec, } impl Converter { @@ -56,6 +59,7 @@ impl Converter { record_len: 0, truncated: false, out: Vec::new(), + record: Vec::new(), } } @@ -77,12 +81,12 @@ impl Converter { ConversionMode::ConvertThenBlock(ct, cbs) => { self.block(&apply_conversion(buf, ct), cbs, None, rstat, write) } - ConversionMode::UnblockOnly(cbs) => write(&unblock(buf, cbs)), + ConversionMode::UnblockOnly(cbs) => self.unblock(buf, cbs, None, write), ConversionMode::UnblockThenConvert(ct, cbs) => { - write(&apply_conversion(&unblock(buf, cbs), ct)) + self.unblock(buf, cbs, Some(ct), write) } ConversionMode::ConvertThenUnblock(ct, cbs) => { - write(&unblock(&apply_conversion(buf, ct), cbs)) + self.unblock(&apply_conversion(buf, ct), cbs, None, write) } } } @@ -92,16 +96,80 @@ impl Converter { &mut self, write: &mut impl FnMut(&[u8]) -> io::Result<()>, ) -> io::Result<()> { - let (cbs, ct) = match self.mode { + match self.mode { ConversionMode::BlockOnly(cbs) | ConversionMode::ConvertThenBlock(_, cbs) => { - (cbs, None) + if self.record_len > 0 { + self.end_record(cbs, None, write)?; + } + } + ConversionMode::BlockThenConvert(ct, cbs) => { + if self.record_len > 0 { + self.end_record(cbs, Some(ct), write)?; + } + } + ConversionMode::UnblockOnly(cbs) => self.finish_unblock(cbs, None, write)?, + ConversionMode::UnblockThenConvert(ct, cbs) => { + self.finish_unblock(cbs, Some(ct), write)? + } + // The conversion is already applied before unblocking here. + ConversionMode::ConvertThenUnblock(_, cbs) => { + self.finish_unblock(cbs, None, write)? + } + ConversionMode::ConvertOnly(_) => return Ok(()), + } + self.flush(write) + } + + /// Emit the trailing partial record if the input did not end on a record boundary. + /// + /// GNU treats the record as if padded with spaces to `cbs`, which [`unblock`] + /// then trims away. + fn finish_unblock( + &mut self, + cbs: usize, + ct: Option<&ConversionTable>, + write: &mut impl FnMut(&[u8]) -> io::Result<()>, + ) -> io::Result<()> { + if !self.record.is_empty() { + let mut record = std::mem::take(&mut self.record); + record.resize(cbs, SPACE); + self.emit(&unblock(&record, cbs), ct); + } + self.flush(write) + } + + /// Append output to the buffer, applying the conversion table if present. + fn emit(&mut self, data: &[u8], ct: Option<&ConversionTable>) { + match ct { + Some(ct) => self.out.extend(data.iter().map(|&b| ct[b as usize])), + None => self.out.extend_from_slice(data), + } + } + + /// Split `buf` into `cbs`-length records, trimming trailing spaces from each. + /// + /// A record may span several reads: the partial record left over from the + /// previous call is finished first, and the new trailing partial record is + /// kept for the next one. + fn unblock( + &mut self, + mut buf: &[u8], + cbs: usize, + ct: Option<&ConversionTable>, + write: &mut impl FnMut(&[u8]) -> io::Result<()>, + ) -> io::Result<()> { + if !self.record.is_empty() { + let take = (cbs - self.record.len()).min(buf.len()); + self.record.extend_from_slice(&buf[..take]); + buf = &buf[take..]; + if self.record.len() == cbs { + let record = std::mem::take(&mut self.record); + self.emit(&unblock(&record, cbs), ct); } - ConversionMode::BlockThenConvert(ct, cbs) => (cbs, Some(ct)), - _ => return Ok(()), - }; - if self.record_len > 0 { - self.end_record(cbs, ct, write)?; } + let full = buf.len() - buf.len() % cbs; + self.emit(&unblock(&buf[..full], cbs), ct); + self.record.extend_from_slice(&buf[full..]); self.flush(write) } @@ -469,4 +537,57 @@ mod tests { assert_eq!(res, exp); } + + /// Unblock each slice of `reads` in turn and collect the whole output. + fn unblock_reads(reads: &[&[u8]], cbs: usize) -> Vec { + let mode = ConversionMode::UnblockOnly(cbs); + let mut converter = Converter::new(mode); + let mut out = Vec::new(); + let mut write = |data: &[u8]| -> io::Result<()> { + out.extend_from_slice(data); + Ok(()) + }; + for buf in reads { + converter.convert(buf, &mut ReadStat::default(), &mut write).unwrap(); + } + converter.finish(&mut write).unwrap(); + out + } + + #[test] + fn unblock_test_record_across_reads() { + // Each read ends halfway through a record; the records must still + // come out aligned instead of being cut at the read boundary. + let res = unblock_reads(&[b"abcde", b"fghij"], 4); + + assert_eq!(res, b"abcd\nefgh\nij\n".to_vec()); + } + + #[test] + fn unblock_test_exact_records_no_extra_newline() { + let res = unblock_reads(&[b"abc", b"def"], 3); + + assert_eq!(res, b"abc\ndef\n".to_vec()); + } + + #[test] + fn unblock_test_eof_partial_record() { + let res = unblock_reads(&[b"abcdefg"], 4); + + assert_eq!(res, b"abcd\nefg\n".to_vec()); + } + + #[test] + fn unblock_test_spaces_trimmed_across_reads() { + let res = unblock_reads(&[b"ab ", b"cd "], 4); + + assert_eq!(res, b"ab\ncd\n".to_vec()); + } + + #[test] + fn unblock_test_all_space_record() { + let res = unblock_reads(&[b" "], 8); + + assert_eq!(res, b"\n".to_vec()); + } } From 7acbb31d03466a2cc9aa16b07b68e989c7193c19 Mon Sep 17 00:00:00 2001 From: mavrodikomsd-eng Date: Tue, 27 Oct 2026 13:08:25 +0300 Subject: [PATCH 2/3] Format blocks.rs with rustfmt --- src/uu/dd/src/blocks.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/uu/dd/src/blocks.rs b/src/uu/dd/src/blocks.rs index 2dafb5b8270..6b9102148f7 100644 --- a/src/uu/dd/src/blocks.rs +++ b/src/uu/dd/src/blocks.rs @@ -82,9 +82,7 @@ impl Converter { self.block(&apply_conversion(buf, ct), cbs, None, rstat, write) } ConversionMode::UnblockOnly(cbs) => self.unblock(buf, cbs, None, write), - ConversionMode::UnblockThenConvert(ct, cbs) => { - self.unblock(buf, cbs, Some(ct), write) - } + ConversionMode::UnblockThenConvert(ct, cbs) => self.unblock(buf, cbs, Some(ct), write), ConversionMode::ConvertThenUnblock(ct, cbs) => { self.unblock(&apply_conversion(buf, ct), cbs, None, write) } @@ -112,9 +110,7 @@ impl Converter { self.finish_unblock(cbs, Some(ct), write)? } // The conversion is already applied before unblocking here. - ConversionMode::ConvertThenUnblock(_, cbs) => { - self.finish_unblock(cbs, None, write)? - } + ConversionMode::ConvertThenUnblock(_, cbs) => self.finish_unblock(cbs, None, write)?, ConversionMode::ConvertOnly(_) => return Ok(()), } self.flush(write) @@ -548,7 +544,9 @@ mod tests { Ok(()) }; for buf in reads { - converter.convert(buf, &mut ReadStat::default(), &mut write).unwrap(); + converter + .convert(buf, &mut ReadStat::default(), &mut write) + .unwrap(); } converter.finish(&mut write).unwrap(); out From 5310e78ee8f3d32f05d3c2eaaed47d5cd1cc97c8 Mon Sep 17 00:00:00 2001 From: mavrodikomsd-eng Date: Tue, 27 Oct 2026 19:03:28 +0300 Subject: [PATCH 3/3] Merge identical finish() match arms and add missing semicolons (clippy) --- src/uu/dd/src/blocks.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uu/dd/src/blocks.rs b/src/uu/dd/src/blocks.rs index 6b9102148f7..638582010a5 100644 --- a/src/uu/dd/src/blocks.rs +++ b/src/uu/dd/src/blocks.rs @@ -105,12 +105,13 @@ impl Converter { self.end_record(cbs, Some(ct), write)?; } } - ConversionMode::UnblockOnly(cbs) => self.finish_unblock(cbs, None, write)?, + ConversionMode::UnblockOnly(cbs) | ConversionMode::ConvertThenUnblock(_, cbs) => { + // For ConvertThenUnblock the conversion is applied before unblocking here. + self.finish_unblock(cbs, None, write)?; + } ConversionMode::UnblockThenConvert(ct, cbs) => { - self.finish_unblock(cbs, Some(ct), write)? + self.finish_unblock(cbs, Some(ct), write)?; } - // The conversion is already applied before unblocking here. - ConversionMode::ConvertThenUnblock(_, cbs) => self.finish_unblock(cbs, None, write)?, ConversionMode::ConvertOnly(_) => return Ok(()), } self.flush(write)