Merge pull request #3826 from niyaznigmatullin/canonicalize_trailing_slash

readlink: GNU compatibility

src/uu/readlink/src/readlink.rs

@@ -36,7 +36,7 @@ const ARG_FILES: &str = "files";
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().get_matches_from(args);
 
-    let mut no_newline = matches.contains_id(OPT_NO_NEWLINE);
+    let mut no_trailing_delimiter = matches.contains_id(OPT_NO_NEWLINE);
     let use_zero = matches.contains_id(OPT_ZERO);
     let silent = matches.contains_id(OPT_SILENT) || matches.contains_id(OPT_QUIET);
     let verbose = matches.contains_id(OPT_VERBOSE);
@@ -66,9 +66,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         return Err(UUsageError::new(1, "missing operand"));
     }
 
-    if no_newline && files.len() > 1 && !silent {
+    if no_trailing_delimiter && files.len() > 1 && !silent {
         show_error!("ignoring --no-newline with multiple arguments");
-        no_newline = false;
+        no_trailing_delimiter = false;
     }
 
     for f in &files {
@@ -79,7 +79,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             canonicalize(&p, can_mode, res_mode)
         };
         match path_result {
-            Ok(path) => show(&path, no_newline, use_zero).map_err_context(String::new)?,
+            Ok(path) => {
+                show(&path, no_trailing_delimiter, use_zero).map_err_context(String::new)?;
+            }
             Err(err) => {
                 if verbose {
                     return Err(USimpleError::new(
@@ -167,12 +169,12 @@ pub fn uu_app<'a>() -> Command<'a> {
         )
 }
 
-fn show(path: &Path, no_newline: bool, use_zero: bool) -> std::io::Result<()> {
+fn show(path: &Path, no_trailing_delimiter: bool, use_zero: bool) -> std::io::Result<()> {
     let path = path.to_str().unwrap();
-    if use_zero {
+    if no_trailing_delimiter {
-        print!("{}\0", path);
-    } else if no_newline {
         print!("{}", path);
+    } else if use_zero {
+        print!("{}\0", path);
     } else {
         println!("{}", path);
     }

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

@@ -22,11 +22,12 @@ use std::collections::VecDeque;
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
+use std::fs::read_dir;
 use std::hash::Hash;
 use std::io::{Error, ErrorKind, Result as IOResult};
 #[cfg(unix)]
 use std::os::unix::{fs::MetadataExt, io::AsRawFd};
-use std::path::{Component, Path, PathBuf};
+use std::path::{Component, Path, PathBuf, MAIN_SEPARATOR};
 #[cfg(target_os = "windows")]
 use winapi_util::AsHandleRef;
 
@@ -318,6 +319,11 @@ pub fn canonicalize<P: AsRef<Path>>(
 ) -> IOResult<PathBuf> {
     const SYMLINKS_TO_LOOK_FOR_LOOPS: i32 = 20;
     let original = original.as_ref();
+    let has_to_be_directory =
+        (miss_mode == MissingHandling::Normal || miss_mode == MissingHandling::Existing) && {
+            let path_str = original.to_string_lossy();
+            path_str.ends_with(MAIN_SEPARATOR) || path_str.ends_with('/')
+        };
     let original = if original.is_absolute() {
         original.to_path_buf()
     } else {
@@ -383,6 +389,24 @@ pub fn canonicalize<P: AsRef<Path>>(
             _ => {}
         }
     }
+    // raise Not a directory if required
+    match miss_mode {
+        MissingHandling::Existing => {
+            if has_to_be_directory {
+                read_dir(&result)?;
+            }
+        }
+        MissingHandling::Normal => {
+            if result.exists() {
+                if has_to_be_directory {
+                    read_dir(&result)?;
+                }
+            } else if let Some(parent) = result.parent() {
+                read_dir(parent)?;
+            }
+        }
+        _ => {}
+    }
     Ok(result)
 }
 

tests/by-util/test_readlink.rs

@@ -1,7 +1,13 @@
+// spell-checker:ignore regfile
 use crate::common::util::*;
 
 static GIBBERISH: &str = "supercalifragilisticexpialidocious";
 
+#[cfg(not(windows))]
+static NOT_A_DIRECTORY: &str = "Not a directory";
+#[cfg(windows)]
+static NOT_A_DIRECTORY: &str = "The directory name is invalid.";
+
 #[test]
 fn test_resolve() {
     let scene = TestScenario::new(util_name!());
@@ -80,3 +86,289 @@ fn test_symlink_to_itself_verbose() {
         .code_is(1)
         .stderr_contains("Too many levels of symbolic links");
 }
+
+#[test]
+fn test_trailing_slash_regular_file() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("regfile");
+    scene
+        .ucmd()
+        .args(&["-ev", "./regfile/"])
+        .fails()
+        .code_is(1)
+        .stderr_contains(NOT_A_DIRECTORY)
+        .no_stdout();
+    scene
+        .ucmd()
+        .args(&["-e", "./regfile"])
+        .succeeds()
+        .stdout_contains("regfile");
+}
+
+#[test]
+fn test_trailing_slash_symlink_to_regular_file() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("regfile");
+    at.relative_symlink_file("regfile", "link");
+    scene
+        .ucmd()
+        .args(&["-ev", "./link/"])
+        .fails()
+        .code_is(1)
+        .stderr_contains(NOT_A_DIRECTORY)
+        .no_stdout();
+    scene
+        .ucmd()
+        .args(&["-e", "./link"])
+        .succeeds()
+        .stdout_contains("regfile");
+    scene
+        .ucmd()
+        .args(&["-e", "./link/more"])
+        .fails()
+        .code_is(1)
+        .no_stdout();
+}
+
+#[test]
+fn test_trailing_slash_directory() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("directory");
+    for query in ["./directory", "./directory/"] {
+        scene
+            .ucmd()
+            .args(&["-e", query])
+            .succeeds()
+            .stdout_contains("directory");
+    }
+}
+
+#[test]
+fn test_trailing_slash_symlink_to_directory() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("directory");
+    at.relative_symlink_dir("directory", "link");
+    for query in ["./link", "./link/"] {
+        scene
+            .ucmd()
+            .args(&["-e", query])
+            .succeeds()
+            .stdout_contains("directory");
+    }
+    scene
+        .ucmd()
+        .args(&["-ev", "./link/more"])
+        .fails()
+        .code_is(1)
+        .stderr_contains("No such file or directory")
+        .no_stdout();
+}
+
+#[test]
+fn test_trailing_slash_symlink_to_missing() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("subdir");
+    at.relative_symlink_file("missing", "link");
+    at.relative_symlink_file("subdir/missing", "link2");
+    for query in [
+        "missing",
+        "./missing/",
+        "link",
+        "./link/",
+        "link/more",
+        "link2",
+        "./link2/",
+        "link2/more",
+    ] {
+        scene
+            .ucmd()
+            .args(&["-ev", query])
+            .fails()
+            .code_is(1)
+            .stderr_contains("No such file or directory")
+            .no_stdout();
+    }
+}
+
+#[test]
+fn test_canonicalize_trailing_slash_regfile() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("regfile");
+    at.relative_symlink_file("regfile", "link1");
+    for name in ["regfile", "link1"] {
+        scene
+            .ucmd()
+            .args(&["-f", name])
+            .succeeds()
+            .stdout_contains("regfile");
+        scene
+            .ucmd()
+            .args(&["-fv", &format!("./{}/", name)])
+            .fails()
+            .code_is(1)
+            .stderr_contains(NOT_A_DIRECTORY)
+            .no_stdout();
+        scene
+            .ucmd()
+            .args(&["-fv", &format!("{}/more", name)])
+            .fails()
+            .code_is(1)
+            .stderr_contains(NOT_A_DIRECTORY)
+            .no_stdout();
+        scene
+            .ucmd()
+            .args(&["-fv", &format!("./{}/more/", name)])
+            .fails()
+            .code_is(1)
+            .stderr_contains(NOT_A_DIRECTORY)
+            .no_stdout();
+    }
+}
+
+#[test]
+fn test_canonicalize_trailing_slash_subdir() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("subdir");
+    at.relative_symlink_dir("subdir", "link2");
+    for name in ["subdir", "link2"] {
+        scene
+            .ucmd()
+            .args(&["-f", name])
+            .succeeds()
+            .stdout_contains("subdir");
+        scene
+            .ucmd()
+            .args(&["-f", &format!("./{}/", name)])
+            .succeeds()
+            .stdout_contains("subdir");
+        scene
+            .ucmd()
+            .args(&["-f", &format!("{}/more", name)])
+            .succeeds()
+            .stdout_contains(path_concat!("subdir", "more"));
+        scene
+            .ucmd()
+            .args(&["-f", &format!("./{}/more/", name)])
+            .succeeds()
+            .stdout_contains(path_concat!("subdir", "more"));
+        scene
+            .ucmd()
+            .args(&["-f", &format!("{}/more/more2", name)])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+        scene
+            .ucmd()
+            .args(&["-f", &format!("./{}/more/more2/", name)])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+    }
+}
+
+#[test]
+fn test_canonicalize_trailing_slash_missing() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.relative_symlink_file("missing", "link3");
+    for name in ["missing", "link3"] {
+        scene
+            .ucmd()
+            .args(&["-f", name])
+            .succeeds()
+            .stdout_contains("missing");
+        scene
+            .ucmd()
+            .args(&["-f", &format!("./{}/", name)])
+            .succeeds()
+            .stdout_contains("missing");
+        scene
+            .ucmd()
+            .args(&["-f", &format!("{}/more", name)])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+        scene
+            .ucmd()
+            .args(&["-f", &format!("./{}/more/", name)])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+    }
+}
+
+#[test]
+fn test_canonicalize_trailing_slash_subdir_missing() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("subdir");
+    at.relative_symlink_file("subdir/missing", "link4");
+    for query in ["link4", "./link4/"] {
+        scene
+            .ucmd()
+            .args(&["-f", query])
+            .succeeds()
+            .stdout_contains(path_concat!("subdir", "missing"));
+    }
+    for query in ["link4/more", "./link4/more/"] {
+        scene
+            .ucmd()
+            .args(&["-f", query])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+    }
+}
+
+#[test]
+fn test_canonicalize_trailing_slash_symlink_loop() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.relative_symlink_file("link5", "link5");
+    for query in ["link5", "./link5/", "link5/more", "./link5/more/"] {
+        scene
+            .ucmd()
+            .args(&["-f", query])
+            .fails()
+            .code_is(1)
+            .no_stdout();
+    }
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_delimiters() {
+    new_ucmd!()
+        .args(&["--zero", "-n", "-m", "/a"])
+        .succeeds()
+        .stdout_only("/a");
+    new_ucmd!()
+        .args(&["-n", "-m", "/a"])
+        .succeeds()
+        .stdout_only("/a");
+    new_ucmd!()
+        .args(&["--zero", "-m", "/a"])
+        .succeeds()
+        .stdout_only("/a\0");
+    new_ucmd!()
+        .args(&["-m", "/a"])
+        .succeeds()
+        .stdout_only("/a\n");
+    new_ucmd!()
+        .args(&["--zero", "-n", "-m", "/a", "/a"])
+        .succeeds()
+        .stderr_contains("ignoring --no-newline with multiple arguments")
+        .stdout_is("/a\0/a\0");
+    new_ucmd!()
+        .args(&["-n", "-m", "/a", "/a"])
+        .succeeds()
+        .stderr_contains("ignoring --no-newline with multiple arguments")
+        .stdout_is("/a\n/a\n");
+}

