From f6cb1324b630b8b0208efadc789d117557a81189 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 20 Jun 2021 13:19:50 +0530 Subject: [PATCH 1/5] ls: Fix problems dealing with dangling symlinks - For dangling symlinks, errors should only be reported if dereferencing options were passed and dereferencing was applicable to the particular symlink - With -i parameter, report '?' as the inode number for dangling symlinks --- src/uu/ls/src/ls.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0bffa2e52..677556ab0 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1196,7 +1196,9 @@ fn list(locs: Vec, config: Config) -> i32 { for loc in &locs { let p = PathBuf::from(&loc); - if !p.exists() { + let path_data = PathData::new(p, None, None, &config, true); + + if !path_data.md().is_some() { show_error!("'{}': {}", &loc, "No such file or directory"); /* We found an error, the return code of ls should not be 0 @@ -1206,8 +1208,6 @@ fn list(locs: Vec, config: Config) -> i32 { continue; } - let path_data = PathData::new(p, None, None, &config, true); - let show_dir_contents = match path_data.file_type() { Some(ft) => !config.directory && ft.is_dir(), None => { @@ -1331,7 +1331,7 @@ fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { if dereference { - entry.metadata().or_else(|_| entry.symlink_metadata()) + entry.metadata() } else { entry.symlink_metadata() } @@ -1733,7 +1733,11 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { #[cfg(unix)] { if config.format != Format::Long && config.inode { - name = get_inode(path.md()?) + " " + &name; + name = path + .md() + .map_or_else(|| "?".to_string(), |md| get_inode(md)) + + " " + + &name; } } From d0039df8c3de451a07a52dc96f1d6b4d71181817 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 20 Jun 2021 13:50:38 +0530 Subject: [PATCH 2/5] tests: Add test for dangling symlinks with ls Add test similar to gnu dangling symlinks test --- tests/by-util/test_ls.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f8aa4453b..741a304e3 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2021,3 +2021,28 @@ fn test_ls_path() { .run() .stdout_is(expected_stdout); } + +#[test] +fn test_ls_dangling_symlinks() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("temp_dir"); + at.symlink_file("does_not_exist", "temp_dir/dangle"); + + scene.ucmd().arg("-L").arg("temp_dir/dangle").fails(); + scene.ucmd().arg("-H").arg("temp_dir/dangle").fails(); + + scene + .ucmd() + .arg("temp_dir/dangle") + .succeeds() + .stdout_contains("dangle"); + + scene + .ucmd() + .arg("-Li") + .arg("temp_dir") + .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display + .stdout_contains("? dangle"); +} From 3b641afadcf57facf160b57c5e90bfeeeb68bed7 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 20 Jun 2021 16:56:25 +0530 Subject: [PATCH 3/5] ls: Fix issue with Windows and dangling symbolic links - Windows hidden file attribute determination would assume symbolic link to be valid and would panic - Check symbolic link's attributes if the link points to non-existing file --- src/uu/ls/src/ls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 677556ab0..220eccb30 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1270,7 +1270,8 @@ fn sort_entries(entries: &mut Vec, config: &Config) { #[cfg(windows)] fn is_hidden(file_path: &DirEntry) -> bool { - let metadata = fs::metadata(file_path.path()).unwrap(); + let path = file_path.path(); + let metadata = fs::metadata(&path).unwrap_or_else(|_| fs::symlink_metadata(&path).unwrap()); let attr = metadata.file_attributes(); (attr & 0x2) > 0 } From ffb6b7152f8699b0a42fe811a576a6a3633f4baf Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 20 Jun 2021 16:58:28 +0530 Subject: [PATCH 4/5] tests: Fix ls dangling symbolic links test output for windows On windows we do not print inode numbers at all, so skip checking for ? for dangling symbolic links in expected output --- tests/by-util/test_ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 741a304e3..67112b4f5 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2044,5 +2044,5 @@ fn test_ls_dangling_symlinks() { .arg("-Li") .arg("temp_dir") .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display - .stdout_contains("? dangle"); + .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); } From 4b3224dd82d2f80bdca5598675f397c5577a568d Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 21 Jun 2021 20:29:22 +0530 Subject: [PATCH 5/5] ls: Fix clippy warning --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 220eccb30..1d050a376 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1198,7 +1198,7 @@ fn list(locs: Vec, config: Config) -> i32 { let p = PathBuf::from(&loc); let path_data = PathData::new(p, None, None, &config, true); - if !path_data.md().is_some() { + if path_data.md().is_none() { show_error!("'{}': {}", &loc, "No such file or directory"); /* We found an error, the return code of ls should not be 0