fabi/EventSnap
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(video): play the actual video, and answer Range requests

Browse Source 
Every video in the app was unplayable. Two independent defects, either one
sufficient on its own, and nothing in the suite covered either — no test
anywhere played media or asserted a `<video>` src.

1. The lightbox handed `<video>` a JPEG.

`pickMediaUrl` is mime-agnostic, and compression only ever produces a THUMBNAIL
for a video (one `ffmpeg -vframes 1` frame) — no preview, no display. So in the
DEFAULT saver mode the element's src resolved to `/api/v1/upload/{id}/thumbnail`,
served as `image/jpeg` with `nosniff` so the browser can't even sniff its way
out. Chromium reports DEMUXER_ERROR_COULD_NOT_OPEN.

Fixed in the lightbox rather than in `pickMediaUrl`: FeedListCard shares that
helper and legitimately wants the thumbnail for its `<img>` poster, so a central
mime branch would break the feed. This mirrors the rule the diashow already
applies ("videos play the original file directly"). Added `preload="none"` so
saver-mode guests on cellular still fetch nothing until they press play — there
is no smaller video derivative to offer them — plus `playsinline`, without which
iOS hijacks playback into fullscreen.

2. `stream_media_file` ignored Range entirely.

It took no request headers, so it could not see `Range`; it always returned 200
with the whole body and never sent Accept-Ranges or Content-Range. iOS Safari
opens every `<video>` with a `Range: bytes=0-1` probe and abandons the load
without a 206 — so video failed on the app's primary platform even in `original`
mode, where the src was already correct.

Adds single-range support (`bytes=N-`, `bytes=N-M`, `bytes=-S`) with 206 +
Content-Range, 416 + `bytes */len` past EOF, and Accept-Ranges advertised on
every response. Anything it won't handle — multi-range, non-bytes units, garbage
— falls back to a full 200, which RFC 9110 explicitly permits and which is safer
than guessing. All four media routes share the helper, so seeking works
uniformly.

`get_original` now serves `inline` instead of `attachment`. An attachment
disposition is hostile to a `<video>` element, and this route is the only source
of playable video bytes; it also matches what the UI promises, since the action
is labelled "Original anzeigen" — view, not download. `no-store` is deliberately
kept so a takedown still revokes access promptly; ranges work fine under it, the
client just re-fetches.

Tests: 11 unit tests pin the parser (the iOS `bytes=0-1` probe, inclusive ends,
suffix ranges, clamping past EOF, 416 vs 200, malformed fallbacks). A new
03-feed/video-playback spec asserts the src is the original and not the
thumbnail, that the browser accepts the bytes as media (readyState > 0, no
MediaError), that no video bytes are delivered before play, and that Range
returns the correct 206 slices and a 416 past EOF — verified on both Chromium
and WebKit.

The "not downloaded before play" test asserts no *delivered body* rather than no
request: WebKit opens a connection for a preload="none" video and immediately
aborts it (GET, no Range, status 0, nothing transferred) while Chromium issues
nothing at all. The portable guarantee is that no response carrying bytes
completes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
fabi
2026-07-28 20:29:16 +02:00
parent f03e392f8c
commit 813a9fa500
3 changed files with 429 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

260
backend/src/handlers/upload.rs
View File

