uucore/fs: use the latest resolution that did not fail

When we ignore failures resolving symbolic links we should keep the
latest resolution that did not fail, not the original path.

src/uucore/src/lib/features/fs.rs

@@ -15,6 +15,7 @@ use libc::{
 use std::borrow::Cow;
 use std::env;
 use std::fs;
+use std::io::Error as IOError;
 use std::io::Result as IOResult;
 use std::io::{Error, ErrorKind};
 #[cfg(any(unix, target_os = "redox"))]
@@ -109,27 +110,43 @@ pub fn normalize_path(path: &Path) -> PathBuf {
     ret
 }
 
-fn resolve<P: AsRef<Path>>(original: P) -> IOResult<PathBuf> {
+fn resolve<P: AsRef<Path>>(original: P) -> Result<PathBuf, (PathBuf, IOError)> {
     const MAX_LINKS_FOLLOWED: u32 = 255;
     let mut followed = 0;
     let mut result = original.as_ref().to_path_buf();
+
+    let mut first_resolution = None;
     loop {
         if followed == MAX_LINKS_FOLLOWED {
-            return Err(Error::new(
+            return Err((
-                ErrorKind::InvalidInput,
+                // When we hit MAX_LINKS_FOLLOWED we should return the first resolution (that's what GNU does - for whatever reason)
-                "maximum links followed",
+                first_resolution.unwrap(),
+                Error::new(ErrorKind::InvalidInput, "maximum links followed"),
             ));
         }
 
-        if !fs::symlink_metadata(&result)?.file_type().is_symlink() {
+        match fs::symlink_metadata(&result) {
+            Ok(meta) => {
+                if !meta.file_type().is_symlink() {
                     break;
                 }
+            }
+            Err(e) => return Err((result, e)),
+        }
 
         followed += 1;
-        let path = fs::read_link(&result)?;
+        match fs::read_link(&result) {
+            Ok(path) => {
                 result.pop();
                 result.push(path);
             }
+            Err(e) => return Err((result, e)),
+        }
+
+        if first_resolution.is_none() {
+            first_resolution = Some(result.clone());
+        }
+    }
     Ok(result)
 }
 
@@ -214,11 +231,10 @@ pub fn canonicalize<P: AsRef<Path>>(
             }
 
             match resolve(&result) {
-                Err(_) if miss_mode == MissingHandling::Missing => continue,
+                Err((path, _)) if miss_mode == MissingHandling::Missing => result = path,
-                Err(e) => return Err(e),
+                Err((_, e)) => return Err(e),
                 Ok(path) => {
-                    result.pop();
+                    result = path;
-                    result.push(path);
                 }
             }
         }
@@ -230,14 +246,12 @@ pub fn canonicalize<P: AsRef<Path>>(
         }
 
         match resolve(&result) {
-            Err(e) if miss_mode == MissingHandling::Existing => {
+            Err((_, e)) if miss_mode == MissingHandling::Existing => {
                 return Err(e);
             }
-            Ok(path) => {
+            Ok(path) | Err((path, _)) => {
-                result.pop();
+                result = path;
-                result.push(path);
             }
-            Err(_) => (),
         }
         if res_mode == ResolveMode::Physical {
             result = normalize_path(&result);

tests/by-util/test_realpath.rs

@@ -139,3 +139,23 @@ fn test_realpath_logical_mode() {
         .succeeds()
         .stdout_contains("dir1\n");
 }
+
+#[test]
+fn test_realpath_dangling() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink_file("nonexistent-file", "link");
+    ucmd.arg("link")
+        .succeeds()
+        .stdout_only(at.plus_as_string("nonexistent-file\n"));
+}
+
+#[test]
+fn test_realpath_loop() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink_file("2", "1");
+    at.symlink_file("3", "2");
+    at.symlink_file("1", "3");
+    ucmd.arg("1")
+        .succeeds()
+        .stdout_only(at.plus_as_string("2\n"));
+}