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

@@ -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<u32>, Option<u32>, 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<u32>, 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<u32>, Option<u32>)> {
+/// Parse the username and groupname
-    let args = spec.split_terminator(':').collect::<Vec<_>>();
+///
-    let usr_only = args.len() == 1 && !args[0].is_empty();
+/// In theory, it should be username:groupname
-    let grp_only = args.len() == 2 && args[0].is_empty();
+/// but ...
-    let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty();
+/// it can user.name:groupname
-    let uid = if usr_only || usr_grp {
+/// or username.groupname
-        Some(
+///
-            Passwd::locate(args[0])
+/// # Arguments
-                .map_err(|_| USimpleError::new(1, format!("invalid user: {}", spec.quote())))?
+///
-                .uid(),
+/// * `spec` - The input from the user
-        )
+/// * `sep` - Should be ':' or '.'
+fn parse_spec(spec: &str, sep: char) -> UResult<(Option<u32>, Option<u32>)> {
+    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!(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!(format!("{}", parse_spec("::", ':').err().unwrap()).starts_with("invalid group: "));
+        assert!(format!("{}", parse_spec("..", ':').err().unwrap()).starts_with("invalid group: "));
     }
 }

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")