From 6c850263ad99c03a59214fe2d342cc8d59546ce2 Mon Sep 17 00:00:00 2001 From: Nico Fricke Date: Wed, 15 Jan 2025 09:29:02 +0100 Subject: [PATCH] move file upload to its own module to prepare for more stuff around files --- casket-backend/src/files/mod.rs | 182 +---------------------------- casket-backend/src/files/upload.rs | 181 ++++++++++++++++++++++++++++ casket-backend/src/routes.rs | 2 +- 3 files changed, 183 insertions(+), 182 deletions(-) create mode 100644 casket-backend/src/files/upload.rs diff --git a/casket-backend/src/files/mod.rs b/casket-backend/src/files/mod.rs index 088678c..709ebba 100644 --- a/casket-backend/src/files/mod.rs +++ b/casket-backend/src/files/mod.rs @@ -1,181 +1 @@ -use crate::{errors, AppState}; -use axum::body::Bytes; -use axum::debug_handler; -use axum::extract::multipart::{Field, MultipartError}; -use axum::extract::{Multipart, State}; -use axum::http::StatusCode; -use futures_util::stream::MapErr; -use futures_util::{StreamExt, TryStreamExt}; -use problem_details::ProblemDetails; -use std::fmt::Debug; -use std::io; -use std::io::Error; -use std::path::{Component, Path, PathBuf}; -use tokio::fs::{create_dir_all, File}; -use tokio::io::AsyncWriteExt; -use tokio_stream::Stream; -use tracing::error; - -#[debug_handler] -pub async fn handle_file_uploads( - State(state): State, - axum::extract::Path(user_id): axum::extract::Path, - mut multipart: Multipart, -) -> Result<(), ProblemDetails> { - while let Some(field) = multipart.next_field().await.unwrap() { - handle_single_file_upload(&state, &user_id, field).await?; - } - Ok(()) -} - -// clippy behaves weird. When removing the lifetime the compilation fails since the Field type has -// a generic lifetime around the multipart. -#[allow(clippy::needless_lifetimes)] -async fn handle_single_file_upload<'field_lifetime>( - state: &AppState, - user_id: &str, - field: Field<'field_lifetime>, -) -> Result<(), ProblemDetails> { - let path = field.name().unwrap().to_string(); - 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( - path: PathBuf, - mut content: impl Stream> + Unpin, -) -> Result<(), Error> { - create_dir_all(path.parent().unwrap()).await?; - let mut file = File::create(path).await?; - while let Some(bytes) = content.next().await { - file.write_all(&bytes?).await?; - } - Ok(()) -} - -fn to_internal_error(error: impl Debug) -> ProblemDetails { - error!("{:?}", error); - ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR) - .with_detail(errors::ERROR_DETAILS_INTERNAL_ERROR) -} - -fn map_error_to_io_error(field: Field) -> MapErr Error> { - field.map_err(|err| Error::new(io::ErrorKind::Other, err)) -} - -fn build_system_path( - base_directory: &Path, - user_id: &str, - path: &str, -) -> Result { - 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 { - 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() - ) - ) - ); - } -} +pub mod upload; diff --git a/casket-backend/src/files/upload.rs b/casket-backend/src/files/upload.rs new file mode 100644 index 0000000..724eb9d --- /dev/null +++ b/casket-backend/src/files/upload.rs @@ -0,0 +1,181 @@ +use crate::{errors, AppState}; +use axum::body::Bytes; +use axum::debug_handler; +use axum::extract::multipart::{Field, MultipartError}; +use axum::extract::{Multipart, State}; +use axum::http::StatusCode; +use futures::Stream; +use futures_util::stream::MapErr; +use futures_util::{StreamExt, TryStreamExt}; +use problem_details::ProblemDetails; +use std::fmt::Debug; +use std::io; +use std::io::Error; +use std::path::{Component, Path, PathBuf}; +use tokio::fs::{create_dir_all, File}; +use tokio::io::AsyncWriteExt; +use tracing::error; + +#[debug_handler] +pub async fn handle_file_uploads( + State(state): State, + axum::extract::Path(user_id): axum::extract::Path, + mut multipart: Multipart, +) -> Result<(), ProblemDetails> { + while let Some(field) = multipart.next_field().await.unwrap() { + handle_single_file_upload(&state, &user_id, field).await?; + } + Ok(()) +} + +// clippy behaves weird. When removing the lifetime the compilation fails since the Field type has +// a generic lifetime around the multipart. +#[allow(clippy::needless_lifetimes)] +async fn handle_single_file_upload<'field_lifetime>( + state: &AppState, + user_id: &str, + field: Field<'field_lifetime>, +) -> Result<(), ProblemDetails> { + let path = field.name().unwrap().to_string(); + 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( + path: PathBuf, + mut content: impl Stream> + Unpin, +) -> Result<(), Error> { + create_dir_all(path.parent().unwrap()).await?; + let mut file = File::create(path).await?; + while let Some(bytes) = content.next().await { + file.write_all(&bytes?).await?; + } + Ok(()) +} + +fn to_internal_error(error: impl Debug) -> ProblemDetails { + error!("{:?}", error); + ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR) + .with_detail(errors::ERROR_DETAILS_INTERNAL_ERROR) +} + +fn map_error_to_io_error(field: Field) -> MapErr Error> { + field.map_err(|err| Error::new(io::ErrorKind::Other, err)) +} + +fn build_system_path( + base_directory: &Path, + user_id: &str, + path: &str, +) -> Result { + 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 { + 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() + ) + ) + ); + } +} diff --git a/casket-backend/src/routes.rs b/casket-backend/src/routes.rs index 4f369e5..cbb9d71 100644 --- a/casket-backend/src/routes.rs +++ b/casket-backend/src/routes.rs @@ -6,7 +6,7 @@ use axum::{response::Html, routing::get, Router}; pub fn routes() -> Router { Router::new() .route("/", get(handler)) - .route("/api/v1/user/{:user_id}/files", post(files::handle_file_uploads)) + .route("/api/v1/user/{:user_id}/files", post(files::upload::handle_file_uploads)) } async fn handler() -> Html<&'static str> {