From 48c0783e36f31776bb19585d5c84804c5d22e67e Mon Sep 17 00:00:00 2001
From: Emi Tatsuo <emi@alchemi.dev>
Date: Mon, 23 Nov 2020 09:39:45 -0500
Subject: [PATCH 1/2] Improve error handling for serve_dir

Specifically:
 - Serve NOT_FOUND when file or directory doesn't exist
 - Serve NOT_FOUND and warn in log when permission denied for file or directory
 - Serve SERVER_ERROR and warn in log when permission denied or not found for path to serve from
 - Serve SERVER_ERROR and warn in log with a link to the GH if an unexpected error happens
---
 src/util.rs | 67 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 8 deletions(-)

diff --git a/src/util.rs b/src/util.rs
index 5c623aa..983e67a 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -23,8 +23,11 @@ pub async fn serve_file<P: AsRef<Path>>(path: P, mime: &Mime) -> Result<Response
     let file = match File::open(path).await {
         Ok(file) => file,
         Err(err) => match err.kind() {
-            io::ErrorKind::NotFound => return Ok(Response::not_found()),
-            _ => return Err(err.into()),
+            std::io::ErrorKind::PermissionDenied => {
+                warn!("Asked to serve {}, but permission denied by OS", path.display());
+                return Ok(Response::not_found());
+            },
+            _ => return warn_unexpected(err, path, line!()),
         }
     };
 
@@ -34,16 +37,44 @@ pub async fn serve_file<P: AsRef<Path>>(path: P, mime: &Mime) -> Result<Response
 #[cfg(feature="serve_dir")]
 pub async fn serve_dir<D: AsRef<Path>, P: AsRef<Path>>(dir: D, virtual_path: &[P]) -> Result<Response> {
     debug!("Dir: {}", dir.as_ref().display());
-    let dir = dir.as_ref().canonicalize()
-        .context("Failed to canonicalize directory")?;
+    let dir = dir.as_ref();
+    let dir = match dir.canonicalize() {
+        Ok(dir) => dir,
+        Err(e) => {
+            match e.kind() {
+                std::io::ErrorKind::NotFound => {
+                    warn!("Path {} not found.  Check your configuration.", dir.display());
+                    return Response::server_error("Server incorrectly configured")
+                },
+                std::io::ErrorKind::PermissionDenied => {
+                    warn!("Permission denied for {}.  Check that the server has access.", dir.display());
+                    return Response::server_error("Server incorrectly configured")
+                },
+                _ => return warn_unexpected(e, dir, line!()),
+            }
+        },
+    };
     let mut path = dir.to_path_buf();
 
     for segment in virtual_path {
         path.push(segment);
     }
 
-    let path = path.canonicalize()
-        .context("Failed to canonicalize path")?;
+    let path = match path.canonicalize() {
+        Ok(dir) => dir,
+        Err(e) => {
+            match e.kind() {
+                std::io::ErrorKind::NotFound => return Ok(Response::not_found()),
+                std::io::ErrorKind::PermissionDenied => {
+                    // Runs when asked to serve a file in a restricted dir
+                    // i.e. not /noaccess, but /noaccess/file
+                    warn!("Asked to serve {}, but permission denied by OS", path.display());
+                    return Ok(Response::not_found());
+                },
+                _ => return warn_unexpected(e, path.as_ref(), line!()),
+            }
+        },
+    };
 
     if !path.starts_with(&dir) {
         return Ok(Response::not_found());
@@ -59,11 +90,15 @@ pub async fn serve_dir<D: AsRef<Path>, P: AsRef<Path>>(dir: D, virtual_path: &[P
 
 #[cfg(feature="serve_dir")]
 async fn serve_dir_listing<P: AsRef<Path>, B: AsRef<Path>>(path: P, virtual_path: &[B]) -> Result<Response> {
-    let mut dir = match fs::read_dir(path).await {
+    let mut dir = match fs::read_dir(path.as_ref()).await {
         Ok(dir) => dir,
         Err(err) => match err.kind() {
             io::ErrorKind::NotFound => return Ok(Response::not_found()),
-            _ => return Err(err.into()),
+            std::io::ErrorKind::PermissionDenied => {
+                warn!("Asked to serve {}, but permission denied by OS", path.as_ref().display());
+                return Ok(Response::not_found());
+            },
+            _ => return warn_unexpected(err, path.as_ref(), line!()),
         }
     };
 
@@ -112,6 +147,22 @@ pub fn guess_mime_from_path<P: AsRef<Path>>(path: P) -> Mime {
     mime_guess::from_ext(extension).first_or_octet_stream()
 }
 
+#[cfg(feature="serve_dir")]
+/// Print a warning to the log asking to file an issue and respond with "Unexpected Error"
+pub (crate) fn warn_unexpected(err: impl std::fmt::Debug, path: &Path, line: u32) -> Result<Response> {
+    warn!(
+        concat!(
+            "Unexpected error serving path {} at util.rs:{}, please report to ",
+            env!("CARGO_PKG_REPOSITORY"),
+            "/issues: {:?}",
+        ),
+        path.display(),
+        line,
+        err
+    );
+    Response::server_error("Unexpected error")
+}
+
 /// A convenience trait alias for `AsRef<T> + Into<T::Owned>`,
 /// most commonly used to accept `&str` or `String`:
 ///

From 4743419f1e7b6df9ae3b911f830796bd34878ca3 Mon Sep 17 00:00:00 2001
From: Emi Tatsuo <emi@alchemi.dev>
Date: Mon, 23 Nov 2020 09:49:04 -0500
Subject: [PATCH 2/2] Update changelog with improved error handling for
 serve_dir

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 472fd82..7b446a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Docments can be converted into responses with std::convert::Into [@Alch-Emi]
 ### Improved
 - build time and size by [@Alch-Emi]
+- Improved error handling in serve_dir [@Alch-Emi]
 
 ## [0.3.0] - 2020-11-14
 ### Added