fix: stream cover uploads with a per-chunk size cap
The cover multipart path called Field::bytes(), buffering the entire field into memory before parse_image checked max_file_bytes — an oversized cover was fully allocated before rejection. Add read_capped/push_capped, which reject with 413 as soon as the running total exceeds the cap (the offending chunk is never copied in), and route both cover handlers and stage_image_part through it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.124.15"
|
||||
version = "0.124.16"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.124.15"
|
||||
version = "0.124.16"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -239,7 +239,8 @@ async fn create(
|
||||
metadata = Some(parse_metadata_json(&bytes)?);
|
||||
}
|
||||
Some("cover") => {
|
||||
let bytes = read_field_bytes(field).await?.to_vec();
|
||||
let bytes =
|
||||
crate::upload::read_capped(field, state.upload.max_file_bytes, "cover").await?;
|
||||
cover = Some(parse_image(bytes, state.upload.max_file_bytes, "cover")?);
|
||||
}
|
||||
_ => continue,
|
||||
@@ -387,7 +388,8 @@ async fn put_cover(
|
||||
let mut cover: Option<UploadedImage> = None;
|
||||
while let Some(field) = next_field(&mut multipart).await? {
|
||||
if field.name() == Some("cover") {
|
||||
let bytes = read_field_bytes(field).await?.to_vec();
|
||||
let bytes =
|
||||
crate::upload::read_capped(field, state.upload.max_file_bytes, "cover").await?;
|
||||
cover = Some(parse_image(bytes, state.upload.max_file_bytes, "cover")?);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,21 +45,13 @@ pub const STAGING_PREFIX: &str = "staging";
|
||||
/// read-all-then-persist path.
|
||||
pub async fn stage_image_part(
|
||||
storage: &dyn Storage,
|
||||
mut field: Field<'_>,
|
||||
field: Field<'_>,
|
||||
upload_id: Uuid,
|
||||
seq: usize,
|
||||
max_size: usize,
|
||||
field_name: &str,
|
||||
) -> AppResult<StagedImage> {
|
||||
let mut bytes: Vec<u8> = Vec::new();
|
||||
while let Some(chunk) = field.chunk().await.map_err(map_multipart_error)? {
|
||||
if bytes.len().saturating_add(chunk.len()) > max_size {
|
||||
return Err(AppError::PayloadTooLarge(format!(
|
||||
"{field_name} exceeds {max_size}-byte cap"
|
||||
)));
|
||||
}
|
||||
bytes.extend_from_slice(&chunk);
|
||||
}
|
||||
let bytes = read_capped(field, max_size, field_name).await?;
|
||||
// Reuse the shared sniff + whitelist check; it re-verifies the size cap
|
||||
// (already enforced above) and derives mime/ext from magic bytes.
|
||||
let img = parse_image(bytes, max_size, field_name)?;
|
||||
@@ -73,6 +65,46 @@ pub async fn stage_image_part(
|
||||
})
|
||||
}
|
||||
|
||||
/// Read one multipart part fully into memory, enforcing the per-file byte cap
|
||||
/// as chunks arrive so an oversized part is rejected (413) **without being fully
|
||||
/// buffered**. Use for parts whose bytes the caller needs in hand — e.g. the
|
||||
/// cover image, which is written to a manga-scoped key. Page parts should prefer
|
||||
/// [`stage_image_part`], which streams straight to storage.
|
||||
///
|
||||
/// Replaces the `Field::bytes()` path, which buffered the entire field before
|
||||
/// any size check ran, letting an attacker allocate an arbitrarily large body
|
||||
/// before the cap kicked in.
|
||||
pub async fn read_capped(
|
||||
mut field: Field<'_>,
|
||||
max_size: usize,
|
||||
field_name: &str,
|
||||
) -> AppResult<Vec<u8>> {
|
||||
let mut bytes: Vec<u8> = Vec::new();
|
||||
while let Some(chunk) = field.chunk().await.map_err(map_multipart_error)? {
|
||||
push_capped(&mut bytes, &chunk, max_size, field_name)?;
|
||||
}
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
/// Append `chunk` to `buf`, rejecting with 413 as soon as the running total
|
||||
/// would exceed `max_size` — so the oversized chunk is never copied in. Split
|
||||
/// out from the read loop so the cap logic is unit-testable without a live
|
||||
/// multipart field.
|
||||
fn push_capped(
|
||||
buf: &mut Vec<u8>,
|
||||
chunk: &[u8],
|
||||
max_size: usize,
|
||||
field_name: &str,
|
||||
) -> AppResult<()> {
|
||||
if buf.len().saturating_add(chunk.len()) > max_size {
|
||||
return Err(AppError::PayloadTooLarge(format!(
|
||||
"{field_name} exceeds {max_size}-byte cap"
|
||||
)));
|
||||
}
|
||||
buf.extend_from_slice(chunk);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn parse_image(bytes: Vec<u8>, max_size: usize, field_name: &str) -> AppResult<UploadedImage> {
|
||||
if bytes.len() > max_size {
|
||||
return Err(AppError::PayloadTooLarge(format!(
|
||||
@@ -172,4 +204,30 @@ mod tests {
|
||||
assert!(matches!(err, AppError::PayloadTooLarge(_)));
|
||||
assert_eq!(err.code(), "payload_too_large");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn push_capped_accumulates_under_cap() {
|
||||
let mut buf = Vec::new();
|
||||
push_capped(&mut buf, b"hello ", 100, "cover").unwrap();
|
||||
push_capped(&mut buf, b"world", 100, "cover").unwrap();
|
||||
assert_eq!(buf, b"hello world");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn push_capped_rejects_before_copying_oversized_chunk() {
|
||||
// The whole point: the offending chunk must NOT be appended — an
|
||||
// oversized part is rejected without buffering it.
|
||||
let mut buf = vec![0u8; 90];
|
||||
let err = push_capped(&mut buf, &[0u8; 20], 100, "cover").unwrap_err();
|
||||
assert!(matches!(err, AppError::PayloadTooLarge(_)));
|
||||
assert_eq!(err.code(), "payload_too_large");
|
||||
assert_eq!(buf.len(), 90, "buffer must not grow past the cap");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn push_capped_allows_exactly_at_cap() {
|
||||
let mut buf = vec![0u8; 90];
|
||||
push_capped(&mut buf, &[0u8; 10], 100, "cover").unwrap();
|
||||
assert_eq!(buf.len(), 100);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.124.15",
|
||||
"version": "0.124.16",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
Reference in New Issue
Block a user