From 079edcc925f6562e0a459fc7f7726bcbf836079d Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Wed, 24 Dec 2025 15:39:16 +0000 Subject: [PATCH 1/4] dd: use seek for stdin skip when possible --- src/uu/dd/src/dd.rs | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index e2344952a0f..651c821485a 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -272,14 +272,38 @@ impl Source { return Ok(len); } } - let m = read_and_discard(f, n, ibs)?; - if m < n { - show_error!( - "{}", - translate!("dd-error-cannot-skip-offset", "file" => "standard input") - ); + // Try seek first; fall back to read if not seekable + match n.try_into().ok().map(|n| f.seek(SeekFrom::Current(n))) { + Some(Ok(pos)) => { + if pos > f.metadata().map(|m| m.len()).unwrap_or(u64::MAX) { + show_error!( + "{}", + translate!("dd-error-cannot-skip-offset", "file" => "standard input") + ); + } + Ok(pos) + } + Some(Err(e)) if e.raw_os_error() == Some(libc::ESPIPE) => { + match io::copy(&mut f.take(n), &mut io::sink()) { + Ok(m) if m < n => { + show_error!( + "{}", + translate!("dd-error-cannot-skip-offset", "file" => "standard input") + ); + Ok(m) + } + other => other, + } + } + _ => { + show_error!( + "{}", + translate!("dd-error-cannot-skip-invalid", "file" => "standard input") + ); + set_exit_code(1); + Ok(0) + } } - Ok(m) } Self::File(f) => f.seek(SeekFrom::Current(n.try_into().unwrap())), #[cfg(unix)] From edd921ff303ccc94d443ef349add6d86deba080d Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 26 Dec 2025 01:16:43 +0000 Subject: [PATCH 2/4] Add Rust tests for skip with seekable stdin --- tests/by-util/test_dd.rs | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 35a1561e498..71fe717fe25 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -669,6 +669,56 @@ fn test_skip_beyond_file() { ); } +#[test] +#[cfg(unix)] +fn test_skip_beyond_file_seekable_stdin() { + // When stdin is a seekable file, dd should use seek to skip bytes. + // This tests that skipping beyond the file size issues a warning. + use std::process::Stdio; + + let (at, mut ucmd) = at_and_ucmd!(); + at.write("in", "abcd"); + + let stdin = OwnedFileDescriptorOrHandle::open_file( + OpenOptions::new().read(true), + at.plus("in").as_path(), + ) + .unwrap(); + + // skip=5 with bs=1 means skip 5 bytes, which is beyond the 4-byte file + ucmd.args(&["bs=1", "skip=5", "count=0", "status=noxfer"]) + .set_stdin(Stdio::from(stdin)) + .succeeds() + .no_stdout() + .stderr_contains( + "'standard input': cannot skip to specified offset\n0+0 records in\n0+0 records out\n", + ); +} + +#[test] +#[cfg(unix)] +fn test_skip_beyond_file_seekable_stdin_bytes() { + use std::process::Stdio; + + let (at, mut ucmd) = at_and_ucmd!(); + at.write("in", "abcd"); + + let stdin = OwnedFileDescriptorOrHandle::open_file( + OpenOptions::new().read(true), + at.plus("in").as_path(), + ) + .unwrap(); + + // skip=2 with bs=3 means skip 6 bytes, which is beyond the 4-byte file + ucmd.args(&["bs=3", "skip=2", "count=0", "status=noxfer"]) + .set_stdin(Stdio::from(stdin)) + .succeeds() + .no_stdout() + .stderr_contains( + "'standard input': cannot skip to specified offset\n0+0 records in\n0+0 records out\n", + ); +} + #[test] fn test_seek_do_not_overwrite() { let (at, mut ucmd) = at_and_ucmd!(); From 27d05cbf3bdac3d4f6387981e9e238b8685a65c4 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Tue, 30 Dec 2025 20:22:13 +0000 Subject: [PATCH 3/4] Address review comments: fix return value, add comment, refactor tests --- src/uu/dd/src/dd.rs | 8 ++++-- tests/by-util/test_dd.rs | 59 ++++++++++++++-------------------------- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 651c821485a..67d172fa482 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -272,17 +272,21 @@ impl Source { return Ok(len); } } + // Get file length before seeking to avoid race condition + let file_len = f.metadata().map(|m| m.len()).unwrap_or(u64::MAX); // Try seek first; fall back to read if not seekable match n.try_into().ok().map(|n| f.seek(SeekFrom::Current(n))) { Some(Ok(pos)) => { - if pos > f.metadata().map(|m| m.len()).unwrap_or(u64::MAX) { + if pos > file_len { show_error!( "{}", translate!("dd-error-cannot-skip-offset", "file" => "standard input") ); } - Ok(pos) + Ok(n) } + // ESPIPE means the file descriptor is not seekable (e.g., a pipe), + // so fall back to reading and discarding bytes Some(Err(e)) if e.raw_os_error() == Some(libc::ESPIPE) => { match io::copy(&mut f.take(n), &mut io::sink()) { Ok(m) if m < n => { diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 71fe717fe25..3e53f9a59e5 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -676,47 +676,30 @@ fn test_skip_beyond_file_seekable_stdin() { // This tests that skipping beyond the file size issues a warning. use std::process::Stdio; - let (at, mut ucmd) = at_and_ucmd!(); - at.write("in", "abcd"); - - let stdin = OwnedFileDescriptorOrHandle::open_file( - OpenOptions::new().read(true), - at.plus("in").as_path(), - ) - .unwrap(); - - // skip=5 with bs=1 means skip 5 bytes, which is beyond the 4-byte file - ucmd.args(&["bs=1", "skip=5", "count=0", "status=noxfer"]) - .set_stdin(Stdio::from(stdin)) - .succeeds() - .no_stdout() - .stderr_contains( - "'standard input': cannot skip to specified offset\n0+0 records in\n0+0 records out\n", - ); -} - -#[test] -#[cfg(unix)] -fn test_skip_beyond_file_seekable_stdin_bytes() { - use std::process::Stdio; + // Test cases: (bs, skip) pairs that skip beyond a 4-byte file + let test_cases = [ + ("bs=1", "skip=5"), // skip 5 bytes + ("bs=3", "skip=2"), // skip 6 bytes + ]; - let (at, mut ucmd) = at_and_ucmd!(); - at.write("in", "abcd"); + for (bs, skip) in test_cases { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("in", "abcd"); - let stdin = OwnedFileDescriptorOrHandle::open_file( - OpenOptions::new().read(true), - at.plus("in").as_path(), - ) - .unwrap(); + let stdin = OwnedFileDescriptorOrHandle::open_file( + OpenOptions::new().read(true), + at.plus("in").as_path(), + ) + .unwrap(); - // skip=2 with bs=3 means skip 6 bytes, which is beyond the 4-byte file - ucmd.args(&["bs=3", "skip=2", "count=0", "status=noxfer"]) - .set_stdin(Stdio::from(stdin)) - .succeeds() - .no_stdout() - .stderr_contains( - "'standard input': cannot skip to specified offset\n0+0 records in\n0+0 records out\n", - ); + ucmd.args(&[bs, skip, "count=0", "status=noxfer"]) + .set_stdin(Stdio::from(stdin)) + .succeeds() + .no_stdout() + .stderr_contains( + "'standard input': cannot skip to specified offset\n0+0 records in\n0+0 records out\n", + ); + } } #[test] From bd6ce1433554fa69ba119c6961d54574653c2fbd Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Tue, 13 Jan 2026 16:21:01 +0000 Subject: [PATCH 4/4] dd: use ibs-sized buffer for ESPIPE fallback in skip --- src/uu/dd/src/dd.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 67d172fa482..f5f4f9365a6 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -286,18 +286,16 @@ impl Source { Ok(n) } // ESPIPE means the file descriptor is not seekable (e.g., a pipe), - // so fall back to reading and discarding bytes + // so fall back to reading and discarding bytes using ibs-sized buffer Some(Err(e)) if e.raw_os_error() == Some(libc::ESPIPE) => { - match io::copy(&mut f.take(n), &mut io::sink()) { - Ok(m) if m < n => { - show_error!( - "{}", - translate!("dd-error-cannot-skip-offset", "file" => "standard input") - ); - Ok(m) - } - other => other, + let m = read_and_discard(f, n, ibs)?; + if m < n { + show_error!( + "{}", + translate!("dd-error-cannot-skip-offset", "file" => "standard input") + ); } + Ok(m) } _ => { show_error!(