add some proper error handling for some edge cases instead of using unwrap and move some common utils to the files mod.rs

casket-backend/src/errors.rs

@@ -2,3 +2,5 @@ pub const ERROR_DETAILS_PATH_TRAVERSAL: &str = "You must not traverse!";
 pub const ERROR_DETAILS_INTERNAL_ERROR: &str =
     "Internal Error! Check the logs of the backend service for details.";
 pub const ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED: &str = "Absolute paths are not allowed.";
+pub const ERROR_DETAILS_NO_NAME_PROVIDED_MULTIPART_FIELD: &str =
+    "No name provided for multipart form field";

casket-backend/src/files/mod.rs

@@ -1 +1,121 @@
+use crate::errors;
+use axum::http::StatusCode;
+use problem_details::ProblemDetails;
+use std::path::{Component, Path, PathBuf};
+
+pub mod list;
 pub mod upload;
+
+fn build_system_path(
+    base_directory: &Path,
+    user_id: &str,
+    user_path: &PathBuf,
+) -> Result<PathBuf, ProblemDetails> {
+    if user_path.is_absolute() {
+        Err(
+            ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail(
+                errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned()
+                    + format!(" Provided Path: {:?}", user_path).as_str(),
+            ),
+        )
+    } else {
+        sanitize_path(&base_directory.join(user_id).join(user_path))
+    }
+}
+
+pub fn sanitize_path(path: &Path) -> Result<PathBuf, ProblemDetails> {
+    let mut ret = PathBuf::new();
+    for component in path.components().peekable() {
+        match component {
+            Component::Prefix(..) => unreachable!(),
+            Component::RootDir => {
+                ret.push(Component::RootDir);
+            }
+            Component::CurDir => {}
+            Component::ParentDir => {
+                return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
+                    .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL));
+            }
+            Component::Normal(c) => {
+                ret.push(c);
+            }
+        }
+    }
+    Ok(ret)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_sanitize_path_success() {
+        let cases = &[
+            ("", ""),
+            (".", ""),
+            (".////./.", ""),
+            ("/", "/"),
+            ("/foo/bar", "/foo/bar"),
+            ("/foo/bar/", "/foo/bar"),
+            ("/foo/bar/./././///", "/foo/bar"),
+            ("foo/bar", "foo/bar"),
+            ("foo/bar/", "foo/bar"),
+            ("foo/bar/./././///", "foo/bar"),
+        ];
+        for (input, expected) in cases {
+            let actual = sanitize_path(&PathBuf::from(input));
+            assert_eq!(actual, Ok(PathBuf::from(expected)), "input: {input}");
+        }
+    }
+
+    #[test]
+    fn test_sanitize_path_error() {
+        let cases = &[
+            "..",
+            "/../",
+            "../../foo/bar",
+            "../../foo/bar/",
+            "../../foo/bar/./././///",
+            "../../foo/bar/..",
+            "../../foo/bar/../..",
+            "../../foo/bar/../../..",
+            "/foo/bar/../../..",
+            "/foo/bar/../..",
+            "/foo/bar/..",
+        ];
+        for input in cases {
+            let actual = sanitize_path(&PathBuf::from(input));
+            assert_eq!(
+                actual,
+                Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
+                    .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)),
+                "input: {input}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_build_system_path_success() {
+        let input_path = PathBuf::from("tmp/blub");
+        let user_id = "bla";
+        assert_eq!(
+            build_system_path(&PathBuf::from("/tmp/bla"), user_id, &input_path).unwrap(),
+            PathBuf::from("/tmp/bla/bla/tmp/blub")
+        );
+    }
+
+    #[test]
+    fn test_build_system_path_error_with_absolute_user_path_returns_error() {
+        let input_path = PathBuf::from("tmp/blub");
+        let user_id = "bla";
+        assert_eq!(
+            build_system_path(&PathBuf::from("/tmp/bla"), user_id, &input_path),
+            Err(
+                ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail(
+                    errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned()
+                        + format!(" Provided Path: {:?}", input_path).as_str()
+                )
+            )
+        );
+    }
+}

casket-backend/src/files/upload.rs

@@ -1,3 +1,4 @@
+use crate::files::build_system_path;
 use crate::{errors, AppState};
 use axum::body::Bytes;
 use axum::debug_handler;
