diff --git a/src/uu/install/locales/en-US.ftl b/src/uu/install/locales/en-US.ftl index f7104d5e415..e71c5799cf8 100644 --- a/src/uu/install/locales/en-US.ftl +++ b/src/uu/install/locales/en-US.ftl @@ -45,7 +45,7 @@ install-error-same-file = { $file1 } and { $file2 } are the same file install-error-extra-operand = extra operand { $operand } { $usage } install-error-not-permitted = cannot remove { $path }: Operation not permitted -install-error-invalid-mode = Invalid mode string: { $error } +install-error-invalid-mode = invalid mode '{ $mode }' install-error-mutually-exclusive-target = Options --target-directory and --no-target-directory are mutually exclusive install-error-mutually-exclusive-compare-strip = Options --compare and --strip are mutually exclusive install-error-missing-file-operand = missing file operand diff --git a/src/uu/install/locales/fr-FR.ftl b/src/uu/install/locales/fr-FR.ftl index 1503c0c7c7d..23c7ce0e60d 100644 --- a/src/uu/install/locales/fr-FR.ftl +++ b/src/uu/install/locales/fr-FR.ftl @@ -44,7 +44,7 @@ install-error-override-directory-failed = impossible d'écraser le répertoire { install-error-same-file = { $file1 } et { $file2 } sont le même fichier install-error-extra-operand = opérande supplémentaire { $operand } { $usage } -install-error-invalid-mode = Chaîne de mode invalide : { $error } +install-error-invalid-mode = mode invalide '{ $mode }' install-error-mutually-exclusive-target = Les options --target-directory et --no-target-directory sont mutuellement exclusives install-error-mutually-exclusive-compare-strip = Les options --compare et --strip sont mutuellement exclusives install-error-missing-file-operand = opérande de fichier manquant diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index b8ff492039c..ac3eb6ddab0 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -374,7 +374,9 @@ fn behavior(matches: &ArgMatches, diag_args: Option<&[OsString]>) -> UResult = if matches.contains_id(OPT_MODE) { let x = matches.get_one::(OPT_MODE).ok_or(1)?; Some(uucore::mode::parse(x, considering_dir, 0).map_err(|err| { - let message = translate!("install-error-invalid-mode", "error" => err.to_string()); + // GNU always says the same thing regardless of what specifically + // went wrong, and does not add a "Try --help" hint here. + let message = translate!("install-error-invalid-mode", "mode" => x.clone()); // When the diagnostic is rendered it is already on stderr; exit quietly. if !diag_args.is_some_and(|args| err.render_mode_value(args, x, 0, &message)) { show_error!("{message}"); diff --git a/src/uu/mkdir/locales/en-US.ftl b/src/uu/mkdir/locales/en-US.ftl index fa2ef528d45..5ce490d4526 100644 --- a/src/uu/mkdir/locales/en-US.ftl +++ b/src/uu/mkdir/locales/en-US.ftl @@ -15,6 +15,7 @@ mkdir-error-file-exists = { $path }: File exists mkdir-error-failed-to-create-tree = failed to create whole tree mkdir-error-cannot-set-permissions = cannot set permissions { $path } mkdir-error-cannot-create-directory = cannot create directory '{ $path }': { $error } +mkdir-error-invalid-mode = invalid mode '{ $mode }' # Verbose output mkdir-verbose-created-directory = { $util_name }: created directory { $path } diff --git a/src/uu/mkdir/locales/fr-FR.ftl b/src/uu/mkdir/locales/fr-FR.ftl index 988f7c21f44..a60fbc2773a 100644 --- a/src/uu/mkdir/locales/fr-FR.ftl +++ b/src/uu/mkdir/locales/fr-FR.ftl @@ -15,6 +15,7 @@ mkdir-error-file-exists = { $path } : Le fichier existe mkdir-error-failed-to-create-tree = échec de la création de l'arborescence complète mkdir-error-cannot-set-permissions = impossible de définir les permissions { $path } mkdir-error-cannot-create-directory = impossible de créer le répertoire '{ $path }': { $error } +mkdir-error-invalid-mode = mode invalide '{ $mode }' # Sortie détaillée mkdir-verbose-created-directory = { $util_name } : répertoire créé { $path } diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 0cb71395ae0..9c26ef5b3ea 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -70,11 +70,15 @@ fn get_mode(matches: &ArgMatches, diag_args: Option<&[OsString]>) -> UResult m.clone()); + if diag_args.is_some_and(|args| err.render_mode_value(args, m, 0, &message)) { // The diagnostic is already on stderr; exit quietly. ExitCode::new(1) } else { - USimpleError::new(1, err.to_string()) + USimpleError::new(1, message) } }) } diff --git a/src/uu/mkfifo/locales/en-US.ftl b/src/uu/mkfifo/locales/en-US.ftl index f66a6b0459a..1633c4ed1df 100644 --- a/src/uu/mkfifo/locales/en-US.ftl +++ b/src/uu/mkfifo/locales/en-US.ftl @@ -7,7 +7,7 @@ mkfifo-help-selinux = set the SELinux security context to default type mkfifo-help-context = like -Z, or if CTX is specified then set the SELinux or SMACK security context to CTX # Error messages -mkfifo-error-invalid-mode = invalid mode: { $error } +mkfifo-error-invalid-mode = invalid mode mkfifo-error-cannot-create-fifo = cannot create fifo { $path }: { $error } mkfifo-error-cannot-set-permissions = cannot set permissions on { $path }: { $error } mkfifo-error-non-file-permission = mode must specify only file permission bits diff --git a/src/uu/mkfifo/locales/fr-FR.ftl b/src/uu/mkfifo/locales/fr-FR.ftl index 038d8284927..4dc349a2374 100644 --- a/src/uu/mkfifo/locales/fr-FR.ftl +++ b/src/uu/mkfifo/locales/fr-FR.ftl @@ -7,7 +7,7 @@ mkfifo-help-selinux = définir le contexte de sécurité SELinux au type par dé mkfifo-help-context = comme -Z, ou si CTX est spécifié, définir le contexte de sécurité SELinux ou SMACK à CTX # Messages d'erreur -mkfifo-error-invalid-mode = mode invalide : { $error } +mkfifo-error-invalid-mode = mode invalide mkfifo-error-cannot-create-fifo = impossible de créer le fifo { $path } : { $error } mkfifo-error-cannot-set-permissions = impossible de définir les permissions sur { $path } : { $error } mkfifo-error-non-file-permission = le mode ne doit spécifier que des bits de permission de fichier diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs index a28151cb443..30794ad6791 100644 --- a/src/uu/mkfifo/src/mkfifo.rs +++ b/src/uu/mkfifo/src/mkfifo.rs @@ -28,7 +28,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; let mode = calculate_mode(matches.get_one::(options::MODE)).map_err(|err| { - let message = translate!("mkfifo-error-invalid-mode", "error" => err.to_string()); + // Unlike `chmod`/`mkdir`/`install`, GNU does not name the mode + // operand here at all, regardless of what went wrong with it. + let message = translate!("mkfifo-error-invalid-mode"); if let Some(args) = &diag_args && let Some(mode) = matches.get_one::(options::MODE) && err.render_mode_value(args, mode, 0, &message) diff --git a/src/uu/mknod/locales/en-US.ftl b/src/uu/mknod/locales/en-US.ftl index 2198bcf382c..30f33a97cf8 100644 --- a/src/uu/mknod/locales/en-US.ftl +++ b/src/uu/mknod/locales/en-US.ftl @@ -28,7 +28,7 @@ mknod-help-context = like -Z, or if CTX is specified then set the SELinux or SMA # Error messages mknod-error-fifo-no-major-minor = Fifos do not have major and minor device numbers. mknod-error-special-require-major-minor = Special files require major and minor device numbers. -mknod-error-invalid-mode = invalid mode ({ $error }) +mknod-error-invalid-mode = invalid mode mknod-error-mode-permission-bits-only = mode must specify only file permission bits mknod-error-missing-device-type = missing device type mknod-error-invalid-device-type = invalid device type { $type } diff --git a/src/uu/mknod/locales/fr-FR.ftl b/src/uu/mknod/locales/fr-FR.ftl index c3917aac52c..6cc1f2b1938 100644 --- a/src/uu/mknod/locales/fr-FR.ftl +++ b/src/uu/mknod/locales/fr-FR.ftl @@ -28,7 +28,7 @@ mknod-help-context = comme -Z, ou si CTX est spécifié, définir le contexte de # Messages d'erreur mknod-error-fifo-no-major-minor = Les fifos n'ont pas de numéros de périphérique majeur et mineur. mknod-error-special-require-major-minor = Les fichiers spéciaux nécessitent des numéros de périphérique majeur et mineur. -mknod-error-invalid-mode = mode invalide ({ $error }) +mknod-error-invalid-mode = mode invalide mknod-error-mode-permission-bits-only = le mode ne doit spécifier que les bits de permission de fichier mknod-error-missing-device-type = type de périphérique manquant mknod-error-invalid-device-type = type de périphérique invalide { $type } diff --git a/src/uu/mknod/src/mknod.rs b/src/uu/mknod/src/mknod.rs index a4398a03323..201fc32231f 100644 --- a/src/uu/mknod/src/mknod.rs +++ b/src/uu/mknod/src/mknod.rs @@ -155,8 +155,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let mode = uucore::mode::parse_chmod(MODE_RW_UGO, str_mode, true, uucore::mode::get_umask()) .map_err(|err| { - let message = - translate!("mknod-error-invalid-mode", "error" => err.to_string()); + // Unlike `chmod`/`mkdir`/`install`, GNU does not name the + // mode operand here at all, regardless of what went wrong + // with it. + let message = translate!("mknod-error-invalid-mode"); if let Some(args) = &diag_args && err.render_mode_value(args, str_mode, 0, &message) { diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 8dfeccb4c94..eab461c3aea 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -131,19 +131,19 @@ impl ModeError { /// The caret label for this error, translated, and the advice that goes /// under it. /// - /// Labelled only where a label would add to the message, per the - /// convention in [`crate::diagnostics`]. + /// Every caller shows the same headline for every `ModeError` -- GNU + /// itself only ever says "invalid mode" (with or without the operand, + /// depending on the utility) -- so the detail that used to live there + /// lives in the label instead. fn describe(&self) -> (Option, Option) { let label = match self.kind { - // The message already names the expected operators. - ModeErrorKind::InvalidOperator => None, - ModeErrorKind::MissingOperator => Some("mode-diag-label-missing-operator"), - ModeErrorKind::InvalidNumber => Some("mode-diag-label-invalid-number"), + // `self.message` already names the operator that was expected + // and the one that was found instead. + ModeErrorKind::InvalidOperator => Some(self.message.clone()), + ModeErrorKind::MissingOperator => Some(translate!("mode-diag-label-missing-operator")), + ModeErrorKind::InvalidNumber => Some(translate!("mode-diag-label-invalid-number")), }; - ( - label.map(|label| translate!(label)), - Some(translate!("mode-diag-help-syntax")), - ) + (label, Some(translate!("mode-diag-help-syntax"))) } /// Where this error sits inside the whole mode, given where the clause it @@ -335,10 +335,12 @@ pub fn parse_chmod( offset += raw_part.len() + 1; let mode_part = raw_part.trim(); - if mode_part.is_empty() { - continue; - } + // An empty clause -- the whole mode, a leading, trailing or doubled + // comma -- is not tolerated: GNU rejects `chmod u+rwx, f` the same + // way it rejects `chmod , f`. `parse_symbolic` already reports that + // correctly for an empty string, so it is not special-cased away + // here. new_mode = if mode_part.chars().any(|c| c.is_ascii_digit()) { parse_numeric(new_mode, mode_part, considering_dir) } else { @@ -469,11 +471,14 @@ mod tests { } #[test] - fn test_parse_empty_string() { - // Empty string should return 0 - assert_eq!(parse("", false, 0).unwrap(), 0); - assert_eq!(parse(" ", false, 0).unwrap(), 0); - assert_eq!(parse(",,", false, 0).unwrap(), 0); + fn test_parse_empty_string_is_invalid() { + // GNU rejects an empty, all-whitespace, or comma-only mode string + // (verified against chmod/install/mkdir 9.11: `chmod '' f`, + // `chmod ' ' f`, `chmod ',' f`, `install -m '' ...`, + // `mkdir -m '' ...` all fail with "invalid mode"). + assert!(parse("", false, 0).is_err()); + assert!(parse(" ", false, 0).is_err()); + assert!(parse(",,", false, 0).is_err()); } #[test] diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index dabd76f01cc..21b4181fbbe 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -339,13 +339,28 @@ fn test_install_mode_failing() { .arg(dir) .arg(mode_arg) .fails() - .stderr_contains("Invalid mode string: invalid digit found in string"); + .stderr_contains("invalid mode '999'"); let dest_file = &format!("{dir}/{file}"); assert!(at.file_exists(file)); assert!(!at.file_exists(dest_file)); } +/// GNU says the same thing for any way `-m`'s mode can be malformed, and it +/// is not the message chmod uses for the same failure: install's has no +/// colon before the quoted mode, and no "Try --help" hint. +#[test] +fn test_install_invalid_mode_names_the_operand() { + // The mode is validated before install so much as looks for the source, + // so a nonexistent one does not confuse this with a different error. + for mode in ["999", "", "a", "u?rwx"] { + new_ucmd!() + .args(&["source_file", "dest", "--mode", mode]) + .fails_with_code(1) + .stderr_only(format!("install: invalid mode '{mode}'\n")); + } +} + #[test] fn test_install_mode_directories() { let (at, mut ucmd) = at_and_ucmd!(); diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index c1796e332e1..11a083878e6 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -108,6 +108,40 @@ fn test_mkdir_mode() { new_ucmd!().arg("-m").arg("755").arg("test_dir").succeeds(); } +/// GNU says the same thing for any way `-m`'s mode can be malformed, and it +/// is not the message chmod uses for the same failure: mkdir's has no +/// colon before the quoted mode, and no "Try --help" hint. +/// +/// `-m` is a no-op on Windows (`get_mode` always returns `None` there), so +/// there is nothing to validate and nothing to fail on that platform. +#[test] +#[cfg(not(windows))] +fn test_mkdir_invalid_mode_names_the_operand() { + for mode in ["999", "", "a", "u?rwx"] { + new_ucmd!() + .args(&["-m", mode, "test_dir"]) + .fails_with_code(1) + .stderr_only(format!("mkdir: invalid mode '{mode}'\n")); + } +} + +/// A comma-separated mode with an empty clause -- leading, trailing, or +/// doubled -- is not tolerated as a no-op clause: GNU rejects the whole +/// thing, the same as it rejects an empty mode outright. +/// +/// `-m` is a no-op on Windows (`get_mode` always returns `None` there), so +/// there is nothing to validate and nothing to fail on that platform. +#[test] +#[cfg(not(windows))] +fn test_mkdir_rejects_an_empty_clause() { + for mode in ["u+rwx,", ",u+rwx", "u+rwx,,g+r"] { + new_ucmd!() + .args(&["-m", mode, "test_dir"]) + .fails_with_code(1) + .stderr_only(format!("mkdir: invalid mode '{mode}'\n")); + } +} + #[test] fn test_mkdir_parent() { let scene = TestScenario::new(util_name!()); diff --git a/tests/by-util/test_mkfifo.rs b/tests/by-util/test_mkfifo.rs index 13d43f5a59a..73e7200c24f 100644 --- a/tests/by-util/test_mkfifo.rs +++ b/tests/by-util/test_mkfifo.rs @@ -83,6 +83,18 @@ fn test_create_one_fifo_already_exists() { .stderr_is("mkfifo: cannot create fifo 'abcdef': File exists\n"); } +/// Unlike `chmod`/`mkdir`/`install`, GNU does not name the mode operand at +/// all here, regardless of what specifically was wrong with it. +#[test] +fn test_invalid_mode_is_not_named() { + for mode in ["999", "", "a", "u?rwx"] { + new_ucmd!() + .args(&["-m", mode, "probe"]) + .fails_with_code(1) + .stderr_only("mkfifo: invalid mode\n"); + } +} + #[test] fn test_create_fifo_with_mode_and_umask() { use uucore::fs::display_permissions; diff --git a/tests/by-util/test_mknod.rs b/tests/by-util/test_mknod.rs index 49fdbc243f9..885356d4c7c 100644 --- a/tests/by-util/test_mknod.rs +++ b/tests/by-util/test_mknod.rs @@ -135,7 +135,19 @@ fn test_mknod_invalid_mode() { .fails() .no_stdout() .code_is(1) - .stderr_contains("invalid mode"); + .stderr_only("mknod: invalid mode\n"); +} + +/// Unlike `chmod`/`mkdir`/`install`, GNU does not name the mode operand at +/// all here, regardless of what specifically was wrong with it. +#[test] +fn test_invalid_mode_is_not_named() { + for mode in ["999", "", "a", "u?rwx"] { + new_ucmd!() + .args(&["--mode", mode, "probe", "p"]) + .fails_with_code(1) + .stderr_only("mknod: invalid mode\n"); + } } #[test]