fix(config): root_dir not defined in config file panics

LeoBorai · LeoBorai · commit 7e483f741a56 · 2021-05-06T20:38:00.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,50 @@
 
 <Empty>
 
+<a name="v0.3.0"></a>
+## v0.3.0
+
+> Requires Rust: rustc 1.49.0 (e1884a8e3 2020-12-29)
+
+#### Features
+
+* Support for Cross-Origin Resource Sharing
+
+* Using the `--cors` flag when running the HTTP Server will now provide a
+CORS configuration which allows requests from any origin
+
+* The server configuration file supports a fully configurable CORS field now
+
+```toml
+[cors]
+allow_credentials = false
+allow_headers = ["content-type", "authorization", "content-length"]
+allow_methods = ["GET", "PATCH", "POST", "PUT", "DELETE"]
+allow_origin = "example.com"
+expose_headers = ["*", "authorization"]
+max_age = 600
+request_headers = ["x-app-version"]
+request_method = "GET"
+```
+
+#### Improvements
+
+* Codebase refactor for `server` module, introducing middleware support for
+services to programatically build a server instance from the provided `Config`
+
+* Replace `full` feature flags on `tokio` and `hyper` with sepecific features,
+to reduce crate load on compile time. This improve build times and crate size.
+
+#### Fixes
+
+* Fix issue where the `root_dir` is taken _as is_ from the CLI
+arguments resulting on `./` instead of the current working
+directory for the `FileExplorer`
+
+* Fix issue where loading config without `root_dir` defined panics
+and uses the current working directory as default `root_dir` as is
+done for CLI
+
 <a name="v0.2.2"></a>
 ## v0.2.2 (2021-04-22)
 
diff --git a/fixtures/config.toml b/fixtures/config.toml
@@ -3,19 +3,19 @@
 
 host = "127.0.0.1"
 port = 7878
-verbose = false
-root_dir = "./"
+# verbose = false
+# root_dir = "./"
 
 # [tls]
 # cert = "cert.pem"
 # key = "key.pem"
 
-[cors]
-allow_credentials = false
-allow_headers = ["content-type", "authorization", "content-length"]
-allow_methods = ["GET", "PATCH", "POST", "PUT", "DELETE"]
-allow_origin = "example.com"
-expose_headers = ["*", "authorization"]
-max_age = 600
-request_headers = ["x-app-version"]
-request_method = "GET"
+# [cors]
+# allow_credentials = false
+# allow_headers = ["content-type", "authorization", "content-length"]
+# allow_methods = ["GET", "PATCH", "POST", "PUT", "DELETE"]
+# allow_origin = "example.com"
+# expose_headers = ["*", "authorization"]
+# max_age = 600
+# request_headers = ["x-app-version"]
+# request_method = "GET"
diff --git a/src/bin/main.rs b/src/bin/main.rs
@@ -1,8 +1,15 @@
 use http_server::make_server;
+use std::process::exit;
 
 #[tokio::main]
 async fn main() {
-    let server = make_server().unwrap();
-
-    server.run().await;
+    match make_server() {
+        Ok(server) => {
+            server.run().await;
+        }
+        Err(error) => {
+            eprint!("{:?}", error);
+            exit(1);
+        }
+    }
 }
diff --git a/src/cli.rs b/src/cli.rs
@@ -68,8 +68,7 @@ impl Default for Cli {
 }
 
 mod tests {
-    use std::str::FromStr;
-
+    #[allow(unused_imports)]
     use super::*;
 
     #[test]
diff --git a/src/config/cors.rs b/src/config/cors.rs
@@ -1,7 +1,6 @@
 use anyhow::{Error, Result};
 use serde::Deserialize;
 use std::convert::TryFrom;
-use std::time::Duration;
 
 /// CORS (Cross Origin Resource Sharing) configuration for the HTTP/S
 /// server.
@@ -236,6 +235,7 @@ impl TryFrom<CorsConfigFile> for CorsConfig {
 }
 
 mod tests {
+    #[allow(unused_imports)]
     use super::*;
 
     #[test]
diff --git a/src/config/file.rs b/src/config/file.rs
@@ -1,8 +1,9 @@
 use anyhow::{Error, Result};
-use serde::Deserialize;
+use serde::{Deserialize, Deserializer};
 use std::fs;
 use std::net::IpAddr;
 use std::path::PathBuf;
+use std::str::FromStr;
 
 use super::cors::CorsConfigFile;
 use super::tls::TlsConfigFile;
@@ -12,6 +13,8 @@ pub struct ConfigFile {
     pub host: IpAddr,
     pub port: u16,
     pub verbose: Option<bool>,
+    #[serde(default = "current_working_dir")]
+    #[serde(deserialize_with = "canonicalize_some")]
     pub root_dir: Option<PathBuf>,
     pub tls: Option<TlsConfigFile>,
     pub cors: Option<CorsConfigFile>,
@@ -36,6 +39,21 @@ impl ConfigFile {
     }
 }
 
+fn canonicalize_some<'de, D>(deserializer: D) -> Result<Option<PathBuf>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let value: &str = Deserialize::deserialize(deserializer)?;
+    let path = PathBuf::from_str(value).unwrap();
+    let canon = fs::canonicalize(path).unwrap();
+
+    Ok(Some(canon))
+}
+
+fn current_working_dir() -> Option<PathBuf> {
+    std::env::current_dir().ok()
+}
+
 #[cfg(test)]
 mod tests {
     use std::net::Ipv4Addr;
@@ -51,17 +69,19 @@ mod tests {
             host = "192.168.0.1"
             port = 7878
             verbose = true
-            root_dir = "~/Desktop"
+            root_dir = "./fixtures"
         "#;
         let host = IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1));
         let port = 7878;
-        let root_dir = PathBuf::from_str("~/Desktop").unwrap();
         let config = ConfigFile::parse_toml(file_contents).unwrap();
+        let mut root_dir = PathBuf::from(std::env::current_dir().unwrap());
+
+        root_dir.push("./fixtures");
 
         assert_eq!(config.host, host);
         assert_eq!(config.port, port);
-        assert_eq!(config.root_dir.unwrap(), root_dir);
         assert_eq!(config.verbose, Some(true));
+        assert_eq!(config.root_dir, Some(root_dir));
     }
 
     #[test]