tests/by-util/test_realpath.rs

@@ -364,3 +364,92 @@ fn test_relative() {
         .succeeds()
         .stdout_is(".\nusr\n"); // spell-checker:disable-line
 }
+
+#[test]
+fn test_realpath_trailing_slash() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("file");
+    at.mkdir("dir");
+    at.relative_symlink_file("file", "link_file");
+    at.relative_symlink_dir("dir", "link_dir");
+    at.relative_symlink_dir("no_dir", "link_no_dir");
+    scene
+        .ucmd()
+        .arg("link_file")
+        .succeeds()
+        .stdout_contains(format!("{}file\n", std::path::MAIN_SEPARATOR));
+    scene.ucmd().arg("link_file/").fails().code_is(1);
+    scene
+        .ucmd()
+        .arg("link_dir")
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .arg("link_dir/")
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .arg("link_no_dir")
+        .succeeds()
+        .stdout_contains(format!("{}no_dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .arg("link_no_dir/")
+        .succeeds()
+        .stdout_contains(format!("{}no_dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-e", "link_file"])
+        .succeeds()
+        .stdout_contains(format!("{}file\n", std::path::MAIN_SEPARATOR));
+    scene.ucmd().args(&["-e", "link_file/"]).fails().code_is(1);
+    scene
+        .ucmd()
+        .args(&["-e", "link_dir"])
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-e", "link_dir/"])
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene.ucmd().args(&["-e", "link_no_dir"]).fails().code_is(1);
+    scene
+        .ucmd()
+        .args(&["-e", "link_no_dir/"])
+        .fails()
+        .code_is(1);
+    scene
+        .ucmd()
+        .args(&["-m", "link_file"])
+        .succeeds()
+        .stdout_contains(format!("{}file\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-m", "link_file/"])
+        .succeeds()
+        .stdout_contains(format!("{}file\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-m", "link_dir"])
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-m", "link_dir/"])
+        .succeeds()
+        .stdout_contains(format!("{}dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-m", "link_no_dir"])
+        .succeeds()
+        .stdout_contains(format!("{}no_dir\n", std::path::MAIN_SEPARATOR));
+    scene
+        .ucmd()
+        .args(&["-m", "link_no_dir/"])
+        .succeeds()
+        .stdout_contains(format!("{}no_dir\n", std::path::MAIN_SEPARATOR));
+}