From 489c7d04c824339faf085418c48bd44c75a6da80 Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Thu, 23 Mar 2023 07:35:31 +0800
Subject: Create shared file utiity module

---
 src/config.rs      |  2 +-
 src/file_upload.rs | 87 ++----------------------------------------------------
 src/file_utils.rs  | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/main.rs        |  1 +
 4 files changed, 88 insertions(+), 86 deletions(-)
 create mode 100644 src/file_utils.rs

(limited to 'src')

diff --git a/src/config.rs b/src/config.rs
index 8976d35..d52b231 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -16,7 +16,7 @@ use rustls_pemfile as pemfile;
 use crate::{
     args::{CliArgs, MediaType},
     auth::RequiredAuth,
-    file_upload::sanitize_path,
+    file_utils::sanitize_path,
     renderer::ThemeSlug,
 };
 
diff --git a/src/file_upload.rs b/src/file_upload.rs
index 2275c73..b9974aa 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -6,8 +6,8 @@ use std::{
 use actix_web::{http::header, HttpRequest, HttpResponse};
 use futures::TryStreamExt;
 
-use crate::errors::ContextualError;
-use crate::listing;
+use crate::{errors::ContextualError, file_utils::sanitize_path};
+use crate::{file_utils::contains_symlink, listing};
 
 /// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
 /// existing file.
@@ -214,86 +214,3 @@ pub async fn upload_file(
         .append_header((header::LOCATION, return_path))
         .finish())
 }
