Skip to content

Commit

Permalink
ls: add already listed message (#3707)
Browse files Browse the repository at this point in the history 
* ls: handle looping symlinks infinite printing

* ls: better coloring and printing symlinks when dereferenced

* tests/ls: add dereferencing and symlink loop tests

* ls: reformat changed using rustfmt

* ls: follow clippy advice for cleaner code

* uucore/fs: fix FileInformation to open directory handles in Windows as
well
  • Loading branch information
@niyaznigmatullin
niyaznigmatullin authored Jul 11, 2022
1 parent 8f4a326 commit da5808d
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 18 deletions.
76 changes: 59 additions & 17 deletions src/uu/ls/src/ls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use glob::Pattern;
use lscolors::LsColors;
use number_prefix::NumberPrefix;
use once_cell::unsync::OnceCell;
use std::collections::HashSet;
#[cfg(windows)]
use std::os::windows::fs::MetadataExt;
use std::{
Expand Down Expand Up @@ -151,6 +152,7 @@ enum LsError {
IOError(std::io::Error),
IOErrorContext(std::io::Error, PathBuf),
BlockSizeParseError(String),
AlreadyListedError(PathBuf),
}

impl UError for LsError {
Expand All @@ -160,6 +162,7 @@ impl UError for LsError {
LsError::IOError(_) => 1,
LsError::IOErrorContext(_, _) => 1,
LsError::BlockSizeParseError(_) => 1,
LsError::AlreadyListedError(_) => 2,
}
}
}
Expand Down Expand Up @@ -236,6 +239,13 @@ impl Display for LsError {
},
}
}
LsError::AlreadyListedError(path) => {
write!(
f,
"{}: not listing already-listed directory",
path.to_string_lossy()
)
}
}
}
}
Expand Down Expand Up @@ -1488,16 +1498,26 @@ impl PathData {

// Why prefer to check the DirEntry file_type()? B/c the call is
// nearly free compared to a metadata() call on a Path
let ft = match de {
Some(ref de) => {
if let Ok(ft_de) = de.file_type() {
OnceCell::from(Some(ft_de))
} else if let Ok(md_pb) = p_buf.metadata() {
OnceCell::from(Some(md_pb.file_type()))
} else {
OnceCell::new()
fn get_file_type(
de: &DirEntry,
p_buf: &Path,
must_dereference: bool,
) -> OnceCell<Option<FileType>> {
if must_dereference {
if let Ok(md_pb) = p_buf.metadata() {
return OnceCell::from(Some(md_pb.file_type()));
}
}
if let Ok(ft_de) = de.file_type() {
OnceCell::from(Some(ft_de))
} else if let Ok(md_pb) = p_buf.symlink_metadata() {
OnceCell::from(Some(md_pb.file_type()))
} else {
OnceCell::new()
}
}
let ft = match de {
Some(ref de) => get_file_type(de, &p_buf, must_dereference),
None => OnceCell::new(),
};

Expand Down Expand Up @@ -1619,7 +1639,12 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
writeln!(out, "\n{}:", path_data.p_buf.display())?;
}
}
enter_directory(path_data, read_dir, config, &mut out)?;
let mut listed_ancestors = HashSet::new();
listed_ancestors.insert(FileInformation::from_path(
&path_data.p_buf,
path_data.must_dereference,
)?);
enter_directory(path_data, read_dir, config, &mut out, &mut listed_ancestors)?;
}

Ok(())
Expand Down Expand Up @@ -1715,6 +1740,7 @@ fn enter_directory(
read_dir: ReadDir,
config: &Config,
out: &mut BufWriter<Stdout>,
listed_ancestors: &mut HashSet<FileInformation>,
) -> UResult<()> {
// Create vec of entries with initial dot files
let mut entries: Vec<PathData> = if config.files == Files::All {
Expand Down Expand Up @@ -1780,8 +1806,17 @@ fn enter_directory(
continue;
}
Ok(rd) => {
writeln!(out, "\n{}:", e.p_buf.display())?;
enter_directory(e, rd, config, out)?;
if !listed_ancestors
.insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?)
{
out.flush()?;
show!(LsError::AlreadyListedError(e.p_buf.clone()));
} else {
writeln!(out, "\n{}:", e.p_buf.display())?;
enter_directory(e, rd, config, out, listed_ancestors)?;
listed_ancestors
.remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?);
}
}
}
}
Expand Down Expand Up @@ -2254,6 +2289,7 @@ fn get_inode(metadata: &Metadata) -> String {
use std::sync::Mutex;
#[cfg(unix)]
use uucore::entries;
use uucore::fs::FileInformation;
use uucore::quoting_style;

#[cfg(unix)]
Expand Down Expand Up @@ -2518,12 +2554,17 @@ fn display_file_name(
let mut width = name.width();

if let Some(ls_colors) = &config.color {
name = color_name(
name,
&path.p_buf,
path.p_buf.symlink_metadata().ok().as_ref(),
ls_colors,
);
let md = path.md(out);
name = if md.is_some() {
color_name(name, &path.p_buf, md, ls_colors)
} else {
color_name(
name,
&path.p_buf,
path.p_buf.symlink_metadata().ok().as_ref(),
ls_colors,
)
};
}

if config.format != Format::Long && !more_info.is_empty() {
Expand Down Expand Up @@ -2564,6 +2605,7 @@ fn display_file_name(
if config.format == Format::Long
&& path.file_type(out).is_some()
&& path.file_type(out).unwrap().is_symlink()
&& !path.must_dereference
{
if let Ok(target) = path.p_buf.read_link() {
name.push_str(" -> ");
Expand Down
5 changes: 4 additions & 1 deletion src/uucore/src/lib/features/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,12 @@ impl FileInformation {
use std::fs::OpenOptions;
use std::os::windows::prelude::*;
let mut open_options = OpenOptions::new();
let mut custom_flags = 0;
if !dereference {
open_options.custom_flags(winapi::um::winbase::FILE_FLAG_OPEN_REPARSE_POINT);
custom_flags |= winapi::um::winbase::FILE_FLAG_OPEN_REPARSE_POINT;
}
custom_flags |= winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS;
open_options.custom_flags(custom_flags);
let file = open_options.read(true).open(path.as_ref())?;
Self::from_file(&file)
}
Expand Down
59 changes: 59 additions & 0 deletions tests/by-util/test_ls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3065,3 +3065,62 @@ fn test_ls_quoting_color() {
.succeeds()
.stdout_contains("\'need quoting\'");
}

#[test]
fn test_ls_dereference_looped_symlinks_recursive() {
let (at, mut ucmd) = at_and_ucmd!();

at.mkdir("loop");
at.relative_symlink_dir("../loop", "loop/sub");

ucmd.args(&["-RL", "loop"])
.fails()
.stderr_contains("not listing already-listed directory");
}

#[test]
fn test_dereference_dangling_color() {
let (at, mut ucmd) = at_and_ucmd!();
at.relative_symlink_file("wat", "nonexistent");
let out_exp = ucmd.args(&["--color"]).run().stdout_move_str();

let (at, mut ucmd) = at_and_ucmd!();
at.relative_symlink_file("wat", "nonexistent");
ucmd.args(&["-L", "--color"])
.fails()
.code_is(1)
.stderr_contains("No such file or directory")
.stdout_is(out_exp);
}

#[test]
fn test_dereference_symlink_dir_color() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("dir1");
at.mkdir("dir1/link");
let out_exp = ucmd.args(&["--color", "dir1"]).run().stdout_move_str();

let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("dir1");
at.mkdir("dir2");
at.relative_symlink_dir("../dir2", "dir1/link");
ucmd.args(&["-L", "--color", "dir1"])
.succeeds()
.stdout_is(out_exp);
}

#[test]
fn test_dereference_symlink_file_color() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("dir1");
at.touch("dir1/link");
let out_exp = ucmd.args(&["--color", "dir1"]).run().stdout_move_str();

let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("dir1");
at.touch("file");
at.relative_symlink_file("../file", "dir1/link");
ucmd.args(&["-L", "--color", "dir1"])
.succeeds()
.stdout_is(out_exp);
}

0 comments on commit da5808d

Please sign in to comment.