fix: retrieve metadata for symbolic links without following them

Previously, the function get_metadata in platform.rs used `fs::metadata` which follows symbolic links
and returns metadata for the target file. This caused issues #421: du / dust disagreement
when trying to determine properties of the symbolic link itself
This commit is contained in:
wugeer
2024-07-28 13:23:18 +08:00
committed by andy.boot
parent dbd18f90e7
commit 733117d0f6
5 changed files with 51 additions and 17 deletions

View File

@@ -115,9 +115,11 @@ fn sort_by_inode(a: &Node, b: &Node) -> std::cmp::Ordering {
fn ignore_file(entry: &DirEntry, walk_data: &WalkData) -> bool { fn ignore_file(entry: &DirEntry, walk_data: &WalkData) -> bool {
let is_dot_file = entry.file_name().to_str().unwrap_or("").starts_with('.'); let is_dot_file = entry.file_name().to_str().unwrap_or("").starts_with('.');
let is_ignored_path = walk_data.ignore_directories.contains(&entry.path()); let is_ignored_path = walk_data.ignore_directories.contains(&entry.path());
let follow_links =
walk_data.follow_links && entry.file_type().map_or(false, |ft| ft.is_symlink());
if !walk_data.allowed_filesystems.is_empty() { if !walk_data.allowed_filesystems.is_empty() {
let size_inode_device = get_metadata(entry.path(), false); let size_inode_device = get_metadata(entry.path(), false, follow_links);
if let Some((_size, Some((_id, dev)), _gunk)) = size_inode_device { if let Some((_size, Some((_id, dev)), _gunk)) = size_inode_device {
if !walk_data.allowed_filesystems.contains(&dev) { if !walk_data.allowed_filesystems.contains(&dev) {
return true; return true;
@@ -128,7 +130,7 @@ fn ignore_file(entry: &DirEntry, walk_data: &WalkData) -> bool {
|| walk_data.filter_modified_time.is_some() || walk_data.filter_modified_time.is_some()
|| walk_data.filter_changed_time.is_some() || walk_data.filter_changed_time.is_some()
{ {
let size_inode_device = get_metadata(entry.path(), false); let size_inode_device = get_metadata(entry.path(), false, follow_links);
if let Some((_, _, (modified_time, accessed_time, changed_time))) = size_inode_device { if let Some((_, _, (modified_time, accessed_time, changed_time))) = size_inode_device {
if entry.path().is_file() if entry.path().is_file()
&& [ && [
@@ -251,7 +253,15 @@ fn walk(dir: PathBuf, walk_data: &WalkData, depth: usize) -> Option<Node> {
} }
vec![] vec![]
}; };
build_node(dir, children, false, false, depth, walk_data) let is_symlink = if walk_data.follow_links {
match fs::symlink_metadata(&dir) {
Ok(metadata) => metadata.file_type().is_symlink(),
Err(_) => false,
}
} else {
false
};
build_node(dir, children, is_symlink, false, depth, walk_data)
} }
mod tests { mod tests {

View File

@@ -228,7 +228,7 @@ fn main() {
let follow_links = options.get_flag("dereference_links"); let follow_links = options.get_flag("dereference_links");
let allowed_filesystems = limit_filesystem let allowed_filesystems = limit_filesystem
.then(|| get_filesystem_devices(&target_dirs)) .then(|| get_filesystem_devices(&target_dirs, follow_links))
.unwrap_or_default(); .unwrap_or_default();
let simplified_dirs = simplify_dir_names(&target_dirs); let simplified_dirs = simplify_dir_names(&target_dirs);

View File

@@ -28,16 +28,16 @@ pub fn build_node(
let use_apparent_size = walk_data.use_apparent_size; let use_apparent_size = walk_data.use_apparent_size;
let by_filecount = walk_data.by_filecount; let by_filecount = walk_data.by_filecount;
get_metadata(&dir, use_apparent_size).map(|data| { get_metadata(
let inode_device = if is_symlink && !use_apparent_size { &dir,
None use_apparent_size,
} else { walk_data.follow_links && is_symlink,
data.1 )
}; .map(|data| {
let inode_device = data.1;
let size = if is_filtered_out_due_to_regex(walk_data.filter_regex, &dir) let size = if is_filtered_out_due_to_regex(walk_data.filter_regex, &dir)
|| is_filtered_out_due_to_invert_regex(walk_data.invert_filter_regex, &dir) || is_filtered_out_due_to_invert_regex(walk_data.invert_filter_regex, &dir)
|| (is_symlink && !use_apparent_size)
|| by_filecount && !is_file || by_filecount && !is_file
|| [ || [
(&walk_data.filter_modified_time, data.2 .0), (&walk_data.filter_modified_time, data.2 .0),

View File

@@ -17,9 +17,15 @@ type FileTime = (i64, i64, i64);
pub fn get_metadata<P: AsRef<Path>>( pub fn get_metadata<P: AsRef<Path>>(
path: P, path: P,
use_apparent_size: bool, use_apparent_size: bool,
follow_links: bool,
) -> Option<(u64, Option<InodeAndDevice>, FileTime)> { ) -> Option<(u64, Option<InodeAndDevice>, FileTime)> {
use std::os::unix::fs::MetadataExt; use std::os::unix::fs::MetadataExt;
match path.as_ref().metadata() { let metadata = if follow_links {
path.as_ref().metadata()
} else {
path.as_ref().symlink_metadata()
};
match metadata {
Ok(md) => { Ok(md) => {
if use_apparent_size { if use_apparent_size {
Some(( Some((
@@ -43,6 +49,7 @@ pub fn get_metadata<P: AsRef<Path>>(
pub fn get_metadata<P: AsRef<Path>>( pub fn get_metadata<P: AsRef<Path>>(
path: P, path: P,
use_apparent_size: bool, use_apparent_size: bool,
follow_links: bool,
) -> Option<(u64, Option<InodeAndDevice>, FileTime)> { ) -> Option<(u64, Option<InodeAndDevice>, FileTime)> {
// On windows opening the file to get size, file ID and volume can be very // On windows opening the file to get size, file ID and volume can be very
// expensive because 1) it causes a few system calls, and more importantly 2) it can cause // expensive because 1) it causes a few system calls, and more importantly 2) it can cause
@@ -142,7 +149,12 @@ pub fn get_metadata<P: AsRef<Path>>(
use std::os::windows::fs::MetadataExt; use std::os::windows::fs::MetadataExt;
let path = path.as_ref(); let path = path.as_ref();
match path.metadata() { let metadata = if follow_links {
path.metadata()
} else {
path.symlink_metadata()
};
match metadata {
Ok(ref md) => { Ok(ref md) => {
const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x20; const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x20;
const FILE_ATTRIBUTE_READONLY: u32 = 0x01; const FILE_ATTRIBUTE_READONLY: u32 = 0x01;

View File

@@ -34,13 +34,25 @@ pub fn simplify_dir_names<P: AsRef<Path>>(dirs: &[P]) -> HashSet<PathBuf> {
top_level_names top_level_names
} }
pub fn get_filesystem_devices<P: AsRef<Path>>(paths: &[P]) -> HashSet<u64> { pub fn get_filesystem_devices<P: AsRef<Path>>(paths: &[P], follow_links: bool) -> HashSet<u64> {
use std::fs;
// Gets the device ids for the filesystems which are used by the argument paths // Gets the device ids for the filesystems which are used by the argument paths
paths paths
.iter() .iter()
.filter_map(|p| match get_metadata(p, false) { .filter_map(|p| {
Some((_size, Some((_id, dev)), _time)) => Some(dev), let follow_links = if follow_links {
_ => None, // slow path: If dereference-links is set, then we check if the file is a symbolic link
match fs::symlink_metadata(p) {
Ok(metadata) => metadata.file_type().is_symlink(),
Err(_) => false,
}
} else {
false
};
match get_metadata(p, false, follow_links) {
Some((_size, Some((_id, dev)), _time)) => Some(dev),
_ => None,
}
}) })
.collect() .collect()
} }