Merge pull request #5278 from sylvestre/ls-no-read

ls -l: show an error when symlink not readable
This commit is contained in:
Daniel Hofstetter 2023-09-16 15:15:36 +02:00 committed by GitHub
commit fe4def8ac0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 71 additions and 43 deletions

View file

@ -2908,55 +2908,64 @@ fn display_file_name(
&& path.file_type(out).unwrap().is_symlink() && path.file_type(out).unwrap().is_symlink()
&& !path.must_dereference && !path.must_dereference
{ {
if let Ok(target) = path.p_buf.read_link() { match path.p_buf.read_link() {
name.push_str(" -> "); Ok(target) => {
name.push_str(" -> ");
// We might as well color the symlink output after the arrow. // We might as well color the symlink output after the arrow.
// This makes extra system calls, but provides important information that // This makes extra system calls, but provides important information that
// people run `ls -l --color` are very interested in. // people run `ls -l --color` are very interested in.
if let Some(ls_colors) = &config.color { if let Some(ls_colors) = &config.color {
// We get the absolute path to be able to construct PathData with valid Metadata. // We get the absolute path to be able to construct PathData with valid Metadata.
// This is because relative symlinks will fail to get_metadata. // This is because relative symlinks will fail to get_metadata.
let mut absolute_target = target.clone(); let mut absolute_target = target.clone();
if target.is_relative() { if target.is_relative() {
if let Some(parent) = path.p_buf.parent() { if let Some(parent) = path.p_buf.parent() {
absolute_target = parent.join(absolute_target); absolute_target = parent.join(absolute_target);
}
} }
}
let target_data = PathData::new(absolute_target, None, None, config, false); let target_data = PathData::new(absolute_target, None, None, config, false);
// If we have a symlink to a valid file, we use the metadata of said file. // If we have a symlink to a valid file, we use the metadata of said file.
// Because we use an absolute path, we can assume this is guaranteed to exist. // Because we use an absolute path, we can assume this is guaranteed to exist.
// Otherwise, we use path.md(), which will guarantee we color to the same // Otherwise, we use path.md(), which will guarantee we color to the same
// color of non-existent symlinks according to style_for_path_with_metadata. // color of non-existent symlinks according to style_for_path_with_metadata.
if path.md(out).is_none() if path.md(out).is_none()
&& get_metadata(target_data.p_buf.as_path(), target_data.must_dereference) && get_metadata(target_data.p_buf.as_path(), target_data.must_dereference)
.is_err() .is_err()
{ {
name.push_str(&path.p_buf.read_link().unwrap().to_string_lossy()); name.push_str(&path.p_buf.read_link().unwrap().to_string_lossy());
} else {
// Use fn get_metadata instead of md() here and above because ls
// should not exit with an err, if we are unable to obtain the target_metadata
let target_metadata = match get_metadata(
target_data.p_buf.as_path(),
target_data.must_dereference,
) {
Ok(md) => md,
Err(_) => path.md(out).unwrap().to_owned(),
};
name.push_str(&color_name(
escape_name(target.as_os_str(), &config.quoting_style),
&target_data.p_buf,
Some(&target_metadata),
ls_colors,
));
}
} else { } else {
// Use fn get_metadata instead of md() here and above because ls // If no coloring is required, we just use target as is.
// should not exit with an err, if we are unable to obtain the target_metadata // Apply the right quoting
let target_metadata = match get_metadata( name.push_str(&escape_name(target.as_os_str(), &config.quoting_style));
target_data.p_buf.as_path(),
target_data.must_dereference,
) {
Ok(md) => md,
Err(_) => path.md(out).unwrap().to_owned(),
};
name.push_str(&color_name(
escape_name(target.as_os_str(), &config.quoting_style),
&target_data.p_buf,
Some(&target_metadata),
ls_colors,
));
} }
} else { }
// If no coloring is required, we just use target as is. Err(err) => {
// Apply the right quoting show!(LsError::IOErrorContext(
name.push_str(&escape_name(target.as_os_str(), &config.quoting_style)); err,
path.p_buf.to_path_buf(),
false
));
} }
} }
} }

View file

@ -3503,3 +3503,22 @@ fn test_invalid_utf8() {
at.touch(filename); at.touch(filename);
ucmd.succeeds(); ucmd.succeeds();
} }
#[cfg(all(unix, feature = "chmod"))]
#[test]
fn test_ls_perm_io_errors() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
at.mkdir("d");
at.symlink_file("/", "d/s");
scene.ccmd("chmod").arg("600").arg("d").succeeds();
scene
.ucmd()
.arg("-l")
.arg("d")
.fails()
.code_is(1)
.stderr_contains("Permission denied");
}