-
-/// Guarantee that the path is relative and cannot traverse back to parent directories
-/// and optionally prevent traversing hidden directories.
-///
-/// See the unit tests tests::test_sanitize_path* for examples
-pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> {
-    let mut buf = PathBuf::new();
-
-    for comp in path.components() {
-        match comp {
-            Component::Normal(name) => buf.push(name),
-            Component::ParentDir => {
-                buf.pop();
-            }
-            _ => (),
-        }
-    }
-
-    // Double-check that all components are Normal and check for hidden dirs
-    for comp in buf.components() {
-        match comp {
-            Component::Normal(_) if traverse_hidden => (),
-            Component::Normal(name) if !name.to_str()?.starts_with('.') => (),
-            _ => return None,
-        }
-    }
-
-    Some(buf)
-}
-
-/// Returns if a path goes through a symolic link
-fn contains_symlink(path: &PathBuf) -> bool {
-    let mut joined_path = PathBuf::new();
-    for path_slice in path {
-        joined_path = joined_path.join(path_slice);
-        if !joined_path.exists() {
-            // On Windows, `\\?\` won't exist even though it's the root
-            // So, we can't just return here
-            // But we don't need to check if it's a symlink since it won't be
-            continue;
-        }
-        if joined_path
-            .symlink_metadata()
-            .map(|m| m.file_type().is_symlink())
-            .unwrap_or(false)
-        {
-            return true;
-        }
-    }
-    false
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use pretty_assertions::assert_eq;
-    use rstest::rstest;
-
-    #[rstest]
-    #[case("/foo", "foo")]
-    #[case("////foo", "foo")]
-    #[case("C:/foo", if cfg!(windows) { "foo" } else { "C:/foo" })]
-    #[case("../foo", "foo")]
-    #[case("../foo/../bar/abc", "bar/abc")]
-    fn test_sanitize_path(#[case] input: &str, #[case] output: &str) {
-        assert_eq!(
-            sanitize_path(Path::new(input), true).unwrap(),
-            Path::new(output)
-        );
-        assert_eq!(
-            sanitize_path(Path::new(input), false).unwrap(),
-            Path::new(output)
-        );
-    }
-
-    #[rstest]
-    #[case(".foo")]
-    #[case("/.foo")]
-    #[case("foo/.bar/foo")]
-    fn test_sanitize_path_no_hidden_files(#[case] input: &str) {
-        assert_eq!(sanitize_path(Path::new(input), false), None);
-    }
-}
diff --git a/src/file_utils.rs b/src/file_utils.rs
new file mode 100644
index 0000000..fdd68ba
--- /dev/null
+++ b/src/file_utils.rs
@@ -0,0 +1,84 @@
+use std::path::{Component, Path, PathBuf};
+
+/// Guarantee that the path is relative and cannot traverse back to parent directories
+/// and optionally prevent traversing hidden directories.
+///
+/// See the unit tests tests::test_sanitize_path* for examples
+pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> {
+    let mut buf = PathBuf::new();
+
+    for comp in path.components() {
+        match comp {
+            Component::Normal(name) => buf.push(name),
+            Component::ParentDir => {
+                buf.pop();
+            }
+            _ => (),
+        }
+    }
+
+    // Double-check that all components are Normal and check for hidden dirs
+    for comp in buf.components() {
+        match comp {
+            Component::Normal(_) if traverse_hidden => (),
+            Component::Normal(name) if !name.to_str()?.starts_with('.') => (),
+            _ => return None,
+        }
+    }
+
+    Some(buf)
+}
+
+/// Returns if a path goes through a symbolic link
+pub fn contains_symlink(path: &PathBuf) -> bool {
+    let mut joined_path = PathBuf::new();
+    for path_slice in path {
+        joined_path = joined_path.join(path_slice);
+        if !joined_path.exists() {
+            // On Windows, `\\?\` won't exist even though it's the root
+            // So, we can't just return here
+            // But we don't need to check if it's a symlink since it won't be
+            continue;
+        }
+        if joined_path
+            .symlink_metadata()
+            .map(|m| m.file_type().is_symlink())
+            .unwrap_or(false)
+        {
+            return true;
+        }
+    }
+    false
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use pretty_assertions::assert_eq;
+    use rstest::rstest;
+
+    #[rstest]
+    #[case("/foo", "foo")]
+    #[case("////foo", "foo")]
+    #[case("C:/foo", if cfg!(windows) { "foo" } else { "C:/foo" })]
+    #[case("../foo", "foo")]
+    #[case("../foo/../bar/abc", "bar/abc")]
+    fn test_sanitize_path(#[case] input: &str, #[case] output: &str) {
+        assert_eq!(
+            sanitize_path(Path::new(input), true).unwrap(),
+            Path::new(output)
+        );
+        assert_eq!(
+            sanitize_path(Path::new(input), false).unwrap(),
+            Path::new(output)
+        );
+    }
+
+    #[rstest]
+    #[case(".foo")]
+    #[case("/.foo")]
+    #[case("foo/.bar/foo")]
+    fn test_sanitize_path_no_hidden_files(#[case] input: &str) {
+        assert_eq!(sanitize_path(Path::new(input), false), None);
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index 2f81baa..a492f4d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -21,6 +21,7 @@ mod config;
 mod consts;
 mod errors;
 mod file_upload;
+mod file_utils;
 mod listing;
 mod pipe;
 mod renderer;
-- 
cgit v1.2.3


From 03915c3d33a6a053a1455a2ba48ad57e0477365a Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Thu, 23 Mar 2023 07:39:00 +0800
Subject: Make file util functions generic

---
 src/file_utils.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/file_utils.rs b/src/file_utils.rs
index fdd68ba..ba976de 100644
--- a/src/file_utils.rs
+++ b/src/file_utils.rs
@@ -4,10 +4,10 @@ use std::path::{Component, Path, PathBuf};
 /// and optionally prevent traversing hidden directories.
 ///
 /// See the unit tests tests::test_sanitize_path* for examples
-pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> {
+pub fn sanitize_path(path: impl AsRef<Path>, traverse_hidden: bool) -> Option<PathBuf> {
     let mut buf = PathBuf::new();
 
-    for comp in path.components() {
+    for comp in path.as_ref().components() {
         match comp {
             Component::Normal(name) => buf.push(name),
             Component::ParentDir => {
@@ -30,9 +30,9 @@ pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> {
 }
 
 /// Returns if a path goes through a symbolic link
-pub fn contains_symlink(path: &PathBuf) -> bool {
+pub fn contains_symlink(path: impl AsRef<Path>) -> bool {
     let mut joined_path = PathBuf::new();
-    for path_slice in path {
+    for path_slice in path.as_ref().iter() {
         joined_path = joined_path.join(path_slice);
         if !joined_path.exists() {
             // On Windows, `\\?\` won't exist even though it's the root
-- 
cgit v1.2.3


From cea3eabae7c369ec658745ba3f8e4328a4de975f Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 13:37:18 +0800
Subject: Rewrite `contains_symlink`

---
 src/file_upload.rs | 28 ++++++++++++++++++++--------
 src/file_utils.rs  | 42 +++++++++++++++++++++---------------------
 2 files changed, 41 insertions(+), 29 deletions(-)

(limited to 'src')

diff --git a/src/file_upload.rs b/src/file_upload.rs
index b9974aa..919ac2c 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -110,10 +110,16 @@ async fn handle_multipart(
         })?;
 
         // Ensure there are no illegal symlinks
-        if !allow_symlinks && contains_symlink(&absolute_path) {
-            return Err(ContextualError::InsufficientPermissionsError(
-                user_given_path.display().to_string(),
-            ));
+        if !allow_symlinks {
+            match contains_symlink(&absolute_path) {
+                Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                    err.to_string(),
+                ))?,
+                Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                    "{user_given_path:?} traverses through a symlink"
+                )))?,
+                Ok(false) => (),
+            }
         }
 
         std::fs::create_dir_all(&absolute_path).map_err(|e| {
@@ -135,10 +141,16 @@ async fn handle_multipart(
     })?;
 
     // Ensure there are no illegal symlinks in the file upload path
-    if !allow_symlinks && contains_symlink(&path) {
-        return Err(ContextualError::InsufficientPermissionsError(
-            filename.to_string(),
-        ));
+    if !allow_symlinks {
+        match contains_symlink(&path) {
+            Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                err.to_string(),
+            ))?,
+            Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                "{path:?} traverses through a symlink"
+            )))?,
+            Ok(false) => (),
+        }
     }
 
     save_file(field, path.join(filename_path), overwrite_files).await
diff --git a/src/file_utils.rs b/src/file_utils.rs
index ba976de..114a08f 100644
--- a/src/file_utils.rs
+++ b/src/file_utils.rs
@@ -1,4 +1,7 @@
-use std::path::{Component, Path, PathBuf};
+use std::{
+    io,
+    path::{Component, Path, PathBuf},
+};
 
 /// Guarantee that the path is relative and cannot traverse back to parent directories
 /// and optionally prevent traversing hidden directories.
@@ -29,26 +32,23 @@ pub fn sanitize_path(path: impl AsRef<Path>, traverse_hidden: bool) -> Option<Pa
     Some(buf)
 }
 
-/// Returns if a path goes through a symbolic link
-pub fn contains_symlink(path: impl AsRef<Path>) -> bool {
-    let mut joined_path = PathBuf::new();
-    for path_slice in path.as_ref().iter() {
-        joined_path = joined_path.join(path_slice);
-        if !joined_path.exists() {
-            // On Windows, `\\?\` won't exist even though it's the root
-            // So, we can't just return here
-            // But we don't need to check if it's a symlink since it won't be
-            continue;
-        }
-        if joined_path
-            .symlink_metadata()
-            .map(|m| m.file_type().is_symlink())
-            .unwrap_or(false)
-        {
-            return true;
-        }
-    }
-    false
+/// Checks if any segment of the path is a symlink.
+///
+/// This function fails if [`std::fs::symlink_metadata`] fails, which usually
+/// means user has no permission to access the path.
+pub fn contains_symlink(path: impl AsRef<Path>) -> io::Result<bool> {
+    let contains_symlink = path
+        .as_ref()
+        .ancestors()
+        // On Windows, `\\?\` won't exist even though it's the root, but there's no need to check it
+        // So we filter it out
+        .filter(|p| p.exists())
+        .map(|p| p.symlink_metadata())
+        .collect::<Result<Vec<_>, _>>()?
+        .into_iter()
+        .any(|p| p.file_type().is_symlink());
+
+    Ok(contains_symlink)
 }
 
 #[cfg(test)]
-- 
cgit v1.2.3


From d84055d0def5d0a7c6d78277f73746e67e22ee14 Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 14:54:48 +0800
Subject: rename `file_upload` to `file_op`

- This is in preparation for adding deletion code
---
 src/file_op.rs     | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/file_upload.rs | 228 ----------------------------------------------------
 src/main.rs        |   4 +-
 3 files changed, 232 insertions(+), 230 deletions(-)
 create mode 100644 src/file_op.rs
 delete mode 100644 src/file_upload.rs

(limited to 'src')

diff --git a/src/file_op.rs b/src/file_op.rs
new file mode 100644
index 0000000..df71be5
--- /dev/null
+++ b/src/file_op.rs
@@ -0,0 +1,230 @@
+//! Handlers for file upload and removal
+
+use std::{
+    io::Write,
+    path::{Component, Path, PathBuf},
+};
+
+use actix_web::{http::header, HttpRequest, HttpResponse};
+use futures::TryStreamExt;
+
+use crate::{errors::ContextualError, file_utils::sanitize_path};
+use crate::{file_utils::contains_symlink, listing};
+
+/// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
+/// existing file.
+///
+/// Returns total bytes written to file.
+async fn save_file(
+    field: actix_multipart::Field,
+    file_path: PathBuf,
+    overwrite_files: bool,
+) -> Result<u64, ContextualError> {
+    if !overwrite_files && file_path.exists() {
+        return Err(ContextualError::DuplicateFileError);
+    }
+
+    let file = std::fs::File::create(&file_path).map_err(|e| {
+        ContextualError::IoError(format!("Failed to create {}", file_path.display()), e)
+    })?;
+
+    let (_, written_len) = field
+        .map_err(ContextualError::MultipartError)
+        .try_fold((file, 0u64), |(mut file, written_len), bytes| async move {
+            file.write_all(bytes.as_ref())
+                .map_err(|e| ContextualError::IoError("Failed to write to file".to_string(), e))?;
+            Ok((file, written_len + bytes.len() as u64))
+        })
+        .await?;
+
+    Ok(written_len)
+}
+
+/// Handles a single field in a multipart form
+async fn handle_multipart(
+    mut field: actix_multipart::Field,
+    path: PathBuf,
+    overwrite_files: bool,
+    allow_mkdir: bool,
+    allow_hidden_paths: bool,
+    allow_symlinks: bool,
+) -> Result<u64, ContextualError> {
+    let field_name = field.name().to_string();
+
+    match std::fs::metadata(&path) {
+        Err(_) => Err(ContextualError::InsufficientPermissionsError(
+            path.display().to_string(),
+        )),
+        Ok(metadata) if !metadata.is_dir() => Err(ContextualError::InvalidPathError(format!(
+            "cannot upload file to {}, since it's not a directory",
+            &path.display()
+        ))),
+        Ok(metadata) if metadata.permissions().readonly() => Err(
+            ContextualError::InsufficientPermissionsError(path.display().to_string()),
+        ),
+        Ok(_) => Ok(()),
+    }?;
+
+    if field_name == "mkdir" {
+        if !allow_mkdir {
+            return Err(ContextualError::InsufficientPermissionsError(
+                path.display().to_string(),
+            ));
+        }
+
+        let mut user_given_path = PathBuf::new();
+        let mut absolute_path = path.clone();
+
+        // Get the path the user gave
+        let mkdir_path_bytes = field.try_next().await;
+        match mkdir_path_bytes {
+            Ok(Some(mkdir_path_bytes)) => {
+                let mkdir_path = std::str::from_utf8(&mkdir_path_bytes).map_err(|e| {
+                    ContextualError::ParseError(
+                        "Failed to parse 'mkdir' path".to_string(),
+                        e.to_string(),
+                    )
+                })?;
+                let mkdir_path = mkdir_path.replace('\\', "/");
+                absolute_path.push(&mkdir_path);
+                user_given_path.push(&mkdir_path);
+            }
+            _ => {
+                return Err(ContextualError::ParseError(
+                    "Failed to parse 'mkdir' path".to_string(),
+                    "".to_string(),
+                ))
+            }
+        };
+
+        // Disallow using `..` (parent) in mkdir path
+        if user_given_path
+            .components()
+            .any(|c| c == Component::ParentDir)
+        {
+            return Err(ContextualError::InvalidPathError(
+                "Cannot use '..' in mkdir path".to_string(),
+            ));
+        }
+        // Hidden paths check
+        sanitize_path(&user_given_path, allow_hidden_paths).ok_or_else(|| {
+            ContextualError::InvalidPathError("Cannot use hidden paths in mkdir path".to_string())
+        })?;
+
+        // Ensure there are no illegal symlinks
+        if !allow_symlinks {
+            match contains_symlink(&absolute_path) {
+                Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                    err.to_string(),
+                ))?,
+                Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                    "{user_given_path:?} traverses through a symlink"
+                )))?,
+                Ok(false) => (),
+            }
+        }
+
+        std::fs::create_dir_all(&absolute_path).map_err(|e| {
+            ContextualError::IoError(format!("Failed to create {}", user_given_path.display()), e)
+        })?;
+
+        return Ok(0);
+    }
+
+    let filename = field.content_disposition().get_filename().ok_or_else(|| {
+        ContextualError::ParseError(
+            "HTTP header".to_string(),
+            "Failed to retrieve the name of the file to upload".to_string(),
+        )
+    })?;
+
+    let filename_path = sanitize_path(Path::new(&filename), false).ok_or_else(|| {
+        ContextualError::InvalidPathError("Invalid file name to upload".to_string())
+    })?;
+
+    // Ensure there are no illegal symlinks in the file upload path
+    if !allow_symlinks {
+        match contains_symlink(&path) {
+            Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                err.to_string(),
+            ))?,
+            Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                "{path:?} traverses through a symlink"
+            )))?,
+            Ok(false) => (),
+        }
+    }
+
+    save_file(field, path.join(filename_path), overwrite_files).await
+}
+
+/// Handle incoming request to upload a file or create a directory.
+/// Target file path is expected as path parameter in URI and is interpreted as relative from
+/// server root directory. Any path which will go outside of this directory is considered
+/// invalid.
+/// This method returns future.
+pub async fn upload_file(
+    req: HttpRequest,
+    payload: actix_web::web::Payload,
+) -> Result<HttpResponse, ContextualError> {
+    let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
+    let return_path = if let Some(header) = req.headers().get(header::REFERER) {
+        header.to_str().unwrap_or("/").to_owned()
+    } else {
+        "/".to_string()
+    };
+
+    let query_params = listing::extract_query_parameters(&req);
+    let upload_path = query_params.path.as_ref().ok_or_else(|| {
+        ContextualError::InvalidHttpRequestError("Missing query parameter 'path'".to_string())
+    })?;
+    let upload_path = sanitize_path(upload_path, conf.show_hidden).ok_or_else(|| {
+        ContextualError::InvalidPathError("Invalid value for 'path' parameter".to_string())
+    })?;
+
+    let app_root_dir = conf.path.canonicalize().map_err(|e| {
+        ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e)
+    })?;
+
+    // Disallow paths outside of allowed directories
+    let upload_allowed = conf.allowed_upload_dir.is_empty()
+        || conf
+            .allowed_upload_dir
+            .iter()
+            .any(|s| upload_path.starts_with(s));
+
+    if !upload_allowed {
+        return Err(ContextualError::UploadForbiddenError);
+    }
+
+    // Disallow the target path to go outside of the served directory
+    // The target directory shouldn't be canonicalized when it gets passed to
+    // handle_multipart so that it can check for symlinks if needed
+    let non_canonicalized_target_dir = app_root_dir.join(upload_path);
+    match non_canonicalized_target_dir.canonicalize() {
+        Ok(path) if !conf.no_symlinks => Ok(path),
+        Ok(path) if path.starts_with(&app_root_dir) => Ok(path),
+        _ => Err(ContextualError::InvalidHttpRequestError(
+            "Invalid value for 'path' parameter".to_string(),
+        )),
+    }?;
+
+    actix_multipart::Multipart::new(req.headers(), payload)
+        .map_err(ContextualError::MultipartError)
+        .and_then(|field| {
+            handle_multipart(
+                field,
+                non_canonicalized_target_dir.clone(),
+                conf.overwrite_files,
+                conf.mkdir_enabled,
+                conf.show_hidden,
+                !conf.no_symlinks,
+            )
+        })
+        .try_collect::<Vec<u64>>()
+        .await?;
+
+    Ok(HttpResponse::SeeOther()
+        .append_header((header::LOCATION, return_path))
+        .finish())
+}
diff --git a/src/file_upload.rs b/src/file_upload.rs
deleted file mode 100644
index 919ac2c..0000000
--- a/src/file_upload.rs
+++ /dev/null
@@ -1,228 +0,0 @@
-use std::{
-    io::Write,
-    path::{Component, Path, PathBuf},
-};
-
-use actix_web::{http::header, HttpRequest, HttpResponse};
-use futures::TryStreamExt;
-
-use crate::{errors::ContextualError, file_utils::sanitize_path};
-use crate::{file_utils::contains_symlink, listing};
-
-/// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
-/// existing file.
-///
-/// Returns total bytes written to file.
-async fn save_file(
-    field: actix_multipart::Field,
-    file_path: PathBuf,
-    overwrite_files: bool,
-) -> Result<u64, ContextualError> {
-    if !overwrite_files && file_path.exists() {
-        return Err(ContextualError::DuplicateFileError);
-    }
-
-    let file = std::fs::File::create(&file_path).map_err(|e| {
-        ContextualError::IoError(format!("Failed to create {}", file_path.display()), e)
-    })?;
-
-    let (_, written_len) = field
-        .map_err(ContextualError::MultipartError)
-        .try_fold((file, 0u64), |(mut file, written_len), bytes| async move {
-            file.write_all(bytes.as_ref())
-                .map_err(|e| ContextualError::IoError("Failed to write to file".to_string(), e))?;
-            Ok((file, written_len + bytes.len() as u64))
-        })
-        .await?;
-
-    Ok(written_len)
-}
-
-/// Handles a single field in a multipart form
-async fn handle_multipart(
-    mut field: actix_multipart::Field,
-    path: PathBuf,
-    overwrite_files: bool,
-    allow_mkdir: bool,
-    allow_hidden_paths: bool,
-    allow_symlinks: bool,
-) -> Result<u64, ContextualError> {
-    let field_name = field.name().to_string();
-
-    match std::fs::metadata(&path) {
-        Err(_) => Err(ContextualError::InsufficientPermissionsError(
-            path.display().to_string(),
-        )),
-        Ok(metadata) if !metadata.is_dir() => Err(ContextualError::InvalidPathError(format!(
-            "cannot upload file to {}, since it's not a directory",
-            &path.display()
-        ))),
-        Ok(metadata) if metadata.permissions().readonly() => Err(
-            ContextualError::InsufficientPermissionsError(path.display().to_string()),
-        ),
-        Ok(_) => Ok(()),
-    }?;
-
-    if field_name == "mkdir" {
-        if !allow_mkdir {
-            return Err(ContextualError::InsufficientPermissionsError(
-                path.display().to_string(),
-            ));
-        }
-
-        let mut user_given_path = PathBuf::new();
-        let mut absolute_path = path.clone();
-
-        // Get the path the user gave
-        let mkdir_path_bytes = field.try_next().await;
-        match mkdir_path_bytes {
-            Ok(Some(mkdir_path_bytes)) => {
-                let mkdir_path = std::str::from_utf8(&mkdir_path_bytes).map_err(|e| {
-                    ContextualError::ParseError(
-                        "Failed to parse 'mkdir' path".to_string(),
-                        e.to_string(),
-                    )
-                })?;
-                let mkdir_path = mkdir_path.replace('\\', "/");
-                absolute_path.push(&mkdir_path);
-                user_given_path.push(&mkdir_path);
-            }
-            _ => {
-                return Err(ContextualError::ParseError(
-                    "Failed to parse 'mkdir' path".to_string(),
-                    "".to_string(),
-                ))
-            }
-        };
-
-        // Disallow using `..` (parent) in mkdir path
-        if user_given_path
-            .components()
-            .any(|c| c == Component::ParentDir)
-        {
-            return Err(ContextualError::InvalidPathError(
-                "Cannot use '..' in mkdir path".to_string(),
-            ));
-        }
-        // Hidden paths check
-        sanitize_path(&user_given_path, allow_hidden_paths).ok_or_else(|| {
-            ContextualError::InvalidPathError("Cannot use hidden paths in mkdir path".to_string())
-        })?;
-
-        // Ensure there are no illegal symlinks
-        if !allow_symlinks {
-            match contains_symlink(&absolute_path) {
-                Err(err) => Err(ContextualError::InsufficientPermissionsError(
-                    err.to_string(),
-                ))?,
-                Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
-                    "{user_given_path:?} traverses through a symlink"
-                )))?,
-                Ok(false) => (),
-            }
-        }
-
-        std::fs::create_dir_all(&absolute_path).map_err(|e| {
-            ContextualError::IoError(format!("Failed to create {}", user_given_path.display()), e)
-        })?;
-
-        return Ok(0);
-    }
-
-    let filename = field.content_disposition().get_filename().ok_or_else(|| {
-        ContextualError::ParseError(
-            "HTTP header".to_string(),
-            "Failed to retrieve the name of the file to upload".to_string(),
-        )
-    })?;
-
-    let filename_path = sanitize_path(Path::new(&filename), false).ok_or_else(|| {
-        ContextualError::InvalidPathError("Invalid file name to upload".to_string())
-    })?;
-
-    // Ensure there are no illegal symlinks in the file upload path
-    if !allow_symlinks {
-        match contains_symlink(&path) {
-            Err(err) => Err(ContextualError::InsufficientPermissionsError(
-                err.to_string(),
-            ))?,
-            Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
-                "{path:?} traverses through a symlink"
-            )))?,
-            Ok(false) => (),
-        }
-    }
-
-    save_file(field, path.join(filename_path), overwrite_files).await
-}
-
-/// Handle incoming request to upload a file or create a directory.
-/// Target file path is expected as path parameter in URI and is interpreted as relative from
-/// server root directory. Any path which will go outside of this directory is considered
-/// invalid.
-/// This method returns future.
-pub async fn upload_file(
-    req: HttpRequest,
-    payload: actix_web::web::Payload,
-) -> Result<HttpResponse, ContextualError> {
-    let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
-    let return_path = if let Some(header) = req.headers().get(header::REFERER) {
-        header.to_str().unwrap_or("/").to_owned()
-    } else {
-        "/".to_string()
-    };
-
-    let query_params = listing::extract_query_parameters(&req);
-    let upload_path = query_params.path.as_ref().ok_or_else(|| {
-        ContextualError::InvalidHttpRequestError("Missing query parameter 'path'".to_string())
-    })?;
-    let upload_path = sanitize_path(upload_path, conf.show_hidden).ok_or_else(|| {
-        ContextualError::InvalidPathError("Invalid value for 'path' parameter".to_string())
-    })?;
-
-    let app_root_dir = conf.path.canonicalize().map_err(|e| {
-        ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e)
-    })?;
-
-    // Disallow paths outside of allowed directories
-    let upload_allowed = conf.allowed_upload_dir.is_empty()
-        || conf
-            .allowed_upload_dir
-            .iter()
-            .any(|s| upload_path.starts_with(s));
-
-    if !upload_allowed {
-        return Err(ContextualError::UploadForbiddenError);
-    }
-
-    // Disallow the target path to go outside of the served directory
-    // The target directory shouldn't be canonicalized when it gets passed to
-    // handle_multipart so that it can check for symlinks if needed
-    let non_canonicalized_target_dir = app_root_dir.join(upload_path);
-    match non_canonicalized_target_dir.canonicalize() {
-        Ok(path) if !conf.no_symlinks => Ok(path),
-        Ok(path) if path.starts_with(&app_root_dir) => Ok(path),
-        _ => Err(ContextualError::InvalidHttpRequestError(
-            "Invalid value for 'path' parameter".to_string(),
-        )),
-    }?;
-
-    actix_multipart::Multipart::new(req.headers(), payload)
-        .map_err(ContextualError::MultipartError)
-        .and_then(|field| {
-            handle_multipart(
-                field,
-                non_canonicalized_target_dir.clone(),
-                conf.overwrite_files,
-                conf.mkdir_enabled,
-                conf.show_hidden,
-                !conf.no_symlinks,
-            )
-        })
-        .try_collect::<Vec<u64>>()
-        .await?;
-
-    Ok(HttpResponse::SeeOther()
-        .append_header((header::LOCATION, return_path))
-        .finish())
-}
diff --git a/src/main.rs b/src/main.rs
index a492f4d..2a4d402 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -20,7 +20,7 @@ mod auth;
 mod config;
 mod consts;
 mod errors;
