Merge pull request #2597 from miDeb/resolve-dangling

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

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

@@ -17,6 +17,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"))]
@@ -111,26 +112,42 @@ 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) {
-            break;
+            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) {
-        result.pop();
+            Ok(path) => {
-        result.push(path);
+                result.pop();
+                result.push(path);
+            }
+            Err(e) => return Err((result, e)),
+        }
+
+        if first_resolution.is_none() {
+            first_resolution = Some(result.clone());
+        }
     }
     Ok(result)
 }
@@ -216,11 +233,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);
                 }
             }
         }
@@ -232,14 +248,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"));
+}