From d5e6bdd136a3094ab8cc2e1ade16fdf221e49cd2 Mon Sep 17 00:00:00 2001
From: Jose Quintana <joseluisquintana20@gmail.com>
Date: Sun, 14 Feb 2021 23:56:47 +0100
Subject: [PATCH] refactor: improve error handling on static files module

---
 src/staticfile_middleware/staticfile.rs | 20 ++++++++------------
 src/staticfiles.rs                      | 36 +++++++++++++++++++++++++-----------
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/src/staticfile_middleware/staticfile.rs b/src/staticfile_middleware/staticfile.rs
index a2a84f4..b211118 100644
--- a/src/staticfile_middleware/staticfile.rs
+++ b/src/staticfile_middleware/staticfile.rs
@@ -9,10 +9,10 @@ use iron::modifiers::Header;
 use iron::prelude::*;
 use iron::status;
 use percent_encoding::percent_decode_str;
+use std::error;
 use std::fs::{File, Metadata};
 use std::path::{Path, PathBuf};
 use std::time::{SystemTime, UNIX_EPOCH};
-use std::{error, io};
 
 use crate::helpers;
 use crate::staticfile_middleware::partial_file::PartialFile;
@@ -25,19 +25,15 @@ pub struct Staticfile {
 }
 
 impl Staticfile {
-    pub fn new<P: AsRef<Path>>(
-        root_dir: P,
-        assets_dir: P,
-        dir_listing: bool,
-    ) -> io::Result<Staticfile>
+    pub fn new<P: AsRef<Path>>(root_dir: P, assets_dir: P, dir_listing: bool) -> Staticfile
     where
         PathBuf: From<P>,
     {
-        Ok(Staticfile {
+        Staticfile {
             root: root_dir.into(),
             assets: assets_dir.into(),
             dir_listing,
-        })
+        }
     }
 
     /// Resolve a specific path which should be relative to base path.
@@ -368,7 +364,7 @@ mod test {
         let fs2 = TestFilesystemSetup::new();
         fs2.dir("assets");
 
-        let sf = Staticfile::new(fs.path(), fs2.path(), false).unwrap();
+        let sf = Staticfile::new(fs.path(), fs2.path(), false);
         let path = sf.resolve_path(&["index.html"]);
         assert!(path.unwrap().ends_with("index.html"));
     }
@@ -381,7 +377,7 @@ mod test {
         let fs2 = TestFilesystemSetup::new();
         fs2.file("assets");
 
-        let sf = Staticfile::new(fs.path(), fs2.path(), false).unwrap();
+        let sf = Staticfile::new(fs.path(), fs2.path(), false);
         let path = sf.resolve_path(&["dir", "index.html"]);
         assert!(path.unwrap().ends_with("dir/index.html"));
     }
@@ -394,7 +390,7 @@ mod test {
         let fs2 = TestFilesystemSetup::new();
         let dir2 = fs2.file("assets");
 
-        let sf = Staticfile::new(dir, dir2, false).unwrap();
+        let sf = Staticfile::new(dir, dir2, false);
         let path = sf.resolve_path(&["..", "naughty.txt"]);
         assert!(path.is_err());
     }
@@ -403,7 +399,7 @@ mod test {
     fn staticfile_disallows_post_requests() {
         let fs = TestFilesystemSetup::new();
         let fs2 = TestFilesystemSetup::new();
-        let sf = Staticfile::new(fs.path(), fs2.path(), false).unwrap();
+        let sf = Staticfile::new(fs.path(), fs2.path(), false);
 
         let response = request::post("http://127.0.0.1/", Headers::new(), "", &sf);
 
diff --git a/src/staticfiles.rs b/src/staticfiles.rs
index ac3ef2e..c503f74 100644
--- a/src/staticfiles.rs
+++ b/src/staticfiles.rs
@@ -33,20 +33,35 @@ impl StaticFiles {
     /// Handle static files for current `StaticFiles` middleware.
     pub fn handle(&self) -> Chain {
         // Check root directory
-        let p = PathBuf::from(&self.opts.root_dir).canonicalize().unwrap();
+        let p = match PathBuf::from(&self.opts.root_dir).canonicalize() {
+            Ok(p) => p,
+            Err(e) => {
+                error!("Root directory path not found or inaccessible");
+                debug!("Error: {}", e);
+                std::process::exit(1)
+            }
+        };
         let root_dir = PathBuf::from(helpers::adjust_canonicalization(p));
 
         // Check assets directory
-        let p = PathBuf::from(&self.opts.assets_dir).canonicalize().unwrap();
+        let p = match PathBuf::from(&self.opts.assets_dir).canonicalize() {
+            Ok(p) => p,
+            Err(e) => {
+                error!("Assets directory path not found or inaccessible",);
+                debug!("Error: {}", e);
+                std::process::exit(1)
+            }
+        };
         let assets_dir = PathBuf::from(helpers::adjust_canonicalization(p));
 
         // Get assets directory name
         let assets_dirname = &match helpers::get_dirname(&assets_dir) {
+            Ok(v) => v,
             Err(e) => {
-                error!("{}", e);
+                error!("Unable to get assets directory name");
+                debug!("Error: {}", e);
                 std::process::exit(1)
             }
-            Ok(v) => v,
         };
 
         if self.opts.directory_listing {
@@ -54,15 +69,14 @@ impl StaticFiles {
         }
 
         // Define middleware chain
-        let mut chain = Chain::new(
-            Staticfile::new(root_dir, assets_dir, self.opts.directory_listing)
-                .expect("Directory to serve files was not found"),
-        );
+        let mut chain = Chain::new(Staticfile::new(
+            root_dir,
+            assets_dir,
+            self.opts.directory_listing,
+        ));
         let one_day = Duration::new(60 * 60 * 24, 0);
         let one_year = Duration::new(60 * 60 * 24 * 365, 0);
-        let default_content_type = "text/html"
-            .parse::<mime::Mime>()
-            .expect("Unable to create a default content type header");
+        let default_content_type = "text/html".parse::<mime::Mime>().unwrap();
 
         // CORS support
         let allowed_hosts = &self.opts.cors_allow_origins;
--
libgit2 1.7.2