Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
20 changes: 18 additions & 2 deletions src/uucore/src/lib/features/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,22 @@ impl<'a> From<Component<'a>> for OwningComponent {
}
}

/// Confirm that `path` (already known to exist) is a directory, without
/// requiring permission to list its contents.
///
/// `read_dir` alone would raise the right error for a non-directory, but
/// also requires listing permission on the target, which a plain "is this a
/// directory" check should not need (e.g. `realpath /root/` succeeds for
/// non-root users even though they can't list `/root`).
fn ensure_is_directory(path: &Path) -> IOResult<()> {
if fs::metadata(path)?.is_dir() {
Ok(())
} else {
read_dir(path)?;
Ok(())
Comment on lines +364 to +365

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks odd. Why not return an error with ErrorKind::NotADirectory?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I just do ErrorKind::NotADirectory.into(), the message changes from GNU's "Not a directory" to rust's generic lowercase "not a directory" — the capitalized version we have now only exists because read_dir surfaces the real OS error text. want me to keep the exact GNU wording with Error::new(ErrorKind::NotADirectory, "Not a directory"), or are you fine with the plain rust message even though it drifts from GNU here?

}
}

/// Return the canonical, absolute form of a path.
///
/// This function is a generalization of [`std::fs::canonicalize`] that
Expand Down Expand Up @@ -457,13 +473,13 @@ pub fn canonicalize<P: AsRef<Path>>(
match miss_mode {
MissingHandling::Existing => {
if has_to_be_directory {
read_dir(&result)?;
ensure_is_directory(&result)?;
}
}
MissingHandling::Normal => {
if result.exists() {
if has_to_be_directory {
read_dir(&result)?;
ensure_is_directory(&result)?;
}
} else if let Some(parent) = result.parent() {
read_dir(parent)?;
Expand Down
26 changes: 26 additions & 0 deletions tests/by-util/test_realpath.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,32 @@ fn test_realpath_trailing_slash() {
.stderr_contains("No such file or directory\n");
}

#[test]
#[cfg(unix)]
fn test_realpath_trailing_slash_unreadable_directory() {
// A trailing slash only asserts that the operand is a directory, which is
// a stat-level check needing search permission on the parent, not read
// permission on the directory itself.
// https://github.com/uutils/coreutils/issues/13151
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
at.mkdir("dir");
at.set_mode("dir", 0o100);

scene
.ucmd()
.arg("dir/")
.succeeds()
.stdout_contains(format!("{MAIN_SEPARATOR}dir\n"));
scene
.ucmd()
.args(&["-e", "dir/"])
.succeeds()
.stdout_contains(format!("{MAIN_SEPARATOR}dir\n"));

at.set_mode("dir", 0o700);
}

#[test]
fn test_realpath_empty() {
new_ucmd!().fails_with_code(1);
Expand Down
Loading