@@ -81,7 +101,6 @@ mod tests {
             host = "192.168.0.1"
             port = 7878
             verbose = false
-            root_dir = "~/Desktop"
 
             [tls]
             cert = "cert_123.pem"
@@ -90,7 +109,7 @@ mod tests {
         "#;
         let host = IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1));
         let port = 7878;
-        let root_dir = PathBuf::from_str("~/Desktop").unwrap();
+        let root_dir = Some(std::env::current_dir().unwrap());
         let tls = TlsConfigFile {
             cert: PathBuf::from_str("cert_123.pem").unwrap(),
             key: PathBuf::from_str("key_123.pem").unwrap(),
@@ -100,7 +119,7 @@ mod tests {
 
         assert_eq!(config.host, host);
         assert_eq!(config.port, port);
-        assert_eq!(config.root_dir.unwrap(), root_dir);
+        assert_eq!(config.root_dir, root_dir);
         assert_eq!(config.tls.unwrap(), tls);
         assert_eq!(config.verbose, Some(false));
     }
@@ -110,7 +129,6 @@ mod tests {
         let file_contents = r#"
             host = "192.168.0.1"
             port = 7878
-            root_dir = "~/Desktop"
 
             [tls]
             cert = "cert_123.pem"
@@ -119,7 +137,7 @@ mod tests {
         "#;
         let host = IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1));
         let port = 7878;
-        let root_dir = PathBuf::from_str("~/Desktop").unwrap();
+        let root_dir = Some(std::env::current_dir().unwrap());
         let tls = TlsConfigFile {
             cert: PathBuf::from_str("cert_123.pem").unwrap(),
             key: PathBuf::from_str("key_123.pem").unwrap(),
@@ -129,7 +147,7 @@ mod tests {
 
         assert_eq!(config.host, host);
         assert_eq!(config.port, port);
-        assert_eq!(config.root_dir.unwrap(), root_dir);
+        assert_eq!(config.root_dir, root_dir);
         assert_eq!(config.tls.unwrap(), tls);
     }
 
@@ -168,9 +186,11 @@ mod tests {
             request_method: None,
         };
         let config = ConfigFile::parse_toml(file_contents).unwrap();
+        let root_dir = Some(std::env::current_dir().unwrap());
 
         assert_eq!(config.host, host);
         assert_eq!(config.port, port);
+        assert_eq!(config.root_dir, root_dir);
         assert_eq!(config.cors.unwrap(), cors);
     }
 
@@ -192,6 +212,7 @@ mod tests {
         "#;
         let host = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0));
         let port = 8080;
