Fix Bug: Handle -L with broken symlink #457 (#754)

4 files changed, 187 insertions, 32 deletions

diff --git a/src/display.rs b/src/display.rs
index 56e16a6..c57b3f6 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -289,6 +289,8 @@ fn get_output(
     tree: (usize, &str),
 ) -> Vec<String> {
     let mut strings: Vec<String> = Vec::new();
+    let colorize_missing = |string: &str| colors.colorize(string, &Elem::NoAccess);
+
     for (i, block) in flags.blocks.0.iter().enumerate() {
         let mut block_vec = if Layout::Tree == flags.layout && tree.0 == i {
             vec![colors.colorize(tree.1, &Elem::TreeEdge)]
@@ -297,28 +299,58 @@ fn get_output(
         };
 
         match block {
-            Block::INode => block_vec.push(meta.inode.render(colors)),
-            Block::Links => block_vec.push(meta.links.render(colors)),
+            Block::INode => block_vec.push(match &meta.inode {
+                Some(inode) => inode.render(colors),
+                None => colorize_missing("?"),
+            }),
+            Block::Links => block_vec.push(match &meta.links {
+                Some(links) => links.render(colors),
+                None => colorize_missing("?"),
+            }),
             Block::Permission => {
                 block_vec.extend([
                     meta.file_type.render(colors),
-                    meta.permissions.render(colors, flags),
-                    meta.access_control.render_method(colors),
+                    match meta.permissions {
+                        Some(permissions) => permissions.render(colors, flags),
+                        None => colorize_missing("?????????"),
+                    },
+                    match &meta.access_control {
+                        Some(access_control) => access_control.render_method(colors),
+                        None => colorize_missing(""),
+                    },
                 ]);
             }
-            Block::User => block_vec.push(meta.owner.render_user(colors)),
-            Block::Group => block_vec.push(meta.owner.render_group(colors)),
-            Block::Context => block_vec.push(meta.access_control.render_context(colors)),
+            Block::User => block_vec.push(match &meta.owner {
+                Some(owner) => owner.render_user(colors),
+                None => colorize_missing("?"),
+            }),
+            Block::Group => block_vec.push(match &meta.owner {
+                Some(owner) => owner.render_group(colors),
+                None => colorize_missing("?"),
+            }),
+            Block::Context => block_vec.push(match &meta.access_control {
+                Some(access_control) => access_control.render_context(colors),
+                None => colorize_missing("?"),
+            }),
             Block::Size => {
                 let pad = if Layout::Tree == flags.layout && 0 == tree.0 && 0 == i {
                     None
                 } else {
                     Some(padding_rules[&Block::SizeValue])
                 };
-                block_vec.push(meta.size.render(colors, flags, pad))
+                block_vec.push(match &meta.size {
+                    Some(size) => size.render(colors, flags, pad),
+                    None => colorize_missing("?"),
+                })
             }
-            Block::SizeValue => block_vec.push(meta.size.render_value(colors, flags)),
-            Block::Date => block_vec.push(meta.date.render(colors, flags)),
+            Block::SizeValue => block_vec.push(match &meta.size {
+                Some(size) => size.render_value(colors, flags),
+                None => colorize_missing("?"),
+            }),
+            Block::Date => block_vec.push(match &meta.date {
+                Some(date) => date.render(colors, flags),
+                None => colorize_missing("?"),
+            }),
             Block::Name => {
                 block_vec.extend([
                     meta.name.render(
@@ -377,7 +409,10 @@ fn detect_size_lengths(metas: &[Meta], flags: &Flags) -> usize {
     let mut max_value_length: usize = 0;
 
     for meta in metas {
-        let value_len = meta.size.value_string(flags).len();
+        let value_len = match &meta.size {
+            Some(size) => size.value_string(flags).len(),
+            None => 0,
+        };
 
         if value_len > max_value_length {
             max_value_length = value_len;
diff --git a/src/meta/filetype.rs b/src/meta/filetype.rs
index e3fa1cc..72ddefa 100644
--- a/src/meta/filetype.rs
+++ b/src/meta/filetype.rs
@@ -149,8 +149,9 @@ mod test {
         let metadata = tmp_dir.path().metadata().expect("failed to get metas");
 
         let colors = Colors::new(ThemeOption::NoLscolors);
+
         #[cfg(not(windows))]
-        let file_type = FileType::new(&metadata, None, &meta.permissions);
+        let file_type = FileType::new(&metadata, None, &meta.permissions.unwrap());
         #[cfg(windows)]
         let file_type = FileType::new(&metadata, None, tmp_dir.path());
 
diff --git a/src/meta/mod.rs b/src/meta/mod.rs
index 286685e..00090dc 100644
--- a/src/meta/mod.rs
+++ b/src/meta/mod.rs
@@ -36,17 +36,17 @@ use std::path::{Component, Path, PathBuf};
 pub struct Meta {
     pub name: Name,
     pub path: PathBuf,
-    pub permissions: Permissions,
-    pub date: Date,
-    pub owner: Owner,
+    pub permissions: Option<Permissions>,
+    pub date: Option<Date>,
+    pub owner: Option<Owner>,
     pub file_type: FileType,
-    pub size: Size,
+    pub size: Option<Size>,
     pub symlink: SymLink,
     pub indicator: Indicator,
-    pub inode: INode,
-    pub links: Links,
+    pub inode: Option<INode>,
+    pub links: Option<Links>,
     pub content: Option<Vec<Meta>>,
-    pub access_control: AccessControl,
+    pub access_control: Option<AccessControl>,
 }
 
 impl Meta {
@@ -169,17 +169,27 @@ impl Meta {
     }
 
     pub fn calculate_total_size(&mut self) {
+        if self.size.is_none() {
+            return;
+        }
+
         if let FileType::Directory { .. } = self.file_type {
             if let Some(metas) = &mut self.content {
-                let mut size_accumulated = self.size.get_bytes();
+                let mut size_accumulated = match &self.size {
+                    Some(size) => size.get_bytes(),
+                    None => 0,
+                };
                 for x in &mut metas.iter_mut() {
                     x.calculate_total_size();
-                    size_accumulated += x.size.get_bytes();
+                    size_accumulated += match &x.size {
+                        Some(size) => size.get_bytes(),
+                        None => 0,
+                    };
                 }
-                self.size = Size::new(size_accumulated);
+                self.size = Some(Size::new(size_accumulated));
             } else {
                 // possibility that 'depth' limited the recursion in 'recurse_into'
-                self.size = Size::new(Meta::calculate_total_file_size(&self.path));
+                self.size = Some(Size::new(Meta::calculate_total_file_size(&self.path)));
             }
         }
     }
@@ -225,6 +235,7 @@ impl Meta {
     pub fn from_path(path: &Path, dereference: bool) -> io::Result<Self> {
         let mut metadata = path.symlink_metadata()?;
         let mut symlink_meta = None;
+        let mut broken_link = false;
         if metadata.file_type().is_symlink() {
             match path.metadata() {
                 Ok(m) => {
@@ -238,7 +249,8 @@ impl Meta {
                     // This case, it is definitely a symlink or
                     // path.symlink_metadata would have errored out
                     if dereference {
-                        return Err(e);
+                        broken_link = true;
+                        eprintln!("lsd: {}: {}\n", path.to_str().unwrap_or(""), e);
                     }
                 }
             }
@@ -252,8 +264,6 @@ impl Meta {
         #[cfg(windows)]
         let (owner, permissions) = windows_utils::get_file_data(path)?;
 
-        let access_control = AccessControl::for_path(path);
-
         #[cfg(not(windows))]
         let file_type = FileType::new(&metadata, symlink_meta.as_ref(), &permissions);
 
@@ -261,16 +271,27 @@ impl Meta {
         let file_type = FileType::new(&metadata, symlink_meta.as_ref(), path);
 
         let name = Name::new(path, file_type);
-        let inode = INode::from(&metadata);
-        let links = Links::from(&metadata);
+
+        let (inode, links, size, date, owner, permissions, access_control) = match broken_link {
+            true => (None, None, None, None, None, None, None),
+            false => (
+                Some(INode::from(&metadata)),
+                Some(Links::from(&metadata)),
+                Some(Size::from(&metadata)),
+                Some(Date::from(&metadata)),
+                Some(owner),
+                Some(permissions),
+                Some(AccessControl::for_path(path)),
+            ),
+        };
 
         Ok(Self {
             inode,
             links,
             path: path.to_path_buf(),
             symlink: SymLink::from(path),
-            size: Size::from(&metadata),
-            date: Date::from(&metadata),
+            size,
+            date,
             indicator: Indicator::from(file_type),
             owner,
             permissions,
@@ -282,15 +303,61 @@ impl Meta {
     }
 }
 
-#[cfg(unix)]
 #[cfg(test)]
 mod tests {
     use super::Meta;
+    use std::fs::File;
+    use tempfile::tempdir;
 
+    #[cfg(unix)]
     #[test]
     fn test_from_path_path() {
         let dir = assert_fs::TempDir::new().unwrap();
         let meta = Meta::from_path(dir.path(), false).unwrap();
         assert_eq!(meta.path, dir.path())
     }
+
+    #[test]
+    fn test_from_path() {
+        let tmp_dir = tempdir().expect("failed to create temp dir");
+
+        let path_a = tmp_dir.path().join("aaa.aa");
+        File::create(&path_a).expect("failed to create file");
+        let meta_a = Meta::from_path(&path_a, false).expect("failed to get meta");
+
+        let path_b = tmp_dir.path().join("bbb.bb");
+        let path_c = tmp_dir.path().join("ccc.cc");
+
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&path_c, &path_b).expect("failed to create broken symlink");
+
+        // this needs to be tested on Windows
+        // likely to fail because of permission issue
+        // see https://doc.rust-lang.org/std/os/windows/fs/fn.symlink_file.html
+        #[cfg(windows)]
+        std::os::windows::fs::symlink_file(&path_c, &path_b)
+            .expect("failed to create broken symlink");
+
+        let meta_b = Meta::from_path(&path_b, true).expect("failed to get meta");
+
+        assert!(
+            meta_a.inode.is_some()
+                && meta_a.links.is_some()
+                && meta_a.size.is_some()
+                && meta_a.date.is_some()
+                && meta_a.owner.is_some()
+                && meta_a.permissions.is_some()
+                && meta_a.access_control.is_some()
+        );
+
+        assert!(
+            meta_b.inode.is_none()
+                && meta_b.links.is_none()
+                && meta_b.size.is_none()
+                && meta_b.date.is_none()
+                && meta_b.owner.is_none()
+                && meta_b.permissions.is_none()
+                && meta_b.access_control.is_none()
+        );
+    }
 }
diff --git a/src/sort.rs b/src/sort.rs
index b744b52..6fb39cb 100644
--- a/src/sort.rs
+++ b/src/sort.rs
@@ -48,7 +48,12 @@ fn with_dirs_first(a: &Meta, b: &Meta) -> Ordering {
 }
 
 fn by_size(a: &Meta, b: &Meta) -> Ordering {
-    b.size.get_bytes().cmp(&a.size.get_bytes())
+    match (&a.size, &b.size) {
+        (Some(a_size), Some(b_size)) => b_size.get_bytes().cmp(&a_size.get_bytes()),
+        (Some(_), None) => Ordering::Greater,
+        (None, Some(_)) => Ordering::Less,
+        (None, None) => Ordering::Equal,
+    }
 }
 
 fn by_name(a: &Meta, b: &Meta) -> Ordering {
@@ -72,6 +77,7 @@ mod tests {
     use super::*;
     use crate::flags::Flags;
     use std::fs::{create_dir, File};
+    use std::io::prelude::*;
     use std::process::Command;
     use tempfile::tempdir;
 
@@ -338,4 +344,50 @@ mod tests {
         let sorter = assemble_sorters(&flags);
         assert_eq!(by_meta(&sorter, &meta_c, &meta_d), Ordering::Equal);
     }
+
+    #[test]
+    fn test_sort_by_size() {
+        let tmp_dir = tempdir().expect("failed to create temp dir");
+
+        let path_a = tmp_dir.path().join("aaa.aa");
+        File::create(&path_a)
+            .expect("failed to create file")
+            .write_all(b"1, 2, 3")
+            .expect("failed to write to file");
+        let meta_a = Meta::from_path(&path_a, false).expect("failed to get meta");
+
+        let path_b = tmp_dir.path().join("bbb.bb");
+        File::create(&path_b)
+            .expect("failed to create file")
+            .write_all(b"1, 2, 3, 4, 5, 6, 7, 8, 9, 10")
+            .expect("failed to write file");
+        let meta_b = Meta::from_path(&path_b, false).expect("failed to get meta");
+
+        let path_c = tmp_dir.path().join("ccc.cc");
+        let path_d = tmp_dir.path().join("ddd.dd");
+
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&path_d, &path_c).expect("failed to create broken symlink");
+
+        // this needs to be tested on Windows
+        // likely to fail because of permission issue
+        // see https://doc.rust-lang.org/std/os/windows/fs/fn.symlink_file.html
+        #[cfg(windows)]
+        std::os::windows::fs::symlink_file(&path_d, &path_c)
+            .expect("failed to create broken symlink");
+
+        let meta_c = Meta::from_path(&path_c, true).expect("failed to get meta");
+
+        assert_eq!(by_size(&meta_a, &meta_a), Ordering::Equal);
+        assert_eq!(by_size(&meta_a, &meta_b), Ordering::Greater);
+        assert_eq!(by_size(&meta_a, &meta_c), Ordering::Greater);
+
+        assert_eq!(by_size(&meta_b, &meta_a), Ordering::Less);
+        assert_eq!(by_size(&meta_b, &meta_b), Ordering::Equal);
+        assert_eq!(by_size(&meta_b, &meta_c), Ordering::Greater);
+
+        assert_eq!(by_size(&meta_c, &meta_a), Ordering::Less);
+        assert_eq!(by_size(&meta_c, &meta_b), Ordering::Less);
+        assert_eq!(by_size(&meta_c, &meta_c), Ordering::Equal);
+    }
 }