Skip to content

dd: warn when skip goes past the end of the input file #13115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eyupcanakman wants to merge 1 commit into
base: main
Choose a base branch
from
Open

dd: warn when skip goes past the end of the input file #13115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions src/uu/dd/src/dd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use uucore::translate;
use std::cmp;
use std::env;
use std::ffi::OsString;
#[cfg(unix)]
use std::fs::Metadata;
use std::fs::{File, OpenOptions};
use std::io::{self, Read, Seek, SeekFrom, Write};
Expand Down Expand Up @@ -231,15 +230,15 @@ impl Source {
Self::StdinFile(f)
}

fn skip(&mut self, n: u64, ibs: usize) -> io::Result<u64> {
fn skip(&mut self, n: u64, ibs: usize, name: &str) -> io::Result<u64> {
match self {
#[cfg(not(unix))]
Self::Stdin(stdin) => {
let m = read_and_discard(stdin, n, ibs)?;
if m < n {
show_error!(
"{}",
translate!("dd-error-cannot-skip-offset", "file" => "standard input")
translate!("dd-error-cannot-skip-offset", "file" => name)
);
}
Ok(m)
Expand All @@ -253,7 +252,7 @@ impl Source {
// this case prints the stats but sets the exit code to 1
show_error!(
"{}",
translate!("dd-error-cannot-skip-invalid", "file" => "standard input")
translate!("dd-error-cannot-skip-invalid", "file" => name)
);
set_exit_code(1);
return Ok(len);
Expand All @@ -263,10 +262,10 @@ impl Source {
// 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 > file_len {
if file_len != 0 && pos > file_len {
show_error!(
"{}",
translate!("dd-error-cannot-skip-offset", "file" => "standard input")
translate!("dd-error-cannot-skip-offset", "file" => name)
);
}
Ok(n)
Expand All @@ -278,22 +277,33 @@ impl Source {
if m < n {
show_error!(
"{}",
translate!("dd-error-cannot-skip-offset", "file" => "standard input")
translate!("dd-error-cannot-skip-offset", "file" => name)
);
}
Ok(m)
}
_ => {
show_error!(
"{}",
translate!("dd-error-cannot-skip-invalid", "file" => "standard input")
translate!("dd-error-cannot-skip-invalid", "file" => name)
);
set_exit_code(1);
Ok(0)
}
}
}
Self::File(f) => f.seek(SeekFrom::Current(n.try_into().unwrap())),
Self::File(f) => {
// Get file length before seeking to avoid race condition
let file_len = f.metadata().as_ref().map_or(u64::MAX, Metadata::len);
let pos = f.seek(SeekFrom::Current(n.try_into().unwrap()))?;
if file_len != 0 && pos > file_len {
show_error!(
"{}",
translate!("dd-error-cannot-skip-offset", "file" => name)
);
}
Ok(pos)
}
#[cfg(unix)]
Self::Fifo(f) => read_and_discard(f, n, ibs),
}
Expand Down Expand Up @@ -386,7 +396,7 @@ impl<'a> Input<'a> {
}

if settings.skip > 0 {
src.skip(settings.skip, settings.ibs)?;
src.skip(settings.skip, settings.ibs, "standard input")?;
}
Ok(Self { src, settings })
}
Expand All @@ -409,7 +419,7 @@ impl<'a> Input<'a> {

let mut src = Source::File(src);
if settings.skip > 0 {
src.skip(settings.skip, settings.ibs)?;
src.skip(settings.skip, settings.ibs, &filename.to_string_lossy())?;
}
Ok(Self { src, settings })
}
Expand All @@ -423,7 +433,7 @@ impl<'a> Input<'a> {
opts.custom_flags(make_linux_iflags(&settings.iflags).unwrap_or(0));
let mut src = Source::Fifo(opts.open(filename)?);
if settings.skip > 0 {
src.skip(settings.skip, settings.ibs)?;
src.skip(settings.skip, settings.ibs, &filename.to_string_lossy())?;
}
Ok(Self { src, settings })
}
Expand Down
42 changes: 42 additions & 0 deletions tests/by-util/test_dd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,48 @@ fn test_skip_beyond_file_seekable_stdin() {
}
}

/// Test that skipping beyond the end of a named input file prints a warning.
#[test]
fn test_skip_beyond_named_file() {
let (at, mut ucmd) = at_and_ucmd!();
at.write("in", "abcd");
ucmd.args(&["if=in", "bs=1", "skip=5", "count=0", "status=noxfer"])
.succeeds()
.no_stdout()
.stderr_contains(
"'in': cannot skip to specified offset\n0+0 records in\n0+0 records out\n",
);
}

/// Test that skipping in an empty named input file does not print a warning.
#[test]
fn test_skip_in_empty_named_file_no_warning() {
let (at, mut ucmd) = at_and_ucmd!();
at.write("in", "");
ucmd.args(&["if=in", "bs=1", "skip=1", "count=0", "status=noxfer"])
.succeeds()
.no_stdout()
.stderr_is("0+0 records in\n0+0 records out\n");
}

/// Test that skipping in an empty seekable stdin does not print a warning.
#[test]
#[cfg(unix)]
fn test_skip_in_empty_seekable_stdin_no_warning() {
let (at, mut ucmd) = at_and_ucmd!();
at.write("in", "");
let stdin = OwnedFileDescriptorOrHandle::open_file(
OpenOptions::new().read(true),
at.plus("in").as_path(),
)
.unwrap();
ucmd.args(&["bs=1", "skip=1", "count=0", "status=noxfer"])
.set_stdin(Stdio::from(stdin))
.succeeds()
.no_stdout()
.stderr_is("0+0 records in\n0+0 records out\n");
}

#[test]
fn test_seek_do_not_overwrite() {
let (at, mut ucmd) = at_and_ucmd!();
Expand Down
Loading