Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 0 additions & 2 deletions src/uu/shred/locales/en-US.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ shred-invalid-number-of-passes = invalid number of passes: {$passes}
shred-cannot-open-random-source = cannot open random source: {$source}
shred-invalid-file-size = invalid file size: {$size}
shred-no-such-file-or-directory = {$file}: No such file or directory
shred-failed-to-open-for-writing-not-a-directory = {$file}: failed to open for writing: Not a directory
shred-failed-to-open-for-writing-is-a-directory = {$file}: failed to open for writing: Is a directory
shred-not-a-file = {$file}: Not a file

# Option help text
Expand Down
2 changes: 0 additions & 2 deletions src/uu/shred/locales/fr-FR.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ shred-invalid-number-of-passes = nombre de passes invalide : {$passes}
shred-cannot-open-random-source = impossible d'ouvrir la source aléatoire : {$source}
shred-invalid-file-size = taille de fichier invalide : {$size}
shred-no-such-file-or-directory = {$file} : Aucun fichier ou répertoire de ce type
shred-failed-to-open-for-writing-not-a-directory = {$file} : impossible d'ouvrir en écriture : N'est pas un répertoire
shred-failed-to-open-for-writing-is-a-directory = {$file} : impossible d'ouvrir en écriture : Est un répertoire
shred-not-a-file = {$file} : N'est pas un fichier

# Texte d'aide des options
Expand Down
28 changes: 14 additions & 14 deletions src/uu/shred/src/shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,19 +651,18 @@ fn wipe_file(
display_path
};

if path_str.as_encoded_bytes().ends_with(b"/") {
if path.is_dir() {
return Err(USimpleError::new(
1,
translate_text!("shred-failed-to-open-for-writing-is-a-directory", "file" => display_path.maybe_quote()),
));
}
if fs::metadata(path).is_err_and(|e| e.kind() == io::ErrorKind::NotADirectory) {
return Err(USimpleError::new(
1,
translate_text!("shred-failed-to-open-for-writing-not-a-directory", "file" => display_path.maybe_quote()),
));
}
if path_str.as_encoded_bytes().ends_with(b"/")
&& let Err(e) = fs::metadata(path)
&& e.kind() == io::ErrorKind::NotADirectory
{
return Err(USimpleError::new(
1,
format!(
"{}: {}",
translate_text!("shred-failed-to-open-for-writing", "file" => display_path.maybe_quote()),
strip_errno(&e)
),
));
}

// On OpenBSD, /dev/fd/1 is a character device rather than a symlink.
Expand Down Expand Up @@ -692,7 +691,8 @@ fn wipe_file(
// "no such file" error while a permission error falls through to the
// open-for-writing below, which surfaces the real reason.
match get_metadata() {
Ok(md) if !md.is_file() => {
// Directories fall through so opening them for writing reports EISDIR.
Ok(md) if !md.is_file() && !md.is_dir() => {
return Err(USimpleError::new(
1,
translate_text!("shred-not-a-file", "file" => display_path.maybe_quote()),
Expand Down
30 changes: 15 additions & 15 deletions tests/by-util/test_shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,23 +467,23 @@ fn test_shred_trailing_slash_on_file() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
at.touch("a");
scene
.ucmd()
.arg("a/")
.fails()
.stderr_contains("Not a directory");
let result = scene.ucmd().arg("a/").fails();
result.stderr_contains("a/: failed to open for writing: ");
// Windows reports this as "The directory name is invalid."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test stderr contains this string on Windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the Windows jobs run this test and pass. Both paths that can hit a/ there (the trailing slash check and the open for writing below it) print the a/: failed to open for writing: prefix; only the OS text after it differs ("The directory name is invalid." on Windows), which is why only the "Not a directory" check is cfg(unix).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean test that stderr contains it on Windows, something like:

#[cfg(windows)]
result.stderr_contains("a/: The directory name is invalid: ");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Done in bafc9d0: the test now checks the whole a/: failed to open for writing: ... line on both, "Not a directory" on unix and "The directory name is invalid." on Windows.

#[cfg(unix)]
result.stderr_contains("Not a directory");
}

#[test]
fn test_shred_trailing_slash_on_dir() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
#[rstest::rstest]
#[case("d")]
#[case("d/")]
#[cfg(unix)]
fn test_shred_on_dir(#[case] arg: &str) {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("d");
scene
.ucmd()
.arg("d/")
.fails()
.stderr_contains("Is a directory");
ucmd.arg(arg).fails_with_code(1).stderr_only(format!(
"shred: {arg}: failed to open for writing: Is a directory\n"
));
}

#[test]
Expand Down Expand Up @@ -588,7 +588,7 @@ fn test_couldnt_rename() {

#[rstest::rstest]
#[case(&["1e3"], "shred: 1e3: No such file or directory\n")]
#[case(&["007"], "shred: 007: Not a file\n")]
#[case(&["007"], "shred: 007: failed to open for writing: Is a directory\n")]
#[cfg(not(windows))]
fn test_numeric_looking_file_names_are_kept_verbatim(
#[case] args: &[&str],
Expand Down
Loading