@@ -22,10 +23,14 @@ pub async fn handle_file_uploads(
     axum::extract::Path(user_id): axum::extract::Path<String>,
     mut multipart: Multipart,
 ) -> Result<(), ProblemDetails> {
-    while let Some(field) = multipart.next_field().await.unwrap() {
+    loop {
-        handle_single_file_upload(&state, &user_id, field).await?;
+        let next = multipart.next_field().await;
+        match next {
+            Err(error) => return Err(to_internal_error(error)),
+            Ok(Some(field)) => handle_single_file_upload(&state, &user_id, field).await?,
+            Ok(None) => return Ok(()),
+        }
     }
-    Ok(())
 }
 
 // clippy behaves weird. When removing the lifetime the compilation fails since the Field type has
@@ -36,11 +41,17 @@ async fn handle_single_file_upload<'field_lifetime>(
     user_id: &str,
     field: Field<'field_lifetime>,
 ) -> Result<(), ProblemDetails> {
-    let path = field.name().unwrap().to_string();
+    match field.name() {
-    let filesystem_path = build_system_path(&state.config.files.directory, user_id, &path)?;
+        None | Some("") => Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
-    save_file(filesystem_path, map_error_to_io_error(field))
+            .with_detail(errors::ERROR_DETAILS_NO_NAME_PROVIDED_MULTIPART_FIELD)),
-        .await
+        Some(field_name) => {
-        .map_err(to_internal_error)
+            let path = PathBuf::from(field_name);
+            let filesystem_path = build_system_path(&state.config.files.directory, user_id, &path)?;
+            save_file(filesystem_path, map_error_to_io_error(field))
+                .await
+                .map_err(to_internal_error)
+        }
+    }
 }
 
 pub async fn save_file(
@@ -64,118 +75,3 @@ fn to_internal_error(error: impl Debug) -> ProblemDetails {
 fn map_error_to_io_error(field: Field) -> MapErr<Field, fn(MultipartError) -> Error> {
     field.map_err(|err| Error::new(io::ErrorKind::Other, err))
 }
-
-fn build_system_path(
-    base_directory: &Path,
-    user_id: &str,
-    path: &str,
-) -> Result<PathBuf, ProblemDetails> {
-    let user_path = PathBuf::from(path);
-    if user_path.is_absolute() {
-        Err(
-            ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail(
-                errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned()
-                    + format!(" Provided Path: {path}").as_str(),
-            ),
-        )
-    } else {
-        sanitize_path(&base_directory.join(user_id).join(path))
-    }
-}
-
-pub fn sanitize_path(path: &Path) -> Result<PathBuf, ProblemDetails> {
-    let mut ret = PathBuf::new();
-    for component in path.components().peekable() {
-        match component {
-            Component::Prefix(..) => unreachable!(),
-            Component::RootDir => {
-                ret.push(Component::RootDir);
-            }
-            Component::CurDir => {}
-            Component::ParentDir => {
-                return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
-                    .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL));
-            }
-            Component::Normal(c) => {
-                ret.push(c);
-            }
-        }
-    }
-    Ok(ret)
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_sanitize_path_success() {
-        let cases = &[
-            ("", ""),
-            (".", ""),
-            (".////./.", ""),
-            ("/", "/"),
-            ("/foo/bar", "/foo/bar"),
-            ("/foo/bar/", "/foo/bar"),
-            ("/foo/bar/./././///", "/foo/bar"),
-            ("foo/bar", "foo/bar"),
-            ("foo/bar/", "foo/bar"),
-            ("foo/bar/./././///", "foo/bar"),
-        ];
-        for (input, expected) in cases {
-            let actual = sanitize_path(&PathBuf::from(input));
-            assert_eq!(actual, Ok(PathBuf::from(expected)), "input: {input}");
-        }
-    }
-
-    #[test]
-    fn test_sanitize_path_error() {
-        let cases = &[
-            "..",
-            "/../",
-            "../../foo/bar",
-            "../../foo/bar/",
-            "../../foo/bar/./././///",
-            "../../foo/bar/..",
-            "../../foo/bar/../..",
-            "../../foo/bar/../../..",
-            "/foo/bar/../../..",
-            "/foo/bar/../..",
-            "/foo/bar/..",
-        ];
-        for input in cases {
-            let actual = sanitize_path(&PathBuf::from(input));
-            assert_eq!(
-                actual,
-                Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
-                    .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)),
-                "input: {input}"
-            );
-        }
-    }
-
-    #[test]
-    fn test_build_system_path_success() {
-        let input_path = "tmp/blub";
-        let user_id = "bla";
-        assert_eq!(
-            build_system_path(&PathBuf::from("/tmp/bla"), user_id, input_path).unwrap(),
-            PathBuf::from("/tmp/bla/bla/tmp/blub")
-        );
-    }
-
-    #[test]
-    fn test_build_system_path_error_with_absolute_user_path_returns_error() {
-        let input_path = "tmp/blub";
-        let user_id = "bla";
-        assert_eq!(
-            build_system_path(&PathBuf::from("/tmp/bla"), user_id, input_path),
-            Err(
-                ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail(
-                    errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned()
-                        + format!(" Provided Path: {input_path}").as_str()
-                )
-            )
-        );
-    }
-}