add tests for resolve_path

src/config.rs

@@ -1,7 +1,11 @@
-use crate::{routes::Route, BunBunError};
+use crate::BunBunError;
 use log::{debug, error, info, trace};
-use serde::{Deserialize, Serialize};
+use serde::{
+  de::{Deserializer, Visitor},
+  Deserialize, Serialize, Serializer,
+};
 use std::collections::HashMap;
+use std::fmt;
 use std::fs::{read_to_string, OpenOptions};
 use std::io::Write;
 
@@ -22,6 +26,73 @@ pub struct RouteGroup {
   pub routes: HashMap<String, Route>,
 }
 
+#[derive(Debug, PartialEq, Clone)]
+pub enum Route {
+  External(String),
+  Path(String),
+}
+
+/// Serialization of the Route enum needs to be transparent, but since the
+/// `#[serde(transparent)]` macro isn't available on enums, so we need to
+/// implement it manually.
+impl Serialize for Route {
+  fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+  where
+    S: Serializer,
+  {
+    match self {
+      Self::External(s) => serializer.serialize_str(s),
+      Self::Path(s) => serializer.serialize_str(s),
+    }
+  }
+}
+
+/// Deserialization of the route string into the enum requires us to figure out
+/// whether or not the string is valid to run as an executable or not. To
+/// determine this, we simply check if it exists on disk or assume that it's a
+/// web path. This incurs a disk check operation, but since users shouldn't be
+/// updating the config that frequently, it should be fine.
+impl<'de> Deserialize<'de> for Route {
+  fn deserialize<D>(deserializer: D) -> Result<Route, D::Error>
+  where
+    D: Deserializer<'de>,
+  {
+    struct RouteVisitor;
+    impl<'de> Visitor<'de> for RouteVisitor {
+      type Value = Route;
+
+      fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("string")
+      }
+
+      fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
+      where
+        E: serde::de::Error,
+      {
+        // Return early if it's a path, don't go through URL parsing
+        if std::path::Path::new(value).exists() {
+          debug!("Parsed {} as a valid local path.", value);
+          Ok(Route::Path(value.into()))
+        } else {
+          debug!("{} does not exist on disk, assuming web path.", value);
+          Ok(Route::External(value.into()))
+        }
+      }
+    }
+
+    deserializer.deserialize_str(RouteVisitor)
+  }
+}
+
+impl std::fmt::Display for Route {
+  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    match self {
+      Self::External(s) => write!(f, "raw ({})", s),
+      Self::Path(s) => write!(f, "file ({})", s),
+    }
+  }
+}
+
 /// Attempts to read the config file. If it doesn't exist, generate one a
 /// default config file before attempting to parse it.
 pub fn read_config(config_file_path: &str) -> Result<Config, BunBunError> {
@@ -58,6 +129,60 @@ pub fn read_config(config_file_path: &str) -> Result<Config, BunBunError> {
   Ok(serde_yaml::from_str(&config_str)?)
 }
 
+#[cfg(test)]
+mod route {
+  use super::*;
+  use serde_yaml::{from_str, to_string};
+  use tempfile::NamedTempFile;
+
+  #[test]
+  fn deserialize_relative_path() {
+    let tmpfile = NamedTempFile::new_in(".").unwrap();
+    let path = format!("{}", tmpfile.path().display());
+    let path = path.get(path.rfind(".").unwrap()..).unwrap();
+    let path = std::path::Path::new(path);
+    assert!(path.is_relative());
+    let path = path.to_str().unwrap();
+    assert_eq!(from_str::<Route>(path).unwrap(), Route::Path(path.into()));
+  }
+
+  #[test]
+  fn deserialize_absolute_path() {
+    let tmpfile = NamedTempFile::new().unwrap();
+    let path = format!("{}", tmpfile.path().display());
+    assert!(tmpfile.path().is_absolute());
+    assert_eq!(from_str::<Route>(&path).unwrap(), Route::Path(path));
+  }
+
+  #[test]
+  fn deserialize_http_path() {
+    assert_eq!(
+      from_str::<Route>("http://google.com").unwrap(),
+      Route::External("http://google.com".into())
+    );
+  }
+
+  #[test]
+  fn deserialize_https_path() {
+    assert_eq!(
+      from_str::<Route>("https://google.com").unwrap(),
+      Route::External("https://google.com".into())
+    );
+  }
+
+  #[test]
+  fn serialize() {
+    assert_eq!(
+      &to_string(&Route::External("hello world".into())).unwrap(),
+      "---\nhello world"
+    );
+    assert_eq!(
+      &to_string(&Route::Path("hello world".into())).unwrap(),
+      "---\nhello world"
+    );
+  }
+}
+
 #[cfg(test)]
 mod read_config {
   use super::*;

src/main.rs

@@ -1,6 +1,6 @@
 #![forbid(unsafe_code)]
 
-use crate::config::{read_config, RouteGroup};
+use crate::config::{read_config, Route, RouteGroup};
 use actix_web::{middleware::Logger, App, HttpServer};
 use clap::{crate_authors, crate_version, load_yaml, App as ClapApp};
 use error::BunBunError;
@@ -24,7 +24,7 @@ pub struct State {
   default_route: Option<String>,
   groups: Vec<RouteGroup>,
   /// Cached, flattened mapping of all routes and their destinations.
-  routes: HashMap<String, routes::Route>,
+  routes: HashMap<String, Route>,
 }
 
 #[actix_rt::main]
@@ -97,7 +97,7 @@ fn init_logger(
 /// Generates a hashmap of routes from the data structure created by the config
 /// file. This should improve runtime performance and is a better solution than
 /// just iterating over the config object for every hop resolution.
-fn cache_routes(groups: &[RouteGroup]) -> HashMap<String, routes::Route> {
+fn cache_routes(groups: &[RouteGroup]) -> HashMap<String, Route> {
   let mut mapping = HashMap::new();
   for group in groups {
     for (kw, dest) in &group.routes {
@@ -218,11 +218,11 @@ mod cache_routes {
 
   fn generate_external_routes(
     routes: &[(&str, &str)],
-  ) -> HashMap<String, routes::Route> {
+  ) -> HashMap<String, Route> {
     HashMap::from_iter(
       routes
         .iter()
-        .map(|(k, v)| ((*k).into(), routes::Route::External((*v).into()))),
+        .map(|(k, v)| ((*k).into(), Route::External((*v).into()))),
     )
   }
 
@@ -287,3 +287,17 @@ mod cache_routes {
     );
   }
 }
+
+#[cfg(test)]
+mod compile_templates {
+  use super::compile_templates;
+
+  /// Successful compilation of the binary guarantees that the templates will be
+  /// present to be registered to. Thus, we only really need to see that
+  /// compilation of the templates don't panic, which is just making sure that
+  /// the function can be successfully called.
+  #[test]
+  fn templates_compile() {
+    let _ = compile_templates();
+  }
+}

src/routes.rs

@@ -1,4 +1,4 @@
-use crate::{template_args, BunBunError, State};
+use crate::{template_args, BunBunError, Route, State};
 use actix_web::web::{Data, Query};
 use actix_web::{get, http::header};
 use actix_web::{HttpRequest, HttpResponse, Responder};
@@ -6,9 +6,8 @@ use handlebars::Handlebars;
 use itertools::Itertools;
 use log::{debug, error};
 use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
-use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
+use serde::Deserialize;
 use std::collections::HashMap;
-use std::fmt;
 use std::path::PathBuf;
 use std::process::Command;
 use std::sync::{Arc, RwLock};
@@ -24,78 +23,43 @@ const FRAGMENT_ENCODE_SET: &AsciiSet = &CONTROLS
 
 type StateData = Data<Arc<RwLock<State>>>;
 
-#[derive(Debug, PartialEq, Clone)]
-pub enum Route {
-  External(String),
-  Path(String),
+#[get("/")]
+pub async fn index(data: StateData, req: HttpRequest) -> impl Responder {
+  let data = data.read().unwrap();
+  HttpResponse::Ok().body(
+    req
+      .app_data::<Handlebars>()
+      .unwrap()
+      .render(
+        "index",
+        &template_args::hostname(data.public_address.clone()),
+      )
+      .unwrap(),
+  )
 }
 
-/// Serialization of the Route enum needs to be transparent, but since the
-/// `#[serde(transparent)]` macro isn't available on enums, so we need to
-/// implement it manually.
-impl Serialize for Route {
-  fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-  where
-    S: Serializer,
-  {
-    match self {
-      Self::External(s) => serializer.serialize_str(s),
-      Self::Path(s) => serializer.serialize_str(s),
-    }
-  }
-}
-
-/// Deserialization of the route string into the enum requires us to figure out
-/// whether or not the string is valid to run as an executable or not. To
-/// determine this, we simply check if it exists on disk or assume that it's a
-/// web path. This incurs a disk check operation, but since users shouldn't be
-/// updating the config that frequently, it should be fine.
-impl<'de> Deserialize<'de> for Route {
-  fn deserialize<D>(deserializer: D) -> Result<Route, D::Error>
-  where
-    D: Deserializer<'de>,
-  {
-    struct RouteVisitor;
-    impl<'de> Visitor<'de> for RouteVisitor {
-      type Value = Route;
-
-      fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-        formatter.write_str("string")
-      }
-
-      fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
-      where
-        E: serde::de::Error,
-      {
-        // Return early if it's a path, don't go through URL parsing
-        if std::path::Path::new(value).exists() {
-          debug!("Parsed {} as a valid local path.", value);
-          Ok(Route::Path(value.into()))
-        } else {
-          debug!("{} does not exist on disk, assuming web path.", value);
-          Ok(Route::External(value.into()))
-        }
-      }
-    }
-
-    deserializer.deserialize_str(RouteVisitor)
-  }
-}
-
-impl std::fmt::Display for Route {
-  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-    match self {
-      Self::External(s) => write!(f, "raw ({})", s),
-      Self::Path(s) => write!(f, "file ({})", s),
-    }
-  }
+#[get("/bunbunsearch.xml")]
+pub async fn opensearch(data: StateData, req: HttpRequest) -> impl Responder {
+  let data = data.read().unwrap();
+  HttpResponse::Ok()
+    .header(
+      header::CONTENT_TYPE,
+      "application/opensearchdescription+xml",
+    )
+    .body(
+      req
+        .app_data::<Handlebars>()
+        .unwrap()
+        .render(
+          "opensearch",
+          &template_args::hostname(data.public_address.clone()),
+        )
+        .unwrap(),
+    )
 }
 
 #[get("/ls")]
-pub async fn list(
-  data: Data<Arc<RwLock<State>>>,
-  req: HttpRequest,
-) -> impl Responder {
+pub async fn list(data: StateData, req: HttpRequest) -> impl Responder {
   let data = data.read().unwrap();
   HttpResponse::Ok().body(
     req
@@ -206,21 +170,6 @@ fn resolve_hop<'a>(
   }
 }
 
-#[get("/")]
-pub async fn index(data: StateData, req: HttpRequest) -> impl Responder {
-  let data = data.read().unwrap();
-  HttpResponse::Ok().body(
-    req
-      .app_data::<Handlebars>()
-      .unwrap()
-      .render(
-        "index",
-        &template_args::hostname(data.public_address.clone()),
-      )
-      .unwrap(),
-  )
-}
-
 /// Runs the executable with the user's input as a single argument. Returns Ok
 /// so long as the executable was successfully executed. Returns an Error if the
 /// file doesn't exist or bunbun did not have permission to read and execute the
@@ -240,80 +189,6 @@ fn resolve_path(path: PathBuf, args: &str) -> Result<Vec<u8>, BunBunError> {
   }
 }
 
-#[get("/bunbunsearch.xml")]
-pub async fn opensearch(data: StateData, req: HttpRequest) -> impl Responder {
-  let data = data.read().unwrap();
-  HttpResponse::Ok()
-    .header(
-      header::CONTENT_TYPE,
-      "application/opensearchdescription+xml",
-    )
-    .body(
-      req
-        .app_data::<Handlebars>()
-        .unwrap()
-        .render(
-          "opensearch",
-          &template_args::hostname(data.public_address.clone()),
-        )
-        .unwrap(),
-    )
-}
-
-#[cfg(test)]
-mod route {
-  use super::*;
-  use serde_yaml::{from_str, to_string};
-  use tempfile::NamedTempFile;
-
-  #[test]
-  fn deserialize_relative_path() {
-    let tmpfile = NamedTempFile::new_in(".").unwrap();
-    let path = format!("{}", tmpfile.path().display());
-    let path = path.get(path.rfind(".").unwrap()..).unwrap();
-    let path = std::path::Path::new(path);
-    assert!(path.is_relative());
-    let path = path.to_str().unwrap();
-    assert_eq!(from_str::<Route>(path).unwrap(), Route::Path(path.into()));
-  }
-
-  #[test]
-  fn deserialize_absolute_path() {
-    let tmpfile = NamedTempFile::new().unwrap();
-    let path = format!("{}", tmpfile.path().display());
-    assert!(tmpfile.path().is_absolute());
-    assert_eq!(from_str::<Route>(&path).unwrap(), Route::Path(path));
-  }
-
-  #[test]
-  fn deserialize_http_path() {
-    assert_eq!(
-      from_str::<Route>("http://google.com").unwrap(),
-      Route::External("http://google.com".into())
-    );
-  }
-
-  #[test]
-  fn deserialize_https_path() {
-    assert_eq!(
-      from_str::<Route>("https://google.com").unwrap(),
-      Route::External("https://google.com".into())
-    );
-  }
-
-  #[test]
-  fn serialize() {
-    assert_eq!(
-      &to_string(&Route::External("hello world".into())).unwrap(),
-      "---\nhello world"
-    );
-    assert_eq!(
-      &to_string(&Route::Path("hello world".into())).unwrap(),
-      "---\nhello world"
-    );
-  }
-}
-
 #[cfg(test)]
 mod resolve_hop {
   use super::*;
@@ -393,3 +268,47 @@ mod resolve_hop {
     );
   }
 }
+
+#[cfg(test)]
+mod resolve_path {
+  use super::resolve_path;
+  use std::env::current_dir;
+  use std::path::PathBuf;
+
+  #[test]
+  fn invalid_path_returns_err() {
+    assert!(resolve_path(PathBuf::from("/bin/aaaa"), "aaaa").is_err());
+  }
+
+  #[test]
+  fn valid_path_returns_ok() {
+    assert!(resolve_path(PathBuf::from("/bin/echo"), "hello").is_ok());
+  }
+
+  #[test]
+  fn relative_path_returns_ok() {
+    // How many ".." needed to get to /
+    let nest_level = current_dir().unwrap().ancestors().count() - 1;
+    let mut rel_path = PathBuf::from("../".repeat(nest_level));
+    rel_path.push("./bin/env");
+    assert!(resolve_path(rel_path, "echo").is_ok());
+  }
+
+  #[test]
+  fn no_permissions_returns_err() {
+    assert!(
+      // Trying to run a command without permission
+      format!(
+        "{}",
+        resolve_path(PathBuf::from("/root/some_exec"), "").unwrap_err()
+      )
+      .contains("Permission denied")
+    );
+  }
+
+  #[test]
+  fn non_success_exit_code_yields_err() {
+    // cat-ing a folder always returns exit code 1
+    assert!(resolve_path(PathBuf::from("/bin/cat"), "/").is_err());
+  }
+}