From 0a98a447ffb8c56189eb44e6036c1b7b5e1183ad Mon Sep 17 00:00:00 2001 From: NdekoCode Date: Thu, 8 Oct 2026 00:32:31 +0200 Subject: [PATCH 1/3] shred: report a FIFO, a directory and a missing file as GNU does Every operand that was not a regular file was reported as "Not a file", and a missing one as "No such file or directory". GNU refuses a FIFO before opening it, as an "invalid file type", and reports a directory or a missing file as the failure to open them for writing. Say so, keep "Not a file" for the character and block devices that shred does not overwrite yet, and let a socket reach the open, whose failure is what GNU reports for it. --- src/uu/shred/locales/en-US.ftl | 2 +- src/uu/shred/locales/fr-FR.ftl | 2 +- src/uu/shred/src/shred.rs | 50 +++++++++++++++++++++++++++++----- tests/by-util/test_shred.rs | 38 ++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/uu/shred/locales/en-US.ftl b/src/uu/shred/locales/en-US.ftl index b2f749a0b82..1df9b4ac99f 100644 --- a/src/uu/shred/locales/en-US.ftl +++ b/src/uu/shred/locales/en-US.ftl @@ -41,10 +41,10 @@ shred-missing-file-operand = missing file operand 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 +shred-invalid-file-type = {$file}: invalid file type # Option help text shred-force-help = change permissions to allow writing if necessary diff --git a/src/uu/shred/locales/fr-FR.ftl b/src/uu/shred/locales/fr-FR.ftl index d03b9d74ad8..83bbdba67fb 100644 --- a/src/uu/shred/locales/fr-FR.ftl +++ b/src/uu/shred/locales/fr-FR.ftl @@ -40,10 +40,10 @@ shred-missing-file-operand = opérande de fichier manquant 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 +shred-invalid-file-type = {$file} : type de fichier invalide # Texte d'aide des options shred-force-help = modifier les permissions pour permettre l'écriture si nécessaire diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 5d550a98edf..67ec767262e 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -612,6 +612,30 @@ fn create_standard_pass_sequence(num_passes: usize) -> Vec { #[allow(clippy::too_many_arguments)] #[allow(clippy::cognitive_complexity)] +#[cfg(unix)] +fn is_fifo(metadata: &fs::Metadata) -> bool { + use std::os::unix::fs::FileTypeExt; + metadata.file_type().is_fifo() +} + +#[cfg(not(unix))] +fn is_fifo(_metadata: &fs::Metadata) -> bool { + false +} + +/// A character or block device, which shred does not overwrite yet. +#[cfg(unix)] +fn is_device(metadata: &fs::Metadata) -> bool { + use std::os::unix::fs::FileTypeExt; + let file_type = metadata.file_type(); + file_type.is_char_device() || file_type.is_block_device() +} + +#[cfg(not(unix))] +fn is_device(metadata: &fs::Metadata) -> bool { + !metadata.is_file() +} + fn wipe_file( path_str: &OsString, n_passes: usize, @@ -644,22 +668,34 @@ fn wipe_file( // `Path::exists()` and `Path::is_file()` both collapse any metadata error // (including a permission error) into `false`, which made shred report a // file whose parent directory lacks search permission as "No such file or - // directory". Inspect the metadata directly so a genuine `ENOENT` stays a - // "no such file" error while a permission error falls through to the - // open-for-writing below, which surfaces the real reason. + // directory". Inspect the metadata directly: a directory and a missing + // file are reported as the failures to open them that GNU meets, a FIFO + // as a type GNU refuses before opening, and a permission error falls + // through to the open-for-writing below, which surfaces the real reason. match fs::metadata(path) { - Ok(md) if !md.is_file() => { + Ok(md) if md.is_dir() => { return Err(USimpleError::new( 1, - translate!("shred-not-a-file", "file" => path.maybe_quote()), + translate!("shred-failed-to-open-for-writing-is-a-directory", "file" => path.maybe_quote()), )); } - Err(err) if err.kind() == io::ErrorKind::NotFound => { + Ok(md) if is_fifo(&md) => { + return Err(USimpleError::new( + 1, + translate!("shred-invalid-file-type", "file" => path.maybe_quote()), + )); + } + Ok(md) if is_device(&md) => { return Err(USimpleError::new( 1, - translate!("shred-no-such-file-or-directory", "file" => path.maybe_quote()), + translate!("shred-not-a-file", "file" => path.maybe_quote()), )); } + Err(err) if err.kind() == io::ErrorKind::NotFound => { + return Err(err).map_err_context( + || translate!("shred-failed-to-open-for-writing", "file" => path.maybe_quote()), + ); + } _ => {} } diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 09cea46b439..9fbc06c532c 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -585,3 +585,41 @@ fn test_couldnt_rename() { .fails_with_code(1) .stderr_is("shred: /proc/self/mem: Couldn't rename to '/proc/self/000': No such file or directory\n"); } + +#[test] +fn test_shred_directory() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("d"); + ucmd.arg("d") + .fails_with_code(1) + .stderr_only("shred: d: failed to open for writing: Is a directory\n"); +} + +#[test] +fn test_shred_missing_file() { + new_ucmd!() + .arg("missing") + .fails_with_code(1) + .stderr_only("shred: missing: failed to open for writing: No such file or directory\n"); +} + +#[test] +#[cfg(unix)] +fn test_shred_fifo() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("p"); + ucmd.arg("p") + .fails_with_code(1) + .stderr_only("shred: p: invalid file type\n"); +} + +#[test] +#[cfg(unix)] +fn test_shred_socket() { + // GNU reaches the open and reports its failure; so does shred here. + let (at, mut ucmd) = at_and_ucmd!(); + let _listener = std::os::unix::net::UnixListener::bind(at.plus("s")).unwrap(); + ucmd.arg("s") + .fails_with_code(1) + .stderr_contains("shred: s: failed to open for writing: "); +} From 0f741edb5a8ffeb261d792cc6f9b7faed15b8c7d Mon Sep 17 00:00:00 2001 From: NdekoCode Date: Fri, 9 Oct 2026 23:21:19 +0200 Subject: [PATCH 2/3] shred: stat the file once and classify it from that Move the helpers above the clippy attributes so these apply to wipe_file again, and rename is_device to is_special_file since off unix it means anything but a regular file. The trailing-slash block and the second metadata call are gone: one stat gives the type, the size and the mode. A directory is reported from its metadata, "f/" with f a file from the NotADirectory error kind, and every other stat error as the failure to open for writing GNU meets, so an unsearchable parent now prints "Permission denied" with the file name instead of "failed to get file metadata". --- src/uu/shred/locales/en-US.ftl | 1 - src/uu/shred/locales/fr-FR.ftl | 1 - src/uu/shred/src/shred.rs | 57 ++++++++++++++-------------------- tests/by-util/test_shred.rs | 12 +++---- 4 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/uu/shred/locales/en-US.ftl b/src/uu/shred/locales/en-US.ftl index 1df9b4ac99f..234f00e24b3 100644 --- a/src/uu/shred/locales/en-US.ftl +++ b/src/uu/shred/locales/en-US.ftl @@ -72,5 +72,4 @@ shred-failed-to-remove-file = {$file}: failed to remove file shred-failed-to-clone-file-handle = failed to clone file handle shred-failed-to-seek-file = failed to seek in file shred-failed-to-read-seed-bytes = failed to read seed bytes from file -shred-failed-to-get-metadata = failed to get file metadata shred-failed-to-set-permissions = failed to set file permissions diff --git a/src/uu/shred/locales/fr-FR.ftl b/src/uu/shred/locales/fr-FR.ftl index 83bbdba67fb..382739ff75d 100644 --- a/src/uu/shred/locales/fr-FR.ftl +++ b/src/uu/shred/locales/fr-FR.ftl @@ -71,5 +71,4 @@ shred-failed-to-remove-file = {$file} : impossible de supprimer le fichier shred-failed-to-clone-file-handle = échec du clonage du descripteur de fichier shred-failed-to-seek-file = échec de la recherche dans le fichier shred-failed-to-read-seed-bytes = échec de la lecture des octets de graine du fichier -shred-failed-to-get-metadata = échec de l'obtention des métadonnées du fichier shred-failed-to-set-permissions = échec de la définition des permissions du fichier diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 67ec767262e..418e93ac21d 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -610,8 +610,6 @@ fn create_standard_pass_sequence(num_passes: usize) -> Vec { sequence } -#[allow(clippy::too_many_arguments)] -#[allow(clippy::cognitive_complexity)] #[cfg(unix)] fn is_fifo(metadata: &fs::Metadata) -> bool { use std::os::unix::fs::FileTypeExt; @@ -625,17 +623,20 @@ fn is_fifo(_metadata: &fs::Metadata) -> bool { /// A character or block device, which shred does not overwrite yet. #[cfg(unix)] -fn is_device(metadata: &fs::Metadata) -> bool { +fn is_special_file(metadata: &fs::Metadata) -> bool { use std::os::unix::fs::FileTypeExt; let file_type = metadata.file_type(); file_type.is_char_device() || file_type.is_block_device() } +/// Anything but a regular file: `std` cannot tell the kinds apart here. #[cfg(not(unix))] -fn is_device(metadata: &fs::Metadata) -> bool { +fn is_special_file(metadata: &fs::Metadata) -> bool { !metadata.is_file() } +#[allow(clippy::too_many_arguments)] +#[allow(clippy::cognitive_complexity)] fn wipe_file( path_str: &OsString, n_passes: usize, @@ -650,29 +651,14 @@ fn wipe_file( // Get these potential errors out of the way first let path = Path::new(path_str); - if path_str.as_encoded_bytes().ends_with(b"/") { - if path.is_dir() { - return Err(USimpleError::new( - 1, - translate!("shred-failed-to-open-for-writing-is-a-directory", "file" => path.maybe_quote()), - )); - } - if fs::metadata(path).is_err_and(|e| e.kind() == io::ErrorKind::NotADirectory) { - return Err(USimpleError::new( - 1, - translate!("shred-failed-to-open-for-writing-not-a-directory", "file" => path.maybe_quote()), - )); - } - } - - // `Path::exists()` and `Path::is_file()` both collapse any metadata error - // (including a permission error) into `false`, which made shred report a - // file whose parent directory lacks search permission as "No such file or - // directory". Inspect the metadata directly: a directory and a missing - // file are reported as the failures to open them that GNU meets, a FIFO - // as a type GNU refuses before opening, and a permission error falls - // through to the open-for-writing below, which surfaces the real reason. - match fs::metadata(path) { + // One stat classifies the file and later gives its size and mode. A FIFO + // has to be refused before the open below, which would block until a + // reader shows up. The stat errors (missing file, "f/" with `f` a file, + // no search permission on the parent) are the ones that open would meet, + // so they are reported the way GNU reports them: a failure to open for + // writing. The directory and "Not a directory" messages are spelled out + // because the OS text differs on Windows. + let metadata = match fs::metadata(path) { Ok(md) if md.is_dir() => { return Err(USimpleError::new( 1, @@ -685,22 +671,25 @@ fn wipe_file( translate!("shred-invalid-file-type", "file" => path.maybe_quote()), )); } - Ok(md) if is_device(&md) => { + Ok(md) if is_special_file(&md) => { return Err(USimpleError::new( 1, translate!("shred-not-a-file", "file" => path.maybe_quote()), )); } - Err(err) if err.kind() == io::ErrorKind::NotFound => { + Ok(md) => md, + Err(err) if err.kind() == io::ErrorKind::NotADirectory => { + return Err(USimpleError::new( + 1, + translate!("shred-failed-to-open-for-writing-not-a-directory", "file" => path.maybe_quote()), + )); + } + Err(err) => { return Err(err).map_err_context( || translate!("shred-failed-to-open-for-writing", "file" => path.maybe_quote()), ); } - _ => {} - } - - let metadata = - fs::metadata(path).map_err_context(|| translate!("shred-failed-to-get-metadata"))?; + }; // If force is true, set file permissions to not-readonly. if force { diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 9fbc06c532c..489d1fdb0ed 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -502,13 +502,11 @@ fn test_shred_inaccessible_file_reports_real_error() { at.touch("locked/file"); set_permissions(at.plus_as_string("locked"), Permissions::from_mode(0o000)).unwrap(); - let result = scene.ucmd().arg("locked/file").fails(); - result.stderr_contains("Permission denied"); - assert!( - !result.stderr_str().contains("No such file"), - "shred misreported an inaccessible file as missing: {}", - result.stderr_str() - ); + scene + .ucmd() + .arg("locked/file") + .fails_with_code(1) + .stderr_only("shred: locked/file: failed to open for writing: Permission denied\n"); // Restore search permission so the fixture directory can be cleaned up. set_permissions(at.plus_as_string("locked"), Permissions::from_mode(0o755)).unwrap(); From ea5777ecc40393c3a229dcc2ced02ee9b93c879d Mon Sep 17 00:00:00 2001 From: NdekoCode Date: Tue, 20 Oct 2026 22:55:10 +0200 Subject: [PATCH 3/3] tests: shred: skip the FIFO and socket tests under WASI The wasm binary has no unix file types, so both are just not a file there. --- tests/by-util/test_shred.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 489d1fdb0ed..d01003fffd0 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -603,6 +603,7 @@ fn test_shred_missing_file() { #[test] #[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI: no FIFO or socket file types")] fn test_shred_fifo() { let (at, mut ucmd) = at_and_ucmd!(); at.mkfifo("p"); @@ -613,6 +614,7 @@ fn test_shred_fifo() { #[test] #[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI: no FIFO or socket file types")] fn test_shred_socket() { // GNU reaches the open and reports its failure; so does shred here. let (at, mut ucmd) = at_and_ucmd!();