From 75e5c42e4038d79bb0fbc69a22815a8042d2a699 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 10 Sep 2021 08:15:23 +0200 Subject: [PATCH] chown: support '.' as 'user.group' separator (like ':') (#2638) * chown: support '.' as 'user.group' separator (like ':') It also manages the case: user.name:group (valid too) Should fix: gnu/tests/chown/separator.sh --- src/uu/chown/src/chown.rs | 65 ++++++++++++++++------ tests/by-util/test_chown.rs | 106 ++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 18 deletions(-) diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 5525f9325..f9412a768 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -5,7 +5,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) COMFOLLOW Passwd RFILE RFILE's derefer dgid duid +// spell-checker:ignore (ToDO) COMFOLLOW Passwd RFILE RFILE's derefer dgid duid groupname #[macro_use] extern crate uucore; @@ -31,7 +31,7 @@ fn get_usage() -> String { fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Option, IfFrom)> { let filter = if let Some(spec) = matches.value_of(options::FROM) { - match parse_spec(spec)? { + match parse_spec(spec, ':')? { (Some(uid), None) => IfFrom::User(uid), (None, Some(gid)) => IfFrom::Group(gid), (Some(uid), Some(gid)) => IfFrom::UserGroup(uid, gid), @@ -49,7 +49,7 @@ fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Optio dest_gid = Some(meta.gid()); dest_uid = Some(meta.uid()); } else { - let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap())?; + let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap(), ':')?; dest_uid = u; dest_gid = g; } @@ -166,23 +166,49 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn parse_spec(spec: &str) -> UResult<(Option, Option)> { - let args = spec.split_terminator(':').collect::>(); - let usr_only = args.len() == 1 && !args[0].is_empty(); - let grp_only = args.len() == 2 && args[0].is_empty(); - let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty(); - let uid = if usr_only || usr_grp { - Some( - Passwd::locate(args[0]) - .map_err(|_| USimpleError::new(1, format!("invalid user: {}", spec.quote())))? - .uid(), - ) +/// Parse the username and groupname +/// +/// In theory, it should be username:groupname +/// but ... +/// it can user.name:groupname +/// or username.groupname +/// +/// # Arguments +/// +/// * `spec` - The input from the user +/// * `sep` - Should be ':' or '.' +fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { + assert!(['.', ':'].contains(&sep)); + let mut args = spec.splitn(2, sep); + let user = args.next().unwrap_or(""); + let group = args.next().unwrap_or(""); + + let uid = if !user.is_empty() { + Some(match Passwd::locate(user) { + Ok(u) => u.uid(), // We have been able to get the uid + Err(_) => + // we have NOT been able to find the uid + // but we could be in the case where we have user.group + { + if spec.contains('.') && !spec.contains(':') && sep == ':' { + // but the input contains a '.' but not a ':' + // we might have something like username.groupname + // So, try to parse it this way + return parse_spec(spec, '.'); + } else { + return Err(USimpleError::new( + 1, + format!("invalid user: {}", spec.quote()), + ))?; + } + } + }) } else { None }; - let gid = if grp_only || usr_grp { + let gid = if !group.is_empty() { Some( - Group::locate(args[1]) + Group::locate(group) .map_err(|_| USimpleError::new(1, format!("invalid group: {}", spec.quote())))? .gid(), ) @@ -198,7 +224,10 @@ mod test { #[test] fn test_parse_spec() { - assert!(matches!(parse_spec(":"), Ok((None, None)))); - assert!(format!("{}", parse_spec("::").err().unwrap()).starts_with("invalid group: ")); + assert!(matches!(parse_spec(":", ':'), Ok((None, None)))); + assert!(matches!(parse_spec(".", ':'), Ok((None, None)))); + assert!(matches!(parse_spec(".", '.'), Ok((None, None)))); + assert!(format!("{}", parse_spec("::", ':').err().unwrap()).starts_with("invalid group: ")); + assert!(format!("{}", parse_spec("..", ':').err().unwrap()).starts_with("invalid group: ")); } } diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 725e4b244..d5ebb4600 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -139,6 +139,14 @@ fn test_chown_only_owner_colon() { .succeeds() .stderr_contains(&"retained as"); + scene + .ucmd() + .arg(format!("{}.", user_name)) + .arg("--verbose") + .arg(file1) + .succeeds() + .stderr_contains(&"retained as"); + scene .ucmd() .arg("root:") @@ -180,6 +188,14 @@ fn test_chown_only_colon() { .arg(file1) .fails() .stderr_contains(&"invalid group: '::'"); + + scene + .ucmd() + .arg("..") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group: '..'"); } #[test] @@ -232,6 +248,22 @@ fn test_chown_owner_group() { } result.stderr_contains(&"retained as"); + scene + .ucmd() + .arg("root:root:root") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group"); + + scene + .ucmd() + .arg("root.root.root") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group"); + // TODO: on macos group name is not recognized correctly: "chown: invalid group: 'root:root' #[cfg(any(windows, all(unix, not(target_os = "macos"))))] scene @@ -243,6 +275,67 @@ fn test_chown_owner_group() { .stderr_contains(&"failed to change"); } +#[test] +// FixME: Fails on freebsd because of chown: invalid group: 'root:root' +#[cfg(not(target_os = "freebsd"))] +fn test_chown_various_input() { + // test chown username:group file.txt + + 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_file1"; + 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()); + + 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(&"retained as"); + + // check that username.groupname is understood + 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(&"retained as"); + + // Fails as user.name doesn't exist in the CI + // but it is valid + scene + .ucmd() + .arg(format!("{}:{}", "user.name", "groupname")) + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"chown: invalid user: 'user.name:groupname'"); +} + #[test] // FixME: on macos & freebsd group name is not recognized correctly: "chown: invalid group: ':groupname' #[cfg(any( @@ -405,6 +498,19 @@ fn test_chown_owner_group_id() { } result.stderr_contains(&"retained as"); + let result = scene + .ucmd() + .arg(format!("{}.{}", user_id, group_id)) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "invalid user") { + // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" + // stderr: "chown: invalid user: '1001.116' + return; + } + result.stderr_contains(&"retained as"); + scene .ucmd() .arg("0:0")