+        let root_dir = Some(std::env::current_dir().unwrap());
         let cors = CorsConfigFile {
             allow_credentials: true,
             allow_headers: Some(vec![
@@ -216,6 +237,7 @@ mod tests {
 
         assert_eq!(config.host, host);
         assert_eq!(config.port, port);
+        assert_eq!(config.root_dir, root_dir);
         assert_eq!(config.cors.unwrap(), cors);
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
@@ -19,7 +19,8 @@ fn resolve_config(cli_arguments: cli::Cli) -> Result<Config> {
     }
 
     // Otherwise configuration is build from CLI arguments
-    Config::try_from(cli_arguments).with_context(|| anyhow::Error::msg("OK"))
+    Config::try_from(cli_arguments)
+        .with_context(|| anyhow::Error::msg("Failed to parse arguments from stdin"))
 }
 
 pub fn make_server() -> Result<Server> {
diff --git a/src/server/handler/file_explorer.rs b/src/server/handler/file_explorer.rs
@@ -23,7 +23,16 @@ pub fn make_file_explorer_handler(file_explorer: Arc<FileExplorer>) -> Handler {
 
         Box::pin(async move {
             if request.method() == Method::GET {
-                return file_explorer.resolve(request).await;
+                return file_explorer
+                    .resolve(request)
+                    .await
+                    .map_err(|e| {
+                        HttpResponseBuilder::new()
+                            .status(StatusCode::INTERNAL_SERVER_ERROR)
+                            .body(Body::from(e.to_string()))
+                            .expect("Unable to build response")
+                    })
+                    .unwrap();
             }
 
             HttpResponseBuilder::new()
@@ -172,42 +181,42 @@ impl<'a> FileExplorer {
     ///
     /// If the matched path resolves to a file, attempts to render it if the
     /// MIME type is supported, otherwise returns the binary (downloadable file)
-    pub async fn resolve(&self, req: Request<Body>) -> Response<Body> {
+    pub async fn resolve(&self, req: Request<Body>) -> Result<Response<Body>> {
         match resolve(&self.root_dir, &req).await.unwrap() {
-            ResolveResult::MethodNotMatched => HttpResponseBuilder::new()
+            ResolveResult::MethodNotMatched => Ok(HttpResponseBuilder::new()
                 .status(StatusCode::BAD_REQUEST)
                 .body(Body::empty())
-                .expect("Failed to build response"),
-            ResolveResult::UriNotMatched => HttpResponseBuilder::new()
+                .expect("Failed to build response")),
+            ResolveResult::UriNotMatched => Ok(HttpResponseBuilder::new()
                 .status(StatusCode::BAD_REQUEST)
                 .body(Body::empty())
-                .expect("Failed to build response"),
+                .expect("Failed to build response")),
             ResolveResult::NotFound => {
                 if req.uri() == "/" {
                     let directory_path = self.make_absolute_path_from_request(&req).unwrap();
 
-                    return self.render_directory_index(directory_path).await.unwrap();
+                    return self.render_directory_index(directory_path).await;
                 }
 
-                HttpResponseBuilder::new()
+                Ok(HttpResponseBuilder::new()
                     .status(StatusCode::NOT_FOUND)
                     .body(Body::empty())
-                    .expect("Failed to build response")
+                    .expect("Failed to build response"))
             }
-            ResolveResult::PermissionDenied => HttpResponseBuilder::new()
+            ResolveResult::PermissionDenied => Ok(HttpResponseBuilder::new()
                 .status(StatusCode::FORBIDDEN)
                 .body(Body::empty())
-                .expect("Failed to build response"),
+                .expect("Failed to build response")),
             ResolveResult::IsDirectory => {
                 let directory_path = self.make_absolute_path_from_request(&req).unwrap();
 
-                return self.render_directory_index(directory_path).await.unwrap();
+                return self.render_directory_index(directory_path).await;
             }
-            ResolveResult::Found(file, metadata, mime) => FileResponseBuilder::new()
+            ResolveResult::Found(file, metadata, mime) => Ok(FileResponseBuilder::new()
                 .request(&req)
                 .cache_headers(self.cache_headers)
                 .build(ResolveResult::Found(file, metadata, mime))
-                .expect("Failed to build response"),
+                .expect("Failed to build response")),
         }
     }
 
diff --git a/src/server/middleware/mod.rs b/src/server/middleware/mod.rs
@@ -1,5 +1,4 @@
 pub mod make_cors_middleware;
-pub mod with_cors_allow_all;
 
 use anyhow::Error;
 use futures::Future;
@@ -27,6 +26,7 @@ pub struct Middleware {
 impl Middleware {
     /// Appends a middleware function to run before handling the
     /// HTTP Request
+    #[allow(dead_code)]
     pub fn before(&mut self, middleware: MiddlewareBefore) {
         self.before.push(middleware);
     }
@@ -74,7 +74,7 @@ impl TryFrom<Config> for Middleware {
         let mut middleware = Middleware::default();
 
         if config.cors().is_some() {
-            let func = make_cors_middleware(config.clone());
+            let func = make_cors_middleware(config);
 
             middleware.after(func);
         }
diff --git a/src/server/middleware/with_cors_allow_all.rs b/src/server/middleware/with_cors_allow_all.rs

Original file line number	Diff line number	Diff line change
`@@ -68,8 +68,7 @@ impl Default for Cli {`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`mod tests {`
`71`		`- use std::str::FromStr;`
`72`		`-`
	`71`	`+ #[allow(unused_imports)]`
`73`	`72`	`use super::*;`
`74`	`73`
`75`	`74`	`#[test]`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,8 @@ fn resolve_config(cli_arguments: cli::Cli) -> Result<Config> {`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`// Otherwise configuration is build from CLI arguments`
`22`		`- Config::try_from(cli_arguments).with_context(\|\| anyhow::Error::msg("OK"))`
	`22`	`+ Config::try_from(cli_arguments)`
	`23`	`+ .with_context(\|\| anyhow::Error::msg("Failed to parse arguments from stdin"))`
`23`	`24`	`}`
`24`	`25`
`25`	`26`	`pub fn make_server() -> Result<Server> {`