@@ -647,12 +647,95 @@ pub async fn compute_storage_quota(state: &AppState) -> QuotaEstimate {
}
}
/// Outcome of parsing a `Range` request header against a known file length.
#[derive(Debug, PartialEq, Eq)]
enum RangeSpec {
/// No `Range` header, or one we deliberately don't honour (multi-range, non-`bytes`
/// unit, malformed). RFC 9110 lets a server ignore a Range it can't process and reply
/// 200 with the full body, which is what every one of these cases does.
Full,
/// A single satisfiable range, resolved to inclusive absolute offsets.
Partial { start: u64, end: u64 },
/// Syntactically valid but starts beyond EOF — must be answered 416, not 200, or a
/// player can loop re-requesting it.
Unsatisfiable,
}
/// Parse a single-range `bytes=` header against `len`.
///
/// Deliberately supports only the three forms a media element actually sends —
/// `bytes=N-`, `bytes=N-M`, `bytes=-S` (suffix) — and treats everything else as `Full`.
/// Multi-range responses need `multipart/byteranges`, which no `<video>` requires.
fn parse_range(header: Option<&str>, len: u64) -> RangeSpec {
let Some(raw) = header else {
return RangeSpec::Full;
};
let Some(spec) = raw.trim().strip_prefix("bytes=") else {
return RangeSpec::Full;
};
// Multi-range → fall back to the whole body rather than lie about the content.
if spec.contains(',') {
return RangeSpec::Full;
}
let Some((from, to)) = spec.split_once('-') else {
return RangeSpec::Full;
};
let (from, to) = (from.trim(), to.trim());
// A zero-length file can satisfy no range at all.
if len == 0 {
return if from.is_empty() && to.is_empty() {
RangeSpec::Full
} else {
RangeSpec::Unsatisfiable
};
}
let (start, end) = if from.is_empty() {
// Suffix form: the last `to` bytes.
let Ok(suffix) = to.parse::<u64>() else {
return RangeSpec::Full;
};
if suffix == 0 {
return RangeSpec::Unsatisfiable;
}
(len.saturating_sub(suffix), len - 1)
} else {
let Ok(start) = from.parse::<u64>() else {
return RangeSpec::Full;
};
let end = if to.is_empty() {
len - 1
} else {
match to.parse::<u64>() {
// An end past EOF is clamped, not rejected (RFC 9110 §14.1.1).
Ok(end) => end.min(len - 1),
Err(_) => return RangeSpec::Full,
}
};
(start, end)
};
if start >= len || start > end {
RangeSpec::Unsatisfiable
} else {
RangeSpec::Partial { start, end }
}
}
/// Stream a media file from disk into an HTTP response with a fixed set of security
/// headers. Every media response (original, preview, thumbnail) goes through here so
/// they consistently carry `X-Content-Type-Options: nosniff` (defense-in-depth against
/// headers. Every media response (original, preview, display, thumbnail) goes through here
/// so they consistently carry `X-Content-Type-Options: nosniff` (defense-in-depth against
/// content-type confusion, even if the edge proxy is bypassed) plus an explicit
/// `Content-Disposition` and `Cache-Control`.
///
/// Honours a single `Range`. This is not an optimisation: iOS Safari opens every `<video>`
/// with a `Range: bytes=0-1` probe and abandons the load unless it gets a `206` with a
/// `Content-Range`. Without this, video is unplayable on the app's primary platform no
/// matter what `src` the element is given. `Accept-Ranges: bytes` is advertised on every
/// response so clients know seeking is available before they ask.
async fn stream_media_file(
req_headers: &axum::http::HeaderMap,
absolute: &std::path::Path,
content_type: String,
disposition: &str,
@@ -660,30 +743,65 @@ async fn stream_media_file(
) -> Result<axum::response::Response, AppError> {
use axum::body::Body;
use axum::http::{Response, StatusCode, header};
use tokio::io::{AsyncReadExt, AsyncSeekExt};
use tokio_util::io::ReaderStream;
if !absolute.exists() {
return Err(AppError::NotFound("Datei nicht gefunden.".into()));
}
let file = tokio::fs::File::open(absolute)
let mut file = tokio::fs::File::open(absolute)
.await
.map_err(|e| AppError::Internal(e.into()))?;
let metadata = file
let len = file
.metadata()
.await
.map_err(|e| AppError::Internal(e.into()))?;
let stream = ReaderStream::new(file);
.map_err(|e| AppError::Internal(e.into()))?
.len();
Response::builder()
.status(StatusCode::OK)
.header(header::CONTENT_TYPE, content_type)
.header(header::CONTENT_DISPOSITION, disposition)
.header(header::CONTENT_LENGTH, metadata.len())
.header(header::CACHE_CONTROL, cache_control)
.header(header::X_CONTENT_TYPE_OPTIONS, "nosniff")
.body(Body::from_stream(stream))
.map_err(|e| AppError::Internal(e.into()))
let range = parse_range(
req_headers
.get(header::RANGE)
.and_then(|v| v.to_str().ok()),
len,
);
let base = |status: StatusCode| {
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, content_type.clone())
.header(header::CONTENT_DISPOSITION, disposition)
.header(header::CACHE_CONTROL, cache_control)
.header(header::X_CONTENT_TYPE_OPTIONS, "nosniff")
.header(header::ACCEPT_RANGES, "bytes")
};
match range {
RangeSpec::Full => base(StatusCode::OK)
.header(header::CONTENT_LENGTH, len)
.body(Body::from_stream(ReaderStream::new(file)))
.map_err(|e| AppError::Internal(e.into())),
RangeSpec::Partial { start, end } => {
file.seek(std::io::SeekFrom::Start(start))
.await
.map_err(|e| AppError::Internal(e.into()))?;
let span = end - start + 1;
base(StatusCode::PARTIAL_CONTENT)
.header(header::CONTENT_LENGTH, span)
.header(
header::CONTENT_RANGE,
format!("bytes {start}-{end}/{len}"),
)
.body(Body::from_stream(ReaderStream::new(file.take(span))))
.map_err(|e| AppError::Internal(e.into()))
}
RangeSpec::Unsatisfiable => base(StatusCode::RANGE_NOT_SATISFIABLE)
.header(header::CONTENT_RANGE, format!("bytes */{len}"))
.body(Body::empty())
.map_err(|e| AppError::Internal(e.into())),
}
}
/// Streaming download of the original file behind an upload. Used by:
@@ -700,6 +818,7 @@ async fn stream_media_file(
/// [`get_preview`] / [`get_thumbnail`]).
pub async fn get_original(
State(state): State<AppState>,
headers: axum::http::HeaderMap,
Path(upload_id): Path<Uuid>,
) -> Result<axum::response::Response, AppError> {
let media = Upload::find_visible_media(&state.pool, upload_id)
@@ -711,10 +830,15 @@ pub async fn get_original(
.file_name()
.and_then(|n| n.to_str())
.unwrap_or("original");
let disposition = format!("attachment; filename=\"{filename}\"");
// `inline`, not `attachment`. This route is the only source of playable video bytes
// (there is no video derivative), and an attachment disposition is hostile to a
// `<video>` element — Safari in particular. It also matches what the UI promises:
// the action is labelled "Original anzeigen", i.e. view, not download.
let disposition = format!("inline; filename=\"{filename}\"");
// Full-res original: force download, never cache at the edge.
stream_media_file(&absolute, media.mime_type, &disposition, "no-store").await
// Full-res original: never cache at the edge, so a takedown revokes access promptly.
// Range requests still work under no-store; the client simply re-fetches each range.
stream_media_file(&headers, &absolute, media.mime_type, &disposition, "no-store").await
}
/// Streaming access to an upload's compressed **preview** image. Gated exactly like
@@ -729,6 +853,7 @@ pub async fn get_original(
/// `upload-deleted` / `user-hidden` SSE events, so this only bounds the raw-URL edge case.
pub async fn get_preview(
State(state): State<AppState>,
headers: axum::http::HeaderMap,
Path(upload_id): Path<Uuid>,
) -> Result<axum::response::Response, AppError> {
let media = Upload::find_visible_media(&state.pool, upload_id)
@@ -739,6 +864,7 @@ pub async fn get_preview(
.ok_or_else(|| AppError::NotFound("Vorschau nicht verfügbar.".into()))?;
let absolute = state.config.media_path.join(&rel);
stream_media_file(
&headers,
&absolute,
"image/jpeg".to_string(),
"inline",
@@ -753,6 +879,7 @@ pub async fn get_preview(
/// back to the original.
pub async fn get_display(
State(state): State<AppState>,
headers: axum::http::HeaderMap,
Path(upload_id): Path<Uuid>,
) -> Result<axum::response::Response, AppError> {
let media = Upload::find_visible_media(&state.pool, upload_id)
@@ -763,6 +890,7 @@ pub async fn get_display(
.ok_or_else(|| AppError::NotFound("Anzeige nicht verfügbar.".into()))?;
let absolute = state.config.media_path.join(&rel);
stream_media_file(
&headers,
&absolute,
"image/jpeg".to_string(),
"inline",
@@ -775,6 +903,7 @@ pub async fn get_display(
/// [`get_preview`].
pub async fn get_thumbnail(
State(state): State<AppState>,
headers: axum::http::HeaderMap,
Path(upload_id): Path<Uuid>,
) -> Result<axum::response::Response, AppError> {
let media = Upload::find_visible_media(&state.pool, upload_id)
@@ -785,6 +914,7 @@ pub async fn get_thumbnail(
.ok_or_else(|| AppError::NotFound("Thumbnail nicht verfügbar.".into()))?;
let absolute = state.config.media_path.join(&rel);
stream_media_file(
&headers,
&absolute,
"image/jpeg".to_string(),
"inline",
@@ -795,7 +925,99 @@ pub async fn get_thumbnail(
#[cfg(test)]
mod tests {
use super::quota_limit_bytes;
use super::{RangeSpec, parse_range, quota_limit_bytes};
// `Range` handling exists because iOS Safari probes every `<video>` with
// `Range: bytes=0-1` and abandons the load without a 206. These pin the forms a
// media element actually sends, plus the edges that decide 206 vs 200 vs 416.
#[test]
fn no_range_header_is_a_full_response() {
assert_eq!(parse_range(None, 100), RangeSpec::Full);
}
#[test]
fn open_ended_range_runs_to_eof() {
assert_eq!(
parse_range(Some("bytes=10-"), 100),
RangeSpec::Partial { start: 10, end: 99 }
);
}
#[test]
fn closed_range_is_inclusive_on_both_ends() {
// The iOS probe. Two bytes, 0 and 1 — an exclusive end would return one.
assert_eq!(
parse_range(Some("bytes=0-1"), 100),
RangeSpec::Partial { start: 0, end: 1 }
);
}
#[test]
fn suffix_range_returns_the_last_n_bytes() {
assert_eq!(
parse_range(Some("bytes=-20"), 100),
RangeSpec::Partial { start: 80, end: 99 }
);
}
#[test]
fn suffix_longer_than_the_file_clamps_to_the_whole_file() {
assert_eq!(
parse_range(Some("bytes=-500"), 100),
RangeSpec::Partial { start: 0, end: 99 }
);
}
#[test]
fn end_past_eof_is_clamped_not_rejected() {
// RFC 9110 §14.1.1 — players routinely ask for more than is there.
assert_eq!(
parse_range(Some("bytes=90-999"), 100),
RangeSpec::Partial { start: 90, end: 99 }
);
}
#[test]
fn start_past_eof_is_416_not_a_full_body() {
// Answering 200 here makes a player re-request forever.
assert_eq!(parse_range(Some("bytes=100-"), 100), RangeSpec::Unsatisfiable);
assert_eq!(parse_range(Some("bytes=200-300"), 100), RangeSpec::Unsatisfiable);
}
#[test]
fn inverted_range_is_unsatisfiable() {
assert_eq!(parse_range(Some("bytes=50-10"), 100), RangeSpec::Unsatisfiable);
}
#[test]
fn unsupported_or_malformed_forms_fall_back_to_the_full_body() {
// Ignoring a Range we can't process and sending 200 is explicitly allowed, and
// safer than guessing. Multi-range would need multipart/byteranges, which no
// <video> asks for.
for header in [
"bytes=0-10,20-30", // multi-range
"items=0-10", // non-bytes unit
"bytes=abc-def", // garbage
"bytes=", // empty spec
"nonsense", // no unit at all
] {
assert_eq!(parse_range(Some(header), 100), RangeSpec::Full, "{header}");
}
}
#[test]
fn last_byte_is_reachable() {
assert_eq!(
parse_range(Some("bytes=99-99"), 100),
RangeSpec::Partial { start: 99, end: 99 }
);
}
#[test]
fn empty_file_satisfies_no_range() {
assert_eq!(parse_range(Some("bytes=0-"), 0), RangeSpec::Unsatisfiable);
assert_eq!(parse_range(None, 0), RangeSpec::Full);
}
#[test]
fn divides_free_space_by_uploaders_with_tolerance() {