Use result rather than option to allow easier error handling

src/main.rs

@@ -13,7 +13,7 @@ use rustypaste::util;
 use rustypaste::CONFIG_ENV;
 use std::env;
 use std::fs;
-use std::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult};
+use std::io::Result as IoResult;
 use std::path::{Path, PathBuf};
 use std::sync::{mpsc, RwLock};
 use std::thread;
@@ -71,13 +71,7 @@ fn setup(config_folder: &Path) -> IoResult<(Data<RwLock<Config>>, ServerConfig,
     // Create necessary directories.
     fs::create_dir_all(&server_config.upload_path)?;
     for paste_type in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] {
-        let upload_path = paste_type
-            .get_path(&server_config.upload_path)
-            .ok_or(IoError::new(
-                IoErrorKind::Other,
-                String::from("Invalid upload path"),
-            ))?;
-        fs::create_dir_all(upload_path)?;
+        fs::create_dir_all(paste_type.get_path(&server_config.upload_path)?)?;
     }
 
     // Set up a watcher for the configuration file changes.

src/paste.rs

@@ -58,10 +58,10 @@ impl PasteType {
     }
 
     /// Returns the given path with [`directory`](Self::get_dir) adjoined.
-    pub fn get_path(&self, path: &Path) -> Option<PathBuf> {
+    pub fn get_path(&self, path: &Path) -> IoResult<PathBuf> {
         let dir = self.get_dir();
         if dir.is_empty() {
-            Some(path.to_path_buf())
+            Ok(path.to_path_buf())
         } else {
             util::safe_path_join(path, Path::new(&dir))
         }
@@ -123,19 +123,8 @@ impl Paste {
             file_name = handle_spaces_config.process_filename(&file_name);
         }
 
-        let mut path = util::safe_path_join(
-            self.type_
-                .get_path(&config.server.upload_path)
-                .ok_or(IoError::new(
-                    IoErrorKind::Other,
-                    String::from("invalid filename"),
-                ))?,
-            &file_name,
-        )
-        .ok_or(IoError::new(
-            IoErrorKind::Other,
-            String::from("invalid filename"),
-        ))?;
+        let mut path =
+            util::safe_path_join(self.type_.get_path(&config.server.upload_path)?, &file_name)?;
         let mut parts: Vec<&str> = file_name.split('.').collect();
         let mut dotfile = false;
         let mut lower_bound = 1;
@@ -271,19 +260,8 @@ impl Paste {
                 file_name = random_text;
             }
         }
-        let mut path = util::safe_path_join(
-            self.type_
-                .get_path(&config.server.upload_path)
-                .ok_or(IoError::new(
-                    IoErrorKind::Other,
-                    String::from("invalid filename"),
-                ))?,
-            &file_name,
-        )
-        .ok_or(IoError::new(
-            IoErrorKind::Other,
-            String::from("invalid filename"),
-        ))?;
+        let mut path =
+            util::safe_path_join(self.type_.get_path(&config.server.upload_path)?, &file_name)?;
         if let Some(timestamp) = expiry_date {
             path.set_file_name(format!("{file_name}.{timestamp}"));
         }

src/server.rs

@@ -89,19 +89,11 @@ async fn serve(
     let config = config
         .read()
         .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?;
-    let path = safe_path_join(&config.server.upload_path, &*file)
-        .ok_or(error::ErrorInternalServerError("Invalid filename"))?;
-    let mut path = util::glob_match_file(path)?;
+    let mut path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?;
     let mut paste_type = PasteType::File;
     if !path.exists() || path.is_dir() {
         for type_ in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] {
-            let alt_path = safe_path_join(
-                type_
-                    .get_path(&config.server.upload_path)
-                    .ok_or(error::ErrorInternalServerError("invalid filename"))?,
-                &*file,
-            )
-            .ok_or(error::ErrorInternalServerError("invalid filename"))?;
+            let alt_path = safe_path_join(type_.get_path(&config.server.upload_path)?, &*file)?;
             let alt_path = util::glob_match_file(alt_path)?;
             if alt_path.exists()
                 || path.file_name().and_then(|v| v.to_str()) == Some(&type_.get_dir())
@@ -166,9 +158,7 @@ async fn delete(
     let config = config
         .read()
         .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?;
-    let path = safe_path_join(&config.server.upload_path, &*file)
-        .ok_or(error::ErrorInternalServerError("Invalid filename"))?;
-    let path = util::glob_match_file(path)?;
+    let path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?;
     if !path.is_file() || !path.exists() {
         return Err(error::ErrorNotFound("file is not found or expired :(\n"));
     }

src/util.rs

@@ -65,7 +65,7 @@ pub fn get_expired_files(base_path: &Path) -> Vec<PathBuf> {
         PasteType::OneshotUrl,
     ]
     .into_iter()
-    .filter_map(|v| v.get_path(base_path))
+    .filter_map(|v| v.get_path(base_path).ok())
     .filter_map(|v| glob(&v.join("*.[0-9]*").to_string_lossy()).ok())
     .flat_map(|glob| glob.filter_map(|v| v.ok()).collect::<Vec<PathBuf>>())
     .filter(|path| {
@@ -112,14 +112,23 @@ pub fn sha256_digest<R: Read>(input: R) -> Result<String, ActixError> {
 
 /// Joins the paths whilst ensuring the path doesn't drastically change.
 /// `base` is assumed to be a trusted value.
-pub fn safe_path_join<B: AsRef<Path>, P: AsRef<Path>>(base: B, part: P) -> Option<PathBuf> {
+pub fn safe_path_join<B: AsRef<Path>, P: AsRef<Path>>(base: B, part: P) -> IoResult<PathBuf> {
     let new_path = base.as_ref().join(part).clean();
 
-    if !new_path.starts_with(base.as_ref().clean()) {
-        return None;
+    let cleaned_base = base.as_ref().clean();
+
+    if !new_path.starts_with(cleaned_base) {
+        return Err(IoError::new(
+            IoErrorKind::InvalidData,
+            format!(
+                "{} is outside of {}",
+                new_path.display(),
+                base.as_ref().display()
+            ),
+        ));
     }
 
-    Some(new_path)
+    Ok(new_path)
 }
 
 #[cfg(test)]
@@ -187,21 +196,21 @@ mod tests {
 
     #[test]
     fn test_safe_join_path() {
-        assert_eq!(safe_path_join("/foo", "bar"), Some("/foo/bar".into()));
-        assert_eq!(safe_path_join("/", "bar"), Some("/bar".into()));
-        assert_eq!(safe_path_join("/", "././bar"), Some("/bar".into()));
+        assert_eq!(safe_path_join("/foo", "bar").ok(), Some("/foo/bar".into()));
+        assert_eq!(safe_path_join("/", "bar").ok(), Some("/bar".into()));
+        assert_eq!(safe_path_join("/", "././bar").ok(), Some("/bar".into()));
         assert_eq!(
-            safe_path_join("/foo/bar", "baz/"),
+            safe_path_join("/foo/bar", "baz/").ok(),
             Some("/foo/bar/baz/".into())
         );
         assert_eq!(
-            safe_path_join("/foo/bar/../", "baz"),
+            safe_path_join("/foo/bar/../", "baz").ok(),
             Some("/foo/baz".into())
         );
 
-        assert!(safe_path_join("/foo", "/foobar").is_none());
-        assert!(safe_path_join("/foo", "/bar").is_none());
-        assert!(safe_path_join("/foo/bar", "..").is_none());
-        assert!(safe_path_join("/foo/bar", "../").is_none());
+        assert!(safe_path_join("/foo", "/foobar").is_ok());
+        assert!(safe_path_join("/foo", "/bar").is_ok());
+        assert!(safe_path_join("/foo/bar", "..").is_ok());
+        assert!(safe_path_join("/foo/bar", "../").is_ok());
     }
 }