Skip to content

Commit

Permalink
♻️ Refactor upload image and add max_size (#449)
Browse files Browse the repository at this point in the history
Refactor upload image and add a configurable max size for uploaded images.
  • Loading branch information
JMicheli authored Sep 19, 2024
1 parent 6948cb6 commit 488f37e
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 47 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
cargo fmt --all -- --check
cargo clippy -- -D warnings
- name: Generate typescript bindinds
- name: Generate typescript bindings
run: cargo codegen -- --skip-prisma
- name: Verify typescript bindings
run: |
Expand Down
10 changes: 7 additions & 3 deletions apps/server/src/routers/api/v1/library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use crate::{
LibraryFilter, LibraryRelationFilter, MediaFilter, SeriesFilter,
},
middleware::auth::{auth_middleware, RequestContext},
utils::{http::ImageResponse, validate_image_upload},
utils::{http::ImageResponse, validate_and_load_image},
};

use super::{
Expand Down Expand Up @@ -107,7 +107,9 @@ pub(crate) fn mount(app_state: AppState) -> Router<AppState> {
.patch(patch_library_thumbnail)
.post(replace_library_thumbnail)
// TODO: configurable max file size
.layer(DefaultBodyLimit::max(20 * 1024 * 1024)) // 20MB
.layer(DefaultBodyLimit::max(
app_state.config.max_image_upload_size,
))
.delete(delete_library_thumbnails),
)
.route("/generate", post(generate_library_thumbnails)),
Expand Down Expand Up @@ -775,7 +777,9 @@ async fn replace_library_thumbnail(
.await?
.ok_or(APIError::NotFound(String::from("Library not found")))?;

let (content_type, bytes) = validate_image_upload(&mut upload).await?;
let (content_type, bytes) =
validate_and_load_image(&mut upload, Some(ctx.config.max_image_upload_size))
.await?;

let ext = content_type.extension();
let library_id = library.id;
Expand Down
10 changes: 7 additions & 3 deletions apps/server/src/routers/api/v1/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use crate::{
middleware::auth::{auth_middleware, RequestContext},
utils::{
http::{ImageResponse, NamedFile},
validate_image_upload,
validate_and_load_image,
},
};

Expand Down Expand Up @@ -88,7 +88,9 @@ pub(crate) fn mount(app_state: AppState) -> Router<AppState> {
.patch(patch_media_thumbnail)
.post(replace_media_thumbnail)
// TODO: configurable max file size
.layer(DefaultBodyLimit::max(20 * 1024 * 1024)), // 20MB
.layer(DefaultBodyLimit::max(
app_state.config.max_image_upload_size,
)),
)
.route("/analyze", post(start_media_analysis))
.route("/page/:page", get(get_media_page))
Expand Down Expand Up @@ -1194,7 +1196,9 @@ async fn replace_media_thumbnail(
.await?
.ok_or(APIError::NotFound(String::from("Media not found")))?;

let (content_type, bytes) = validate_image_upload(&mut upload).await?;
let (content_type, bytes) =
validate_and_load_image(&mut upload, Some(ctx.config.max_image_upload_size))
.await?;
let ext = content_type.extension();
let book_id = media.id;

Expand Down
10 changes: 7 additions & 3 deletions apps/server/src/routers/api/v1/series.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use crate::{
},
middleware::auth::{auth_middleware, RequestContext},
routers::api::v1::library::library_not_hidden_from_user_filter,
utils::{http::ImageResponse, validate_image_upload},
utils::{http::ImageResponse, validate_and_load_image},
};

use super::{
Expand Down Expand Up @@ -88,7 +88,9 @@ pub(crate) fn mount(app_state: AppState) -> Router<AppState> {
.patch(patch_series_thumbnail)
.post(replace_series_thumbnail)
// TODO: configurable max file size
.layer(DefaultBodyLimit::max(20 * 1024 * 1024)), // 20MB
.layer(DefaultBodyLimit::max(
app_state.config.max_image_upload_size,
)),
)
.route(
"/complete",
Expand Down Expand Up @@ -759,7 +761,9 @@ async fn replace_series_thumbnail(
.await?
.ok_or(APIError::NotFound(String::from("Series not found")))?;

let (content_type, bytes) = validate_image_upload(&mut upload).await?;
let (content_type, bytes) =
validate_and_load_image(&mut upload, Some(ctx.config.max_image_upload_size))
.await?;
let ext = content_type.extension();
let series_id = series.id;

Expand Down
12 changes: 7 additions & 5 deletions apps/server/src/routers/api/v1/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
errors::{APIError, APIResult},
filter::{chain_optional_iter, UserQueryRelation},
middleware::auth::{auth_middleware, RequestContext},
utils::{get_session_user, http::ImageResponse, validate_image_upload},
utils::{get_session_user, http::ImageResponse, validate_and_load_image},
};

pub(crate) fn mount(app_state: AppState) -> Router<AppState> {
Expand Down Expand Up @@ -73,9 +73,9 @@ pub(crate) fn mount(app_state: AppState) -> Router<AppState> {
)
.route(
"/avatar",
get(get_user_avatar)
.post(upload_user_avatar)
.layer(DefaultBodyLimit::max(20 * 1024 * 1024)), // 20MB
get(get_user_avatar).post(upload_user_avatar).layer(
DefaultBodyLimit::max(app_state.config.max_image_upload_size),
),
),
)
.layer(middleware::from_fn_with_state(app_state, auth_middleware))
Expand Down Expand Up @@ -1160,7 +1160,9 @@ async fn upload_user_avatar(
.await?
.ok_or(APIError::NotFound("User not found".to_string()))?;

let (content_type, bytes) = validate_image_upload(&mut upload).await?;
let (content_type, bytes) =
validate_and_load_image(&mut upload, Some(ctx.config.max_image_upload_size))
.await?;

let ext = content_type.extension();
let username = user.username.clone();
Expand Down
164 changes: 133 additions & 31 deletions apps/server/src/utils/upload.rs
Original file line number Diff line number Diff line change
@@ -1,59 +1,161 @@
use axum::extract::Multipart;
use axum::extract::{multipart::Field, Multipart};
use futures_util::Stream;
use stump_core::filesystem::ContentType;

use crate::errors::{APIError, APIResult};

// TODO: it would be a great enhancement to allow hookup of a malware scanner here, e.g. clamav
// TODO: Allow configuration of maximum file size
/// A helper function to validate an image upload. This function will return the content type of the
/// uploaded image if it is valid.
pub async fn validate_image_upload(
/// A helper function to validate and stream the bytes of an image upload from multipart form data, represented
/// by [Multipart]. This function will return the content type of the uploaded image if it is valid.
pub async fn validate_and_load_image(
upload: &mut Multipart,
max_size: Option<usize>,
) -> APIResult<(ContentType, Vec<u8>)> {
validate_and_load_upload(
upload,
max_size,
|content_type| content_type.is_image(),
Some("image"),
)
.await
}

/// An internal helper function to validate and load a generic upload.
/// The validator will load an image from multipart form data ([Multipart]) and check that it
/// is not larger than the max_size. Then, it will check that the `ContentType` information from
/// the header and/or magic numbers in the first 5 bytes of the file match the expected type using
/// the provided `is_valid_content_type` function.
///
/// Optionally, an `expected_type_name` can be provided so that the error message can specify what
/// type was expected.
async fn validate_and_load_upload(
upload: &mut Multipart,
max_size: Option<usize>,
is_valid_content_type: impl Fn(&ContentType) -> bool,
expected_type_name: Option<&str>,
) -> APIResult<(ContentType, Vec<u8>)> {
let field = upload.next_field().await?.ok_or_else(|| {
APIError::BadRequest(String::from("No file provided in multipart"))
})?;

let raw_content_type = field.content_type().map(ContentType::from);

let bytes = field.bytes().await?;
let file_size = bytes.len();

if bytes.is_empty() || bytes.len() < 5 {
return Err(APIError::BadRequest("Uploaded file is empty".to_string()));
}

let mut magic_bytes = vec![0; 5];
magic_bytes.copy_from_slice(&bytes[..5]);
// Load bytes of uploaded file
let (bytes, file_size) = load_field_up_to_size(field, max_size).await?;

let inferred_content_type = ContentType::from_bytes(&magic_bytes);
// Use first 5 bytes to infer content type.
// See: https://en.wikipedia.org/wiki/List_of_file_signatures
let magic_bytes = &bytes[..5];
let inferred_content_type = ContentType::from_bytes(magic_bytes);

let content_type = match (raw_content_type, inferred_content_type) {
(Some(provided), inferred) if !provided.is_image() && !inferred.is_image() => {
Err(APIError::BadRequest(
"Uploaded file is not an image".to_string(),
))
(Some(provided), inferred)
if !is_valid_content_type(&provided) && !is_valid_content_type(&inferred) =>
{
Err(validation_err(expected_type_name))
},
(Some(provided), inferred) => {
if provided != inferred {
tracing::warn!(?provided, ?inferred, "Content type mismatch");
}
let content_type = (inferred.is_image())
.then_some(inferred)
.or_else(|| provided.is_image().then_some(provided))
.ok_or_else(|| {
APIError::BadRequest("Uploaded file is not an image".to_string())
})?;
let content_type = if is_valid_content_type(&inferred) {
inferred
} else if is_valid_content_type(&provided) {
provided
} else {
return Err(validation_err(expected_type_name));
};
Ok(content_type)
},
(None, inferred) => (inferred.is_image()).then_some(inferred).ok_or_else(|| {
APIError::BadRequest("Uploaded file is not an image".to_string())
}),
(None, inferred) => {
if is_valid_content_type(&inferred) {
Ok(inferred)
} else {
Err(validation_err(expected_type_name))
}
},
}?;

tracing::trace!(?content_type, file_size, "Validated image upload");

tracing::trace!(?content_type, file_size, "Validated upload");
Ok((content_type, bytes.to_vec()))
}

/// Load up to `max_size` bytes of a field (erroring if `max_size` is exceeded).
/// Returns the loaded bytes as [Vec<u8>] and the total bytes as [usize].
async fn load_field_up_to_size(
mut field: Field<'_>,
max_size: Option<usize>,
) -> APIResult<(Vec<u8>, usize)> {
let name = field.file_name().unwrap_or("<no filename>").to_string();

// Check size hint if one was provided
if let (Some(max_size), (_, Some(size_hint))) = (max_size, field.size_hint()) {
if size_hint > max_size {
return Err(max_size_err(max_size, &name, size_hint));
}
}

// Load field in chunks
let mut bytes = Vec::new();
let mut total_size = 0;
while let Some(chunk) = field.chunk().await? {
// Increase chunk size and check against max size (if any)
total_size += chunk.len();
if let Some(max_size) = max_size {
if total_size > max_size {
return Err(max_size_err(max_size, &name, total_size));
}
}

bytes.extend_from_slice(&chunk);
}

if bytes.is_empty() || total_size < 5 {
return Err(APIError::BadRequest("Uploaded file is empty".to_string()));
}

Ok((bytes, total_size))
}

/// Formats the validation errors used elsewhere in this module when a type doesn't
/// match the expected type (optionally specified as `expected_type_name`).
fn validation_err(expected_type_name: Option<&str>) -> APIError {
if let Some(type_name) = expected_type_name {
if !type_name.is_empty() {
return APIError::BadRequest(format!(
"Uploaded file was expected to be {}",
type_name
));
}
}

APIError::BadRequest("Uploaded file does not match expected type".to_string())
}

/// Formats the max size errors used elsewhere in this module when `max_size` is exceeded
/// by a thing named `name` with a value that is `actual_size`.
fn max_size_err(max_size: usize, name: &str, actual_size: usize) -> APIError {
APIError::BadRequest(format!(
"Max size of {} bytes exceeded by {} which is {} bytes",
max_size, name, actual_size
))
}

// TODO: validate_media_upload (books)

#[cfg(test)]
mod tests {
use super::*;

#[tokio::test]
async fn test_max_size_err() {
max_size_err(500, "file_name", 512);
}

#[tokio::test]
async fn test_validation_err() {
validation_err(Some("image"));
validation_err(Some("pdf"));
validation_err(Some(""));
validation_err(None);
}
}
13 changes: 12 additions & 1 deletion core/src/config/stump_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub mod env_keys {
pub const SESSION_EXPIRY_INTERVAL_KEY: &str = "SESSION_EXPIRY_CLEANUP_INTERVAL";
pub const MAX_SCANNER_CONCURRENCY_KEY: &str = "STUMP_MAX_SCANNER_CONCURRENCY";
pub const MAX_THUMBNAIL_CONCURRENCY_KEY: &str = "STUMP_MAX_THUMBNAIL_CONCURRENCY";
pub const MAX_IMAGE_UPLOAD_SIZE_KEY: &str = "STUMP_MAX_IMAGE_UPLOAD_SIZE";
}
use env_keys::*;

Expand All @@ -41,10 +42,11 @@ pub mod defaults {
pub const DEFAULT_SESSION_EXPIRY_CLEANUP_INTERVAL: u64 = 60 * 60 * 24; // 24 hours
pub const DEFAULT_MAX_SCANNER_CONCURRENCY: usize = 200;
pub const DEFAULT_MAX_THUMBNAIL_CONCURRENCY: usize = 50;
pub const DEFAULT_MAX_IMAGE_UPLOAD_SIZE: usize = 20 * 1024 * 1024; // 20 MB
}
use defaults::*;

/// Represents the configuration of a Stump application. This file is generated at startup
/// Represents the configuration of a Stump application. This struct is generated at startup
/// using a TOML file, environment variables, or both and is input when creating a `StumpCore`
/// instance.
///
Expand Down Expand Up @@ -164,6 +166,12 @@ pub struct StumpConfig {
#[default_value(DEFAULT_MAX_THUMBNAIL_CONCURRENCY)]
#[env_key(MAX_THUMBNAIL_CONCURRENCY_KEY)]
pub max_thumbnail_concurrency: usize,

/// The maxium file size, in bytes, of images that can be uploaded, e.g., as thumbnails for users,
/// libraries, series, or media.
#[default_value(DEFAULT_MAX_IMAGE_UPLOAD_SIZE)]
#[env_key(MAX_IMAGE_UPLOAD_SIZE_KEY)]
pub max_image_upload_size: usize,
}

impl StumpConfig {
Expand Down Expand Up @@ -307,6 +315,7 @@ mod tests {
expired_session_cleanup_interval: None,
max_scanner_concurrency: None,
max_thumbnail_concurrency: None,
max_image_upload_size: None,
};
partial_config.apply_to_config(&mut config);

Expand Down Expand Up @@ -342,6 +351,7 @@ mod tests {
),
max_scanner_concurrency: Some(DEFAULT_MAX_SCANNER_CONCURRENCY),
max_thumbnail_concurrency: Some(DEFAULT_MAX_THUMBNAIL_CONCURRENCY),
max_image_upload_size: Some(DEFAULT_MAX_IMAGE_UPLOAD_SIZE)
}
);

Expand Down Expand Up @@ -392,6 +402,7 @@ mod tests {
custom_templates_dir: None,
max_scanner_concurrency: DEFAULT_MAX_SCANNER_CONCURRENCY,
max_thumbnail_concurrency: DEFAULT_MAX_THUMBNAIL_CONCURRENCY,
max_image_upload_size: DEFAULT_MAX_IMAGE_UPLOAD_SIZE,
}
);
},
Expand Down

0 comments on commit 488f37e

Please sign in to comment.