-mod file_upload;
+mod file_op;
 mod file_utils;
 mod listing;
 mod pipe;
@@ -328,7 +328,7 @@ fn configure_app(app: &mut web::ServiceConfig, conf: &MiniserveConfig) {
     } else {
         if conf.file_upload {
             // Allow file upload
-            app.service(web::resource("/upload").route(web::post().to(file_upload::upload_file)));
+            app.service(web::resource("/upload").route(web::post().to(file_op::upload_file)));
         }
         // Handle directories
         app.service(dir_service());
-- 
cgit v1.2.3


From 6b5a46bbb9a101df555cbf45c56e759e2bfa6cd5 Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 15:01:07 +0800
Subject: Use distinct query type for file op APIs

---
 src/file_op.rs  | 21 ++++++++++++---------
 src/listing.rs  | 13 ++++++-------
 src/renderer.rs |  4 ++--
 3 files changed, 20 insertions(+), 18 deletions(-)

(limited to 'src')

diff --git a/src/file_op.rs b/src/file_op.rs
index df71be5..901c1d6 100644
--- a/src/file_op.rs
+++ b/src/file_op.rs
@@ -5,11 +5,12 @@ use std::{
     path::{Component, Path, PathBuf},
 };
 
-use actix_web::{http::header, HttpRequest, HttpResponse};
+use actix_web::{http::header, web, HttpRequest, HttpResponse};
 use futures::TryStreamExt;
+use serde::Deserialize;
 
+use crate::file_utils::contains_symlink;
 use crate::{errors::ContextualError, file_utils::sanitize_path};
-use crate::{file_utils::contains_symlink, listing};
 
 /// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
 /// existing file.
@@ -158,6 +159,12 @@ async fn handle_multipart(
     save_file(field, path.join(filename_path), overwrite_files).await
 }
 
+/// Query parameters used by upload and rm APIs
+#[derive(Deserialize, Default)]
+pub struct FileOpQueryParameters {
+    path: PathBuf,
+}
+
 /// Handle incoming request to upload a file or create a directory.
 /// Target file path is expected as path parameter in URI and is interpreted as relative from
 /// server root directory. Any path which will go outside of this directory is considered
@@ -165,7 +172,8 @@ async fn handle_multipart(
 /// This method returns future.
 pub async fn upload_file(
     req: HttpRequest,
-    payload: actix_web::web::Payload,
+    query: web::Query<FileOpQueryParameters>,
+    payload: web::Payload,
 ) -> Result<HttpResponse, ContextualError> {
     let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
     let return_path = if let Some(header) = req.headers().get(header::REFERER) {
@@ -174,14 +182,9 @@ pub async fn upload_file(
         "/".to_string()
     };
 
-    let query_params = listing::extract_query_parameters(&req);
-    let upload_path = query_params.path.as_ref().ok_or_else(|| {
-        ContextualError::InvalidHttpRequestError("Missing query parameter 'path'".to_string())
-    })?;
-    let upload_path = sanitize_path(upload_path, conf.show_hidden).ok_or_else(|| {
+    let upload_path = sanitize_path(&query.path, conf.show_hidden).ok_or_else(|| {
         ContextualError::InvalidPathError("Invalid value for 'path' parameter".to_string())
     })?;
-
     let app_root_dir = conf.path.canonicalize().map_err(|e| {
         ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e)
     })?;
diff --git a/src/listing.rs b/src/listing.rs
index e360368..a8feeb4 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -1,6 +1,6 @@
 #![allow(clippy::format_push_string)]
 use std::io;
-use std::path::{Component, Path, PathBuf};
+use std::path::{Component, Path};
 use std::time::SystemTime;
 
 use actix_web::{dev::ServiceResponse, web::Query, HttpMessage, HttpRequest, HttpResponse};
@@ -28,10 +28,9 @@ mod percent_encode_sets {
     pub const PATH_SEGMENT: &AsciiSet = &PATH.add(b'/').add(b'\\');
 }
 
-/// Query parameters
+/// Query parameters used by listing APIs
 #[derive(Deserialize, Default)]
-pub struct QueryParameters {
-    pub path: Option<PathBuf>,
+pub struct ListingQueryParameters {
     pub sort: Option<SortingMethod>,
     pub order: Option<SortingOrder>,
     pub raw: Option<bool>,
@@ -393,13 +392,13 @@ pub fn directory_listing(
     }
 }
 
-pub fn extract_query_parameters(req: &HttpRequest) -> QueryParameters {
-    match Query::<QueryParameters>::from_query(req.query_string()) {
+pub fn extract_query_parameters(req: &HttpRequest) -> ListingQueryParameters {
+    match Query::<ListingQueryParameters>::from_query(req.query_string()) {
         Ok(Query(query_params)) => query_params,
         Err(e) => {
             let err = ContextualError::ParseError("query parameters".to_string(), e.to_string());
             errors::log_error_chain(err.to_string());
-            QueryParameters::default()
+            ListingQueryParameters::default()
         }
     }
 }
diff --git a/src/renderer.rs b/src/renderer.rs
index 1257a67..b94817d 100644
--- a/src/renderer.rs
+++ b/src/renderer.rs
@@ -15,7 +15,7 @@ use strum::{Display, IntoEnumIterator};
 
 use crate::auth::CurrentUser;
 use crate::consts;
-use crate::listing::{Breadcrumb, Entry, QueryParameters, SortingMethod, SortingOrder};
+use crate::listing::{Breadcrumb, Entry, ListingQueryParameters, SortingMethod, SortingOrder};
 use crate::{archive::ArchiveMethod, MiniserveConfig};
 
 #[allow(clippy::too_many_arguments)]
@@ -25,7 +25,7 @@ pub fn page(
     readme: Option<(String, String)>,
     abs_uri: &Uri,
     is_root: bool,
-    query_params: QueryParameters,
+    query_params: ListingQueryParameters,
     breadcrumbs: &[Breadcrumb],
     encoded_dir: &str,
     conf: &MiniserveConfig,
-- 
cgit v1.2.3


From 2cd6debc0906b2487bcaf794630514ec3016152c Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 16:32:20 +0800
Subject: Minor code style refactors

---
 src/config.rs  | 28 ++++++++++++++++------------
 src/file_op.rs | 20 +++++++++++---------
 2 files changed, 27 insertions(+), 21 deletions(-)

(limited to 'src')

diff --git a/src/config.rs b/src/config.rs
index d52b231..50b7343 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -234,6 +234,21 @@ impl MiniserveConfig {
             })
         });
 
+        let allowed_upload_dir = args
+            .allowed_upload_dir
+            .as_ref()
+            .map(|v| {
+                v.iter()
+                    .map(|p| {
+                        sanitize_path(p, false)
+                            .map(|p| p.display().to_string().replace("\\", "/"))
+                            .ok_or(anyhow!("Illegal path {p:?}: upward traversal not allowed"))
+                    })
+                    .collect()
+            })
+            .transpose()?
+            .unwrap_or_default();
+
         Ok(MiniserveConfig {
             verbose: args.verbose,
             path: args.path.unwrap_or_else(|| PathBuf::from(".")),
@@ -254,18 +269,7 @@ impl MiniserveConfig {
             show_qrcode: args.qrcode,
             mkdir_enabled: args.mkdir_enabled,
             file_upload: args.allowed_upload_dir.is_some(),
-            allowed_upload_dir: args
-                .allowed_upload_dir
-                .unwrap_or_default()
-                .iter()
-                .map(|x| {
-                    sanitize_path(x, false)
-                        .unwrap()
-                        .to_str()
-                        .unwrap()
-                        .replace('\\', "/")
-                })
-                .collect(),
+            allowed_upload_dir,
             uploadable_media_type,
             tar_enabled: args.enable_tar,
             tar_gz_enabled: args.enable_tar_gz,
diff --git a/src/file_op.rs b/src/file_op.rs
index 901c1d6..329ee8c 100644
--- a/src/file_op.rs
+++ b/src/file_op.rs
@@ -9,8 +9,10 @@ use actix_web::{http::header, web, HttpRequest, HttpResponse};
 use futures::TryStreamExt;
 use serde::Deserialize;
 
-use crate::file_utils::contains_symlink;
-use crate::{errors::ContextualError, file_utils::sanitize_path};
+use crate::{
+    config::MiniserveConfig, errors::ContextualError, file_utils::contains_symlink,
+    file_utils::sanitize_path,
+};
 
 /// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
 /// existing file.
@@ -171,17 +173,11 @@ pub struct FileOpQueryParameters {
 /// invalid.
 /// This method returns future.
 pub async fn upload_file(
+    conf: web::Data<MiniserveConfig>,
     req: HttpRequest,
     query: web::Query<FileOpQueryParameters>,
     payload: web::Payload,
 ) -> Result<HttpResponse, ContextualError> {
-    let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
-    let return_path = if let Some(header) = req.headers().get(header::REFERER) {
-        header.to_str().unwrap_or("/").to_owned()
-    } else {
-        "/".to_string()
-    };
-
     let upload_path = sanitize_path(&query.path, conf.show_hidden).ok_or_else(|| {
         ContextualError::InvalidPathError("Invalid value for 'path' parameter".to_string())
     })?;
@@ -227,6 +223,12 @@ pub async fn upload_file(
         .try_collect::<Vec<u64>>()
         .await?;
 
+    let return_path = req
+        .headers()
+        .get(header::REFERER)
+        .and_then(|h| h.to_str().ok())
+        .unwrap_or("/");
+
     Ok(HttpResponse::SeeOther()
         .append_header((header::LOCATION, return_path))
         .finish())
-- 
cgit v1.2.3


From 87f380b5526029207bde400de3beb95aa20db4f6 Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 18:01:38 +0800
Subject: Fix incorrect usage of app data extractor

- `Data` extractor can only be used when app data is wrapped with `Data`
---
 src/file_op.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/file_op.rs b/src/file_op.rs
index 329ee8c..d9786c4 100644
--- a/src/file_op.rs
+++ b/src/file_op.rs
@@ -173,11 +173,11 @@ pub struct FileOpQueryParameters {
 /// invalid.
 /// This method returns future.
 pub async fn upload_file(
-    conf: web::Data<MiniserveConfig>,
     req: HttpRequest,
     query: web::Query<FileOpQueryParameters>,
     payload: web::Payload,
 ) -> Result<HttpResponse, ContextualError> {
+    let conf = req.app_data::<MiniserveConfig>().unwrap();
     let upload_path = sanitize_path(&query.path, conf.show_hidden).ok_or_else(|| {
         ContextualError::InvalidPathError("Invalid value for 'path' parameter".to_string())
     })?;
-- 
cgit v1.2.3


From e19370505ff8b428c9ba3daeba830c5785d95a0a Mon Sep 17 00:00:00 2001
From: cyqsimon <28627918+cyqsimon@users.noreply.github.com>
Date: Tue, 5 Sep 2023 18:13:51 +0800
Subject: Fix clippy complaints

---
 src/config.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/config.rs b/src/config.rs
index 50b7343..0959e81 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -241,7 +241,7 @@ impl MiniserveConfig {
                 v.iter()
                     .map(|p| {
                         sanitize_path(p, false)
-                            .map(|p| p.display().to_string().replace("\\", "/"))
+                            .map(|p| p.display().to_string().replace('\\', "/"))
                             .ok_or(anyhow!("Illegal path {p:?}: upward traversal not allowed"))
                     })
                     .collect()
-- 
cgit v1.2.3