From 325f7728fd6e6ac10f0dab236fff281936a9d6e9 Mon Sep 17 00:00:00 2001 From: mavrodikomsd-eng Date: Mon, 26 Oct 2026 20:15:37 +0300 Subject: [PATCH] split: retry short writes in custom_write so the input tail is not silently dropped A single write() may accept only part of the data when the output volume fills up or a file size limit is reached. ByteChunkWriter then broke out of its loop on the short count and LineChunkWriter ignored the count entirely, so the last chunk could end up truncated with exit code 0. GNU split reports the write error instead. custom_write now keeps writing until all bytes are accepted; the next write() call surfaces the underlying error. BrokenPipe handling for --filter is unchanged. Fixes #15820 --- src/uu/split/src/split.rs | 121 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 116 insertions(+), 5 deletions(-) diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index d683bb34328..149004f0a46 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -399,12 +399,40 @@ fn ignorable_io_error(error: &io::Error, settings: &Settings) -> bool { /// If ignorable io error occurs, return number of bytes as if all bytes written /// Should not be used for Kth chunk number sub-strategies /// as those do not work with `--filter` option -fn custom_write(bytes: &[u8], writer: &mut T, settings: &Settings) -> io::Result { - match writer.write(bytes) { - Ok(n) => Ok(n), - Err(e) if ignorable_io_error(&e, settings) => Ok(bytes.len()), - Err(e) => Err(e), +/// A single `write()` may accept only part of the data, e.g. when the output +/// volume fills up or a file size limit is reached. Keep writing until +/// everything is written so the tail of the input is never silently dropped; +/// the next `write()` call then reports the underlying error. +/// With `ignore_broken_pipe`, a `BrokenPipe` error is reported as if all bytes +/// were written (for `--filter`, whose child may exit early). +fn write_repeating( + mut bytes: &[u8], + writer: &mut T, + ignore_broken_pipe: bool, +) -> io::Result { + let full_len = bytes.len(); + let mut total = 0; + while !bytes.is_empty() { + match writer.write(bytes) { + Ok(0) => { + // No progress is possible; report an error instead of looping forever. + return Err(io::Error::new(ErrorKind::WriteZero, "write failed")); + } + Ok(n) => { + total += n; + bytes = &bytes[n..]; + } + Err(e) if ignore_broken_pipe && e.kind() == ErrorKind::BrokenPipe => { + return Ok(full_len); + } + Err(e) => return Err(e), + } } + Ok(total) +} + +fn custom_write(bytes: &[u8], writer: &mut T, settings: &Settings) -> io::Result { + write_repeating(bytes, writer, settings.filter.is_some()) } /// Custom wrapper for `write_all()` method @@ -1497,3 +1525,86 @@ fn print_creating_file(name: &OsString) -> io::Result<()> { translate!("split-creating-file", "file" => name.quote()) ) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Accepts at most `cap` bytes in total; every `write()` takes what it can + /// and once the cap is hit reports the device as full. Mimics a volume + /// filling up in the middle of a write. + struct FillingWriter { + cap: usize, + } + + impl Write for FillingWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + if self.cap == 0 { + return Err(io::Error::from(ErrorKind::StorageFull)); + } + let n = buf.len().min(self.cap); + self.cap -= n; + Ok(n) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + struct BrokenPipeWriter; + + impl Write for BrokenPipeWriter { + fn write(&mut self, _: &[u8]) -> io::Result { + Err(io::Error::from(ErrorKind::BrokenPipe)) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + struct ZeroWriter; + + impl Write for ZeroWriter { + fn write(&mut self, _: &[u8]) -> io::Result { + Ok(0) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + #[test] + fn test_write_repeating_finishes_short_writes() { + let mut w = FillingWriter { cap: 10 }; + assert_eq!(write_repeating(b"abc", &mut w, false).unwrap(), 3); + } + + #[test] + fn test_write_repeating_errors_when_device_fills_up() { + let mut w = FillingWriter { cap: 5 }; + // The old single-write implementation returned Ok(5) here, silently + // dropping the tail of the input. + assert!(write_repeating(&[0u8; 10], &mut w, false).is_err()); + } + + #[test] + fn test_write_repeating_ignores_broken_pipe_when_asked() { + let mut w = BrokenPipeWriter; + assert_eq!(write_repeating(b"abc", &mut w, true).unwrap(), 3); + } + + #[test] + fn test_write_repeating_propagates_broken_pipe_by_default() { + let mut w = BrokenPipeWriter; + assert!(write_repeating(b"abc", &mut w, false).is_err()); + } + + #[test] + fn test_write_repeating_errors_on_no_progress() { + let mut w = ZeroWriter; + assert!(write_repeating(b"abc", &mut w, false).is_err()); + } +}