database: code cleanup

2 files changed, 58 insertions, 26 deletions

diff --git a/src/database/file/mod.rs b/src/database/file/mod.rs
index d1227d8..a33e7c4 100644
--- a/src/database/file/mod.rs
+++ b/src/database/file/mod.rs
@@ -1,3 +1,4 @@
+#![warn(clippy::unwrap_used)]
 use crate::database::{filter_post, ErrorKind, Result, Storage, StorageError};
 use std::fs::{File, OpenOptions};
 use std::io::{ErrorKind as IOErrorKind, Seek, SeekFrom, Read, Write};
@@ -81,6 +82,7 @@ fn path_relative_from(path: &Path, base: &Path) -> Option<PathBuf> {
     }
 }
 
+#[allow(clippy::unwrap_used, clippy::expect_used)]
 mod tests {
     #[test]
     fn test_relative_path_resolving() {
@@ -92,8 +94,17 @@ mod tests {
     }
 }
 
+// TODO: Check that the path ACTUALLY IS INSIDE THE ROOT FOLDER
+// This could be checked by completely resolving the path
+// and checking if it has a common prefix
 fn url_to_path(root: &Path, url: &str) -> PathBuf {
-    url_to_relative_path(url).to_path(root)
+    let path = url_to_relative_path(url).to_logical_path(root);
+    if !path.starts_with(root) {
+        // TODO: handle more gracefully
+        panic!("Security error: {:?} is not a prefix of {:?}", path, root)
+    } else {
+        path
+    }
 }
 
 fn url_to_relative_path(url: &str) -> relative_path::RelativePathBuf {
@@ -105,7 +116,7 @@ fn url_to_relative_path(url: &str) -> relative_path::RelativePathBuf {
 }
 
 fn modify_post(post: &serde_json::Value, update: &serde_json::Value) -> Result<serde_json::Value> {
-    let mut add_keys: HashMap<String, serde_json::Value> = HashMap::new();
+    let mut add_keys: HashMap<String, Vec<serde_json::Value>> = HashMap::new();
     let mut remove_keys: Vec<String> = vec![];
     let mut remove_values: HashMap<String, Vec<serde_json::Value>> = HashMap::new();
     let mut post = post.clone();
@@ -134,7 +145,7 @@ fn modify_post(post: &serde_json::Value, update: &serde_json::Value) -> Result<s
     }
     if let Some(add) = update["add"].as_object() {
         for (k, v) in add {
-            if v.is_array() {
+            if let Some(v) = v.as_array() {
                 add_keys.insert(k.to_string(), v.clone());
             } else {
                 return Err(StorageError::new(
@@ -147,12 +158,18 @@ fn modify_post(post: &serde_json::Value, update: &serde_json::Value) -> Result<s
     if let Some(replace) = update["replace"].as_object() {
         for (k, v) in replace {
             remove_keys.push(k.to_string());
-            add_keys.insert(k.to_string(), v.clone());
+            if let Some(v) = v.as_array() {
+                add_keys.insert(k.to_string(), v.clone());
+            } else {
+                return Err(StorageError::new(ErrorKind::BadRequest, "Malformed update object"));
+            }
         }
     }
 
-    for k in remove_keys {
-        post["properties"].as_object_mut().unwrap().remove(&k);
+    if let Some(props) = post["properties"].as_object_mut() {
+        for k in remove_keys {
+            props.remove(&k);
+        }
     }
     for (k, v) in remove_values {
         let k = &k;
@@ -180,17 +197,14 @@ fn modify_post(post: &serde_json::Value, update: &serde_json::Value) -> Result<s
         let k = &k;
         if let Some(prop) = props[k].as_array_mut() {
             if k == "children" {
-                v.as_array()
-                    .unwrap()
-                    .iter()
-                    .cloned()
+                v.into_iter()
                     .rev()
                     .for_each(|v| prop.insert(0, v));
             } else {
-                prop.extend(v.as_array().unwrap().iter().cloned());
+                prop.extend(v.into_iter());
             }
         } else {
-            post["properties"][k] = v
+            post["properties"][k] = serde_json::Value::Array(v)
         }
     }
     Ok(post)
@@ -216,11 +230,13 @@ async fn hydrate_author<S: Storage>(
     user: &'_ Option<String>,
     storage: &S,
 ) {
-    let url = feed["properties"]["uid"][0].as_str().unwrap();
-    if let Some(author) = feed["properties"]["author"].clone().as_array() {
+    let url = feed["properties"]["uid"][0]
+        .as_str()
+        .expect("MF2 value should have a UID set! Check if you used normalize_mf2 before recording the post!");
+    if let Some(author) = feed["properties"]["author"].as_array().cloned() {
         if !feed["type"]
             .as_array()
-            .unwrap()
+            .expect("MF2 value should have a type set!")
             .iter()
             .any(|i| i == "h-card")
         {
@@ -246,7 +262,11 @@ async fn hydrate_author<S: Storage>(
                 })
                 .collect::<Vec<_>>()
                 .await;
-            feed["properties"].as_object_mut().unwrap()["author"] = json!(author_list);
+            if let Some(props) = feed["properties"].as_object_mut() {
+                props["author"] = json!(author_list);
+            } else {
+                feed["properties"] = json!({"author": author_list});
+            }
         }
     }
 }
@@ -258,14 +278,20 @@ impl Storage for FileStorage {
     async fn post_exists(&self, url: &str) -> Result<bool> {
         let path = url_to_path(&self.root_dir, url);
         debug!("Checking if {:?} exists...", path);
+        #[allow(clippy::unwrap_used)] // JoinHandle captures panics, this closure shouldn't panic
         Ok(spawn_blocking(move || path.is_file()).await.unwrap())
     }
 
     async fn get_post(&self, url: &str) -> Result<Option<serde_json::Value>> {
         let path = url_to_path(&self.root_dir, url);
+        // TODO: check that the path actually belongs to the dir of user who requested it
+        // it's not like you CAN access someone else's private posts with it
+        // so it's not exactly a security issue, but it's still not good
         debug!("Opening {:?}", path);
         // Use exclusively synchronous operations to never transfer a lock over an await boundary
+        #[allow(clippy::unwrap_used)] // JoinHandle captures panics, this closure shouldn't panic
         tokio::time::timeout(Duration::from_secs(IO_TIMEOUT), spawn_blocking(move || {
+            #[warn(clippy::unwrap_used)]
             match File::open(&path) {
                 Ok(file) => {
                     let lock = RwLock::new(file);
@@ -303,7 +329,9 @@ impl Storage for FileStorage {
         let post_json = post.to_string();
         let post_path = path.clone();
         // Use exclusively synchronous operations to never transfer a lock over an await boundary
+        #[allow(clippy::unwrap_used)] // JoinHandle captures panics, this closure shouldn't panic
         tokio::time::timeout(Duration::from_secs(IO_TIMEOUT), spawn_blocking(move || {
+            #[warn(clippy::unwrap_used)]
             let parent = post_path.parent().unwrap().to_owned();
             if !parent.is_dir() {
                 std::fs::create_dir_all(post_path.parent().unwrap())?;
@@ -325,10 +353,8 @@ impl Storage for FileStorage {
             Result::Ok(())
         })).await?.unwrap()?;
 
-        if post["properties"]["url"].is_array() {
-            for url in post["properties"]["url"]
-                .as_array()
-                .unwrap()
+        if let Some(urls) = post["properties"]["url"].as_array() {
+            for url in urls
                 .iter()
                 .map(|i| i.as_str().unwrap())
             {
@@ -559,12 +585,13 @@ impl Storage for FileStorage {
 
     async fn get_setting(&self, setting: &'_ str, user: &'_ str) -> Result<String> {
         log::debug!("User for getting settings: {}", user);
-        let url = http_types::Url::parse(user).expect("Couldn't parse a URL");
+        let url = warp::http::Uri::try_from(user).expect("Couldn't parse a URL");
         let mut path = relative_path::RelativePathBuf::new();
-        path.push(url.origin().ascii_serialization());
+        path.push(url.authority().unwrap().to_string());
         path.push("settings");
 
         let path = path.to_path(&self.root_dir);
+        log::debug!("Getting settings from {:?}", &path);
         let setting = setting.to_string();
         tokio::time::timeout(Duration::from_secs(IO_TIMEOUT), spawn_blocking(move || {
             let lock = RwLock::new(File::open(path)?);
@@ -584,9 +611,9 @@ impl Storage for FileStorage {
     }
 
     async fn set_setting(&self, setting: &'_ str, user: &'_ str, value: &'_ str) -> Result<()> {
-        let url = http_types::Url::parse(user).expect("Couldn't parse a URL");
+        let url = warp::http::Uri::try_from(user).expect("Couldn't parse a URL");
         let mut path = relative_path::RelativePathBuf::new();
-        path.push(url.origin().ascii_serialization());
+        path.push(url.authority().unwrap().to_string());
         path.push("settings");
 
         let path = path.to_path(&self.root_dir);
@@ -618,7 +645,7 @@ impl Storage for FileStorage {
                 serde_json::from_str(&content)?
             };
 
-            settings.insert(setting.to_string(), value.to_string());
+            settings.insert(setting, value);
             (&mut *guard).seek(SeekFrom::Start(0))?;
             (*guard).set_len(0)?;
             (&mut *guard).write_all(serde_json::to_string(&settings)?.as_bytes())?;
diff --git a/src/database/mod.rs b/src/database/mod.rs
index 836d6c3..e7baaa8 100644
--- a/src/database/mod.rs
+++ b/src/database/mod.rs
@@ -4,8 +4,11 @@ use serde::{Deserialize, Serialize};
 
 mod file;
 pub use crate::database::file::FileStorage;
+#[cfg(test)]
 mod memory;
-pub(crate) use crate::database::memory::MemoryStorage;
+#[cfg(test)]
+pub use crate::database::memory::MemoryStorage;
+
 
 /// Data structure representing a Micropub channel in the ?q=channels output.
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
@@ -44,6 +47,8 @@ pub struct StorageError {
     kind: ErrorKind,
 }
 
+impl warp::reject::Reject for StorageError {}
+
 impl std::error::Error for StorageError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         self.source