Properly handle errors when following a symlink

Fixes #123. The code assumes that every File that has its link_target() method called would first have been checked to make sure it’s actually a link first. Unfortunately it also assumed that the only thing that can go wrong while following a link is if the file wasn’t a link, meaning it crashes when given a link it doesn’t have permission to follow.

This makes the file_target() method able to return either a file or path for displaying, as before, but also an IO error for when things go wrong.

src/fs/file.rs

@@ -3,6 +3,7 @@
 use std::ascii::AsciiExt;
 use std::env::current_dir;
 use std::fs;
+use std::io::Error as IOError;
 use std::io::Result as IOResult;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
 use std::path::{Path, PathBuf};
@@ -147,7 +148,7 @@ impl<'dir> File<'dir> {
     /// Whether this file is a symlink on the filesystem.
     pub fn is_link(&self) -> bool {
         self.metadata.file_type().is_symlink()
-    }    
+    }
 
     /// Whether this file is a dotfile, based on its name. In Unix, file names
     /// beginning with a dot represent system or configuration files, and
@@ -162,10 +163,10 @@ impl<'dir> File<'dir> {
     /// If statting the file fails (usually because the file on the
     /// other end doesn't exist), returns the path to the file
     /// that should be there.
-    pub fn link_target(&self) -> Result<File<'dir>, PathBuf> {
+    pub fn link_target(&self) -> FileTarget<'dir> {
         let path = match fs::read_link(&self.path) {
             Ok(path)  => path,
-            Err(_)    => panic!("This was not a link!"),
+            Err(e)    => return FileTarget::Err(e),
         };
 
         let target_path = match self.dir {
@@ -180,7 +181,7 @@ impl<'dir> File<'dir> {
 
         // Use plain `metadata` instead of `symlink_metadata` - we *want* to follow links.
         if let Ok(metadata) = fs::metadata(&target_path) {
-            Ok(File {
+            FileTarget::Ok(File {
                 path:      target_path.to_path_buf(),
                 dir:       self.dir,
                 metadata:  metadata,
@@ -189,7 +190,7 @@ impl<'dir> File<'dir> {
             })
         }
         else {
-            Err(target_path)
+            FileTarget::Broken(target_path)
         }
     }
 
@@ -359,17 +360,17 @@ impl<'dir> File<'dir> {
     pub fn is_pipe(&self) -> bool {
         self.metadata.file_type().is_fifo()
     }
-    
+
     /// Whether this file is a char device on the filesystem.
     pub fn is_char_device(&self) -> bool {
         self.metadata.file_type().is_char_device()
     }
-    
+
     /// Whether this file is a block device on the filesystem.
     pub fn is_block_device(&self) -> bool {
         self.metadata.file_type().is_block_device()
     }
-    
+
     /// Whether this file is a socket on the filesystem.
     pub fn is_socket(&self) -> bool {
         self.metadata.file_type().is_socket()
@@ -382,17 +383,17 @@ impl<'dir> File<'dir> {
     pub fn is_pipe(&self) -> bool {
         false
     }
-    
+
     /// Whether this file is a char device on the filesystem.
     pub fn is_char_device(&self) -> bool {
         false
     }
-    
+
     /// Whether this file is a block device on the filesystem.
     pub fn is_block_device(&self) -> bool {
         false
     }
-    
+
     /// Whether this file is a socket on the filesystem.
     pub fn is_socket(&self) -> bool {
         false
@@ -425,6 +426,23 @@ fn ext(path: &Path) -> Option<String> {
 }
 
 
+/// The result of following a symlink.
+pub enum FileTarget<'dir> {
+
+    /// The symlink pointed at a file that exists.
+    Ok(File<'dir>),
+
+    /// The symlink pointed at a file that does not exist. Holds the path
+    /// where the file would be, if it existed.
+    Broken(PathBuf),
+
+    /// There was an IO error when following the link. This can happen if the
+    /// file isn't a link to begin with, but also if, say, we don't have
+    /// permission to follow it.
+    Err(IOError),
+}
+
+
 #[cfg(test)]
 mod test {
     use super::ext;

src/fs/mod.rs

@@ -2,7 +2,7 @@ mod dir;
 pub use self::dir::Dir;
 
 mod file;
-pub use self::file::File;
+pub use self::file::{File, FileTarget};
 
 pub mod feature;
 pub mod fields;

src/output/mod.rs

@@ -1,6 +1,6 @@
 use ansi_term::Style;
 
-use fs::File;
+use fs::{File, FileTarget};
 
 pub use self::cell::{TextCell, TextCellContents, DisplayWidth};
 pub use self::colours::Colours;
@@ -42,7 +42,7 @@ pub fn filename(file: &File, colours: &Colours, links: bool) -> TextCellContents
 
     if links && file.is_link() {
         match file.link_target() {
-            Ok(target) => {
+            FileTarget::Ok(target) => {
                 bits.push(Style::default().paint(" "));
                 bits.push(colours.punctuation.paint("->"));
                 bits.push(Style::default().paint(" "));
@@ -67,12 +67,16 @@ pub fn filename(file: &File, colours: &Colours, links: bool) -> TextCellContents
                 }
             },
 
-            Err(broken_path) => {
+            FileTarget::Broken(broken_path) => {
                 bits.push(Style::default().paint(" "));
                 bits.push(colours.broken_arrow.paint("->"));
                 bits.push(Style::default().paint(" "));
                 bits.push(colours.broken_filename.paint(broken_path.display().to_string()));
             },
+
+            FileTarget::Err(_) => {
+                // Do nothing -- the error gets displayed on the next line
+            }
         }
     }
 
@@ -84,11 +88,11 @@ pub fn file_colour(colours: &Colours, file: &File) -> Style {
         f if f.is_directory()        => colours.filetypes.directory,
         f if f.is_executable_file()  => colours.filetypes.executable,
         f if f.is_link()             => colours.filetypes.symlink,
-        f if f.is_pipe()               => colours.filetypes.pipe,
-        f if f.is_char_device() 
+        f if f.is_pipe()             => colours.filetypes.pipe,
+        f if f.is_char_device()
            | f.is_block_device()     => colours.filetypes.device,
         f if f.is_socket()           => colours.filetypes.socket,
-        f if !f.is_file()            => colours.filetypes.special,        
+        f if !f.is_file()            => colours.filetypes.special,
         f if f.is_immediate()        => colours.filetypes.immediate,
         f if f.is_image()            => colours.filetypes.image,
         f if f.is_video()            => colours.filetypes.video,

xtests/proc_1_root

@@ -0,0 +1,2 @@
+lrwxrwxrwx 0 root 29 Oct 17:23 /proc/1/root
+                               └── <Permission denied (os error 13)>

xtests/run.sh

@@ -52,5 +52,7 @@ $exa $testcases/links -T 2>&1 | diff -q - $results/links_T  || exit 1
 
 COLUMNS=80 $exa $testcases/links 2>&1 | diff -q - $results/links  || exit 1
 
+$exa /proc/1/root -l 2>&1 | diff - $results/proc_1_root  || exit 1
+
 
 echo "All the tests passed!"