fix(upload): a raised size limit destroyed videos instead of refusing them
`MAX_UPLOAD_BYTES` (576 MiB) is the router's `DefaultBodyLimit` on the upload
route. Its coupling to the admin-tunable `max_image_size_mb` /
`max_video_size_mb` was enforced by a COMMENT — "if an admin raises
max_video_size_mb above this, bump MAX_UPLOAD_BYTES" — while `patch_config`
accepted 1024 and 10240 respectively and the dashboard rendered
"Max. Videogröße (MB)" as a bare number field with no stated ceiling.
Set it to 1000 and every video between 576 MB and the new limit is not refused,
it is DESTROYED, and the shape is worse than the size:
* The body limit trips MID-UPLOAD, inside `field.chunk()`, so
`stream_field_to_file` maps it to `AppError::BadRequest` — a 400, not a 413
carrying the `quota_exceeded` code the client knows how to keep a blob for.
* `classifyUploadStatus` puts every non-401/408/429 4xx in the `terminal`
bucket, and `isReversibleLock(400, 'bad_request')` is false — so the queue
DELETES the blob from IndexedDB and moves the row to `blocked`, which by
design offers no retry button.
* All of that after the guest pushed 600 MB over cellular, and the message
they get names a read failure rather than a limit.
"Raise the video limit" is exactly the change a host makes after a guest
complains a clip was too big, so this is reachable by an operator doing the
obvious thing.
The ceiling is now DERIVED from the body limit rather than written down twice
(`MAX_CONFIGURABLE_UPLOAD_MB`, 575) and enforced at both ends, because either
alone leaves a hole: `patch_config` bounds what can be WRITTEN, and the upload
handler clamps what it READS, since a row stored before this bound existed — or
edited straight into the `config` table — would sail past the first check.
A compile-time assertion pins both directions against the ACTUAL field caps
(`MAX_CAPTION_BYTES + MAX_HASHTAGS_BYTES + MAX_CLIENT_UPLOAD_ID_BYTES` plus
framing), so raising `MAX_CAPTION_LENGTH` fails the build rather than silently
eating the envelope margin; a lower bound keeps the ceiling clear of the 500 MB
`max_video_size_mb` seeded by migration 005.
Ordering is now guaranteed: at 575 MiB the handler's own cap trips while the
body is ~1 MiB short of axum's, so the clean "Datei ist zu groß" 400 always wins
the race against the mid-stream abort.
Frontend, both halves of the same rule:
* the composer's pre-flight moves from 576 MiB (the raw body limit) to 575 MB,
so a guest is rejected locally against the same number the server enforces
and never pushes the file to find out;
* both size fields gain a hint naming the 575 ceiling, so the operator learns
the bound from the form instead of from an error after typing 1000.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
@@ -115,9 +115,15 @@ pub async fn patch_config(
|
||||
// accept but that silently revert to the hardcoded default at read time
|
||||
// (get_usize/get_i64 can't parse negatives/NaN/fractionals). `compression_concurrency`
|
||||
// is intentionally absent — it's read once at boot, so a live edit was a no-op.
|
||||
// The two size limits are bounded by what the router's `DefaultBodyLimit` can carry, NOT by
|
||||
// a round number. They used to allow 1024 and 10240 MB against a 576 MiB body cap, so the
|
||||
// admin dashboard's plain "Max. Videogröße (MB)" field could be set to a value that destroys
|
||||
// every video above the cap — the limit trips mid-body, surfaces as a 400, and the client
|
||||
// purges the blob as terminal. See `crate::MAX_CONFIGURABLE_UPLOAD_MB` for the full chain.
|
||||
const MAX_SIZE_MB: f64 = crate::MAX_CONFIGURABLE_UPLOAD_MB as f64;
|
||||
const NUMERIC_SPECS: &[(&str, bool, f64, f64)] = &[
|
||||
("max_image_size_mb", true, 1.0, 1024.0),
|
||||
("max_video_size_mb", true, 1.0, 10240.0),
|
||||
("max_image_size_mb", true, 1.0, MAX_SIZE_MB),
|
||||
("max_video_size_mb", true, 1.0, MAX_SIZE_MB),
|
||||
("upload_rate_per_hour", true, 1.0, 100_000.0),
|
||||
("feed_rate_per_min", true, 1.0, 100_000.0),
|
||||
("export_rate_per_day", true, 1.0, 100_000.0),
|
||||
|
||||
@@ -25,10 +25,15 @@ const MAX_CAPTION_LENGTH: usize = 2000;
|
||||
/// `MAX_CAPTION_LENGTH` check only ran afterwards, on a string that had already been built.
|
||||
/// 4 bytes per code point is the worst case for UTF-8, so this can never reject a caption the
|
||||
/// character limit would have accepted.
|
||||
const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4;
|
||||
pub(crate) const MAX_CAPTION_BYTES: usize = MAX_CAPTION_LENGTH * 4;
|
||||
|
||||
/// Byte ceiling for the raw hashtag CSV. Generous next to what the tag caps below allow.
|
||||
const MAX_HASHTAGS_BYTES: usize = 4 * 1024;
|
||||
pub(crate) const MAX_HASHTAGS_BYTES: usize = 4 * 1024;
|
||||
|
||||
/// Byte ceiling for the `client_upload_id` field. 64 bytes fits a hyphenated UUID (36) with
|
||||
/// room to spare; named rather than inline so the multipart-envelope test in `main.rs` can
|
||||
/// account for every text field this handler will read.
|
||||
pub(crate) const MAX_CLIENT_UPLOAD_ID_BYTES: usize = 64;
|
||||
|
||||
/// Hashtags stored per upload. The CSV was never length-checked at all and was split into an
|
||||
/// unbounded `Vec`, then upserted TAG BY TAG inside the commit transaction — which holds a
|
||||
@@ -280,9 +285,24 @@ pub async fn upload(
|
||||
return Err(AppError::UploadsLocked("Uploads sind gesperrt.".into()));
|
||||
}
|
||||
|
||||
// Read config limits from DB
|
||||
let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20).await;
|
||||
let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500).await;
|
||||
// Read config limits from DB.
|
||||
//
|
||||
// CLAMPED to what the router's `DefaultBodyLimit` can actually carry. `patch_config` now
|
||||
// refuses a larger value, but that guard only covers values written THROUGH it: a config row
|
||||
// stored before the bound existed, or edited straight into the table, would otherwise sail
|
||||
// past it and hand the guest the worst failure in the app — the body limit tripping mid-upload,
|
||||
// surfacing as a 400, and the client purging the blob as terminal. See
|
||||
// `crate::MAX_CONFIGURABLE_UPLOAD_MB`.
|
||||
//
|
||||
// Clamping (rather than refusing the upload) is right here: the operator's intent was "allow
|
||||
// bigger files", and the honest answer to an unsatisfiable limit is the largest one that
|
||||
// works, applied consistently by both the streaming cap and the per-class check below.
|
||||
let max_image_mb: i64 = config::get_i64(&state.config_cache, "max_image_size_mb", 20)
|
||||
.await
|
||||
.min(crate::MAX_CONFIGURABLE_UPLOAD_MB);
|
||||
let max_video_mb: i64 = config::get_i64(&state.config_cache, "max_video_size_mb", 500)
|
||||
.await
|
||||
.min(crate::MAX_CONFIGURABLE_UPLOAD_MB);
|
||||
|
||||
// The uploaded file is streamed straight to a temp file on disk (never buffered
|
||||
// whole in memory — a 500 MB video used to cost 500 MB of RAM per concurrent
|
||||
@@ -379,8 +399,8 @@ pub async fn upload(
|
||||
// `hashtags` were bounded by the helper for exactly this reason; this field
|
||||
// arrived later (migration 022) and missed it.
|
||||
//
|
||||
// 64 bytes fits a hyphenated UUID (36) with room to spare.
|
||||
let raw = read_text_field_bounded(field, 64).await?;
|
||||
// See `MAX_CLIENT_UPLOAD_ID_BYTES`.
|
||||
let raw = read_text_field_bounded(field, MAX_CLIENT_UPLOAD_ID_BYTES).await?;
|
||||
// A malformed key is not worth rejecting an upload over — the photo is the
|
||||
// thing the guest cares about. Drop the key and lose only the retry
|
||||
// protection, which is exactly where we were before it existed.
|
||||
|
||||
@@ -19,7 +19,36 @@ use state::AppState;
|
||||
|
||||
/// Hard HTTP body cap for the upload endpoint (576 MiB). Backstop against
|
||||
/// memory-exhaustion; precise per-class size limits are enforced in the handler.
|
||||
const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024;
|
||||
pub(crate) const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024;
|
||||
|
||||
/// Largest per-file size limit (MB) an operator may configure for `max_image_size_mb` /
|
||||
/// `max_video_size_mb`, derived from [`MAX_UPLOAD_BYTES`] rather than written down twice.
|
||||
///
|
||||
/// This used to be enforced by a COMMENT — "if an admin raises max_video_size_mb above this,
|
||||
/// bump MAX_UPLOAD_BYTES" — while `patch_config` happily accepted 10240 and the admin dashboard
|
||||
/// rendered "Max. Videogröße (MB)" as a bare number field. Set it to 1000 and every video
|
||||
/// between 576 MB and the new limit is destroyed, in a shape that is much worse than a refusal:
|
||||
///
|
||||
/// * The body limit trips mid-upload, inside `field.chunk()`, so `stream_field_to_file` maps
|
||||
/// it to `AppError::BadRequest` — a 400, not a 413 with a `quota_exceeded` code.
|
||||
/// * `classifyUploadStatus` (frontend/src/lib/upload-queue.ts) puts every non-401/408/429 4xx
|
||||
/// in the `terminal` bucket, and `isReversibleLock(400, 'bad_request')` is false — so the
|
||||
/// queue DELETES the blob from IndexedDB and moves the row to `blocked`, which by design
|
||||
/// offers no retry button.
|
||||
/// * All of that after the guest has already pushed 600 MB over cellular, and the message they
|
||||
/// get names a read failure rather than a limit.
|
||||
///
|
||||
/// So the ceiling is enforced where the value is SET (`patch_config`) and again where it is READ
|
||||
/// (`handlers::upload`), because a value stored before this bound existed — or written by hand
|
||||
/// into the `config` table — would otherwise walk straight past the first check.
|
||||
///
|
||||
/// The subtraction is the multipart envelope: the body carries the file PLUS the boundary
|
||||
/// framing and the `caption` / `hashtags` / `client_upload_id` fields. 1 MiB is enormously more
|
||||
/// than those can occupy (see the test below, which pins it against their actual caps) and costs
|
||||
/// nothing — the alternative is a limit that is satisfiable in theory and off-by-a-header in
|
||||
/// practice.
|
||||
pub(crate) const MAX_CONFIGURABLE_UPLOAD_MB: i64 =
|
||||
(MAX_UPLOAD_BYTES as i64 - 1024 * 1024) / (1024 * 1024);
|
||||
|
||||
#[tokio::main]
|
||||
async fn main() -> Result<()> {
|
||||
@@ -118,7 +147,9 @@ async fn main() -> Result<()> {
|
||||
// the precise per-class limits from DB config (max_image/video_size_mb); this
|
||||
// layer just stops a multi-GB body from being buffered into memory before that
|
||||
// check runs. Sized generously above the default 500 MB video limit + multipart
|
||||
// overhead — if an admin raises max_video_size_mb above this, bump MAX_UPLOAD_BYTES.
|
||||
// overhead. The DB-configured limits can no longer exceed it: both are clamped to
|
||||
// MAX_CONFIGURABLE_UPLOAD_MB at write time and at read time — see that constant
|
||||
// for why a comment was not enough.
|
||||
.route(
|
||||
"/api/v1/upload",
|
||||
post(handlers::upload::upload).route_layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES)),
|
||||
@@ -467,3 +498,56 @@ async fn shutdown_signal() {
|
||||
std::process::exit(0);
|
||||
});
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{MAX_CONFIGURABLE_UPLOAD_MB, MAX_UPLOAD_BYTES};
|
||||
use crate::handlers::upload::{
|
||||
MAX_CAPTION_BYTES, MAX_CLIENT_UPLOAD_ID_BYTES, MAX_HASHTAGS_BYTES,
|
||||
};
|
||||
|
||||
/// Boundary lines, `Content-Disposition` / `Content-Type` headers and CRLFs for the four
|
||||
/// fields the handler reads. A few hundred bytes in reality; 4 KiB is a deliberately fat
|
||||
/// allowance so this test asserts the invariant rather than a precise byte count.
|
||||
const MULTIPART_FRAMING_BYTES: usize = 4 * 1024;
|
||||
|
||||
/// Both bounds on `MAX_CONFIGURABLE_UPLOAD_MB`, asserted at COMPILE time.
|
||||
///
|
||||
/// `const _: () = assert!(...)` rather than a runtime `assert!`, matching the precedent in
|
||||
/// `handlers::upload` and `services::compression`: every operand is a constant, so a
|
||||
/// violation is a build failure rather than something that has to be run to be noticed. The
|
||||
/// cost is that a const panic takes a static message — the reasoning lives here instead.
|
||||
///
|
||||
/// UPPER: a file at the largest configurable limit must still fit inside the body limit the
|
||||
/// router enforces, envelope included. If it does not, an operator can set a limit the
|
||||
/// handler accepts and the router then refuses MID-BODY — a 400 that the upload queue
|
||||
/// classifies as terminal and answers by deleting the guest's only copy of the photo. See
|
||||
/// `MAX_CONFIGURABLE_UPLOAD_MB`. Written against the field caps rather than a hardcoded
|
||||
/// number, so raising `MAX_CAPTION_LENGTH` (or adding another text field to the envelope)
|
||||
/// fails HERE instead of silently eating the margin.
|
||||
///
|
||||
/// LOWER: the ceiling must not be so conservative that it forbids the shipped default.
|
||||
/// `max_video_size_mb` is seeded at 500 (migration 005), so a bound below that would clamp
|
||||
/// every video upload on a stock install and reject the stock config through `patch_config`.
|
||||
#[test]
|
||||
fn the_configurable_ceiling_is_bounded_at_both_ends() {
|
||||
const FILE: usize = MAX_CONFIGURABLE_UPLOAD_MB as usize * 1024 * 1024;
|
||||
const ENVELOPE: usize = MAX_CAPTION_BYTES
|
||||
+ MAX_HASHTAGS_BYTES
|
||||
+ MAX_CLIENT_UPLOAD_ID_BYTES
|
||||
+ MULTIPART_FRAMING_BYTES;
|
||||
const _: () = {
|
||||
assert!(
|
||||
FILE + ENVELOPE <= MAX_UPLOAD_BYTES,
|
||||
"a file at MAX_CONFIGURABLE_UPLOAD_MB plus its multipart envelope exceeds \
|
||||
MAX_UPLOAD_BYTES — an operator could configure a limit that DESTROYS uploads \
|
||||
(400 mid-body, blob purged as terminal) instead of refusing them"
|
||||
);
|
||||
assert!(
|
||||
MAX_CONFIGURABLE_UPLOAD_MB >= 500,
|
||||
"MAX_CONFIGURABLE_UPLOAD_MB must clear the 500 MB max_video_size_mb default \
|
||||
seeded by migration 005, or a stock install clamps every video upload"
|
||||
);
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user