diff --git a/src/uu/chown/locales/en-US.ftl b/src/uu/chown/locales/en-US.ftl index 0dfe8301e9d..9bcb725a6c3 100644 --- a/src/uu/chown/locales/en-US.ftl +++ b/src/uu/chown/locales/en-US.ftl @@ -21,3 +21,6 @@ chown-error-failed-to-get-attributes = failed to get attributes of { $file } chown-error-invalid-user = invalid user: { $user } chown-error-invalid-group = invalid group: { $group } chown-error-invalid-spec = invalid spec: { $spec } + +# Warning messages +chown-warning-dot-separator = '.' should be ':': { $spec } diff --git a/src/uu/chown/locales/fr-FR.ftl b/src/uu/chown/locales/fr-FR.ftl index 48e39853a3e..deaa7a620e9 100644 --- a/src/uu/chown/locales/fr-FR.ftl +++ b/src/uu/chown/locales/fr-FR.ftl @@ -21,3 +21,6 @@ chown-error-failed-to-get-attributes = échec de l'obtention des attributs de { chown-error-invalid-user = utilisateur invalide : { $user } chown-error-invalid-group = groupe invalide : { $group } chown-error-invalid-spec = spécification invalide : { $spec } + +# Messages d'avertissement +chown-warning-dot-separator = '.' devrait être ':' : { $spec } diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 6f0599ce811..b3de9eec6fa 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -9,6 +9,7 @@ use uucore::display::Quotable; pub use uucore::entries::{self, Group, Locate, Passwd}; use uucore::format_usage; use uucore::perms::{GidUidOwnerFilter, IfFrom, chown_base, options}; +use uucore::show_warning; use uucore::translate; use uucore::error::{FromIo, UResult, USimpleError}; @@ -151,7 +152,7 @@ pub fn uu_app() -> Command { } /// Parses the user string to extract the UID. -fn parse_uid(user: &str, spec: &str, sep: char) -> UResult> { +fn parse_uid(user: &str, spec: &str) -> UResult> { if user.is_empty() { return Ok(None); } @@ -160,11 +161,6 @@ fn parse_uid(user: &str, spec: &str, sep: char) -> UResult> { return Ok(Some(u.uid)); } - // Handle `username.groupname` syntax (e.g. when sep is ':' but spec contains '.') - if spec.contains('.') && !spec.contains(':') && sep == ':' { - return parse_spec(spec, '.').map(|(uid, _)| uid); - } - // Fallback: `user` string contains a numeric user ID user.parse().map(Some).map_err(|_| { USimpleError::new( @@ -209,7 +205,23 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { let user = args.next().unwrap_or(""); let group = args.next().unwrap_or(""); - let uid = parse_uid(user, spec, sep)?; + // Handle `owner.group` syntax: when the separator is ':' but the spec + // contains '.' and not ':', the full spec is treated as the user field. + // If that user lookup fails, re-parse with '.' as separator (like GNU does) + // so that both owner and group are applied. + if sep == ':' && !spec.contains(':') && spec.contains('.') { + if let Ok(uid) = parse_uid(user, spec) { + let gid = parse_gid(group, spec)?; + return Ok((uid, gid)); + } + show_warning!( + "{}", + translate!("chown-warning-dot-separator", "spec" => spec.quote()) + ); + return parse_spec(spec, '.'); + } + + let uid = parse_uid(user, spec)?; let gid = parse_gid(group, spec)?; if user.chars().next().is_some_and(char::is_numeric) && group.is_empty() && spec != user { diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 2602aabde06..3ac8a6d7c78 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -149,7 +149,8 @@ fn test_chown_only_owner_colon() { .arg("--verbose") .arg(file1) .succeeds() - .stderr_contains("retained as"); + .stderr_contains("retained as") + .stderr_contains("warning: '.' should be ':'"); scene .ucmd() @@ -160,6 +161,68 @@ fn test_chown_only_owner_colon() { .stderr_contains("failed to change"); } +#[test] +fn test_chown_dot_separator_warning() { + // test that using '.' as separator emits a warning + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let result = scene.cmd("whoami").run(); + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { + return; + } + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); + + let file1 = "test_chown_dot_warn"; + at.touch(file1); + + let result = scene.cmd("id").arg("-gn").run(); + if skipping_test_is_okay(&result, "id: cannot find name for group ID") { + return; + } + let group_name = String::from(result.stdout_str().trim()); + assert!(!group_name.is_empty()); + + // chown user. file should warn about '.' separator + scene + .ucmd() + .arg(format!("{user_name}.")) + .arg(file1) + .succeeds() + .stderr_contains("warning: '.' should be ':'"); + + // chown user.group file should warn AND apply both owner and group + let result = scene + .ucmd() + .arg(format!("{user_name}.{group_name}")) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "chown: invalid group:") { + return; + } + result.stderr_contains("warning: '.' should be ':'"); + // verbose output confirms both owner and group were processed; + // "retained as" on Linux (file group matches id -gn), "changed ownership" + // on BSDs where files inherit group from parent dir (e.g. /tmp -> wheel) + assert!( + result.stderr_str().contains("retained as") + || result.stderr_str().contains("changed ownership"), + "expected verbose ownership output, got: {}", + result.stderr_str() + ); + + // chown user: file should NOT warn + scene + .ucmd() + .arg(format!("{user_name}:")) + .arg(file1) + .succeeds() + .stderr_does_not_contain("warning"); +} + #[test] fn test_chown_only_colon() { // test chown : file.txt