Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ls: panicking on dangling symlink with --color=auto -l #6346

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/uu/ls/src/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3353,10 +3353,9 @@ fn color_name(
// use the optional target_symlink
// Use fn get_metadata_with_deref_opt instead of get_metadata() here because ls
// should not exit with an err, if we are unable to obtain the target_metadata
let md = get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference)
.unwrap_or_else(|_| target.get_metadata(out).unwrap().clone());

apply_style_based_on_metadata(path, Some(&md), ls_colors, style_manager, &name)
let md_res = get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference);
let md = md_res.or(path.p_buf.symlink_metadata());
apply_style_based_on_metadata(path, md.ok().as_ref(), ls_colors, style_manager, &name)
} else {
let md_option = path.get_metadata(out);
let symlink_metadata = path.p_buf.symlink_metadata().ok();
Expand Down
39 changes: 38 additions & 1 deletion tests/by-util/test_ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef
// spell-checker:ignore (words) READMECAREFULLY birthtime doesntexist oneline somebackup lrwx somefile somegroup somehiddenbackup somehiddenfile tabsize aaaaaaaa bbbb cccc dddddddd ncccc neee naaaaa nbcdef nfffff dired subdired tmpfs mdir COLORTERM mexe bcdef mfoo

#[cfg(any(unix, feature = "feat_selinux"))]
use crate::common::util::expected_result;
Expand Down Expand Up @@ -1376,6 +1376,43 @@ fn test_ls_long_symlink_color() {
}
}

/// This test is for "ls -l --color=auto|--color=always"
/// We use "--color=always" as the colors are the same regardless of the color option being "auto" or "always"
/// using 'ln' to create a dangling symlink
/// Only tests whether the specific color of the target and the dangling_symlink are equal
BenWiederhake marked this conversation as resolved.
Show resolved Hide resolved
/// Modified version of the "test_ls_long_symlink_color" test. Please see that for more details
#[test]
fn test_ls_long_dangling_symlink_color() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;

at.mkdir("dir1");
at.symlink_dir("foo", "dir1/dangling_symlink");
let result = ts
.ucmd()
.arg("-l")
.arg("--color=always")
.arg("dir1/dangling_symlink")
.succeeds();

let stdout = result.stdout_str();
// stdout contains output like in the below sequence. We match for the color i.e. 01;36
// \x1b[0m\x1b[01;36mdir1/dangling_symlink\x1b[0m -> \x1b[01;36mfoo\x1b[0m
let color_regex = Regex::new(r"(\d\d;)\d\dm").unwrap();
// colors_vec[0] contains the symlink color and style and colors_vec[1] contains the color and style of the file the
// symlink points to.
let colors_vec: Vec<_> = color_regex
.find_iter(stdout)
.map(|color| color.as_str())
.collect();
// constructs the string of file path with the color code
let symlink_color_name = colors_vec[0].to_owned() + "dir1/dangling_symlink\x1b";
let target_color_name = colors_vec[1].to_owned() + at.plus_as_string("foo\x1b").as_str();

assert!(stdout.contains(&symlink_color_name));
assert!(stdout.contains(&target_color_name));
}

#[test]
fn test_ls_long_total_size() {
let scene = TestScenario::new(util_name!());
Expand Down
Loading