Merge branch 'fix/video-playback'

backend/src/handlers/upload.rs

@@ -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() {

e2e/specs/03-feed/video-playback.spec.ts

@@ -0,0 +1,172 @@
+/**
+ * Regression guard — videos must actually play.
+ *
+ * Two independent defects made every video unplayable, and nothing in the suite covered
+ * either one (no test anywhere played media or asserted a `<video>` src):
+ *
+ *  1. The lightbox fed `<video>` the URL from `pickMediaUrl`, which is mime-agnostic.
+ *     Compression only ever produces a *thumbnail* for a video — one ffmpeg frame — so in
+ *     the DEFAULT saver mode the element's src was `/api/v1/upload/{id}/thumbnail`: a JPEG,
+ *     served as image/jpeg with nosniff so the browser can't even sniff its way out.
+ *     Chromium reported DEMUXER_ERROR_COULD_NOT_OPEN.
+ *
+ *  2. `stream_media_file` ignored `Range` entirely — always 200 with the whole body, never
+ *     an 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` data mode.
+ *
+ * Fixing either alone still leaves video broken, so both are asserted here.
+ */
+import { test, expect } from '../../fixtures/test';
+import { uploadRaw } from '../../helpers/upload-client';
+import { BASE } from '../../helpers/env';
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+
+const SAMPLE_MP4 = join(process.cwd(), 'fixtures', 'media', 'sample.mp4');
+
+async function seedVideo(jwt: string): Promise<string> {
+  const res = await uploadRaw(jwt, readFileSync(SAMPLE_MP4), {
+    filename: 'clip.mp4',
+    contentType: 'video/mp4',
+    caption: 'ein Video',
+  });
+  if (res.status !== 201) throw new Error(`video seed failed: ${res.status}`);
+  return ((await res.json()) as { id: string }).id;
+}
+
+test.describe('Video — the lightbox plays it', () => {
+  test('the <video> src is the original, not the thumbnail JPEG', async ({
+    page,
+    guest,
+    signIn,
+  }) => {
+    const g = await guest('VideoWatcher');
+    const id = await seedVideo(g.jwt);
+
+    await signIn(page, g);
+    await page.goto('/feed');
+
+    const card = page.locator('article').first();
+    await expect(card).toBeVisible({ timeout: 15_000 });
+    await card.getByRole('button', { name: 'Bild vergrößern' }).click();
+
+    const video = page.locator('video');
+    await expect(video).toBeVisible({ timeout: 10_000 });
+
+    // The bug in one assertion: this was `/thumbnail` in the default data mode.
+    await expect(video).toHaveAttribute('src', `/api/v1/upload/${id}/original`);
+
+    // The poster SHOULD still be the thumbnail — that's what it's for.
+    await expect(video).toHaveAttribute('poster', `/api/v1/upload/${id}/thumbnail`);
+
+    // And the browser must accept the bytes as media. preload="none" means nothing is
+    // fetched until we ask, so drive a load explicitly and wait for metadata.
+    const readyState = await video.evaluate(async (el: HTMLVideoElement) => {
+      el.preload = 'metadata';
+      el.load();
+      await new Promise<void>((resolve) => {
+        if (el.readyState > 0) return resolve();
+        el.addEventListener('loadedmetadata', () => resolve(), { once: true });
+        el.addEventListener('error', () => resolve(), { once: true });
+        setTimeout(resolve, 15_000);
+      });
+      return { ready: el.readyState, err: el.error?.message ?? null };
+    });
+    expect(readyState.err, `the browser rejected the media: ${readyState.err}`).toBeNull();
+    expect(readyState.ready, 'video metadata must load').toBeGreaterThan(0);
+  });
+
+  test('the video body is not downloaded before the user presses play', async ({
+    page,
+    guest,
+    signIn,
+  }) => {
+    // saver mode exists to protect a guest's mobile data, and there is no smaller video
+    // derivative to fall back to — so opening the lightbox must not pull the file down.
+    //
+    // Asserting "no request at all" would be wrong: WebKit opens a connection for a
+    // preload="none" <video> and immediately ABORTS it (observed: GET with no Range,
+    // response status 0, nothing transferred), whereas Chromium issues nothing. The
+    // portable guarantee — and the one that actually protects the data plan — is that no
+    // response carrying the body ever completes. Dropping preload="none" fails this.
+    const g = await guest('VideoThrifty');
+    const id = await seedVideo(g.jwt);
+
+    await signIn(page, g);
+    const delivered: string[] = [];
+    page.on('response', (r) => {
+      if (!r.url().includes(`/upload/${id}/original`)) return;
+      // status 0 = aborted before any bytes landed.
+      if (r.status() === 200 || r.status() === 206) {
+        delivered.push(`${r.status()} len=${r.headers()['content-length'] ?? '?'}`);
+      }
+    });
+
+    await page.goto('/feed');
+    const card = page.locator('article').first();
+    await expect(card).toBeVisible({ timeout: 15_000 });
+    await card.getByRole('button', { name: 'Bild vergrößern' }).click();
+    await expect(page.locator('video')).toBeVisible({ timeout: 10_000 });
+    await page.waitForTimeout(2000);
+
+    expect(delivered, 'no video bytes may be delivered before play').toEqual([]);
+  });
+});
+
+test.describe('Media — HTTP Range', () => {
+  test('a range request is answered 206 with the right slice', async ({ guest }) => {
+    const g = await guest('RangeReader');
+    const id = await seedVideo(g.jwt);
+    const url = `${BASE}/api/v1/upload/${id}/original`;
+
+    const full = await fetch(url);
+    expect(full.status).toBe(200);
+    expect(full.headers.get('accept-ranges'), 'clients must be told seeking works').toBe('bytes');
+    const total = Number(full.headers.get('content-length'));
+    expect(total).toBeGreaterThan(0);
+
+    // The exact probe iOS Safari opens a <video> with. Two bytes, inclusive.
+    const probe = await fetch(url, { headers: { Range: 'bytes=0-1' } });
+    expect(probe.status, 'iOS abandons the load without a 206').toBe(206);
+    expect(probe.headers.get('content-range')).toBe(`bytes 0-1/${total}`);
+    expect(Number(probe.headers.get('content-length'))).toBe(2);
+    expect((await probe.arrayBuffer()).byteLength).toBe(2);
+
+    // A mid-file seek must return the matching slice, not the whole body.
+    const mid = await fetch(url, { headers: { Range: 'bytes=10-19' } });
+    expect(mid.status).toBe(206);
+    expect(mid.headers.get('content-range')).toBe(`bytes 10-19/${total}`);
+    const midBytes = Buffer.from(await mid.arrayBuffer());
+    expect(midBytes.byteLength).toBe(10);
+    expect(midBytes).toEqual(Buffer.from(await full.arrayBuffer()).subarray(10, 20));
+
+    // Open-ended range runs to EOF.
+    const tail = await fetch(url, { headers: { Range: `bytes=${total - 5}-` } });
+    expect(tail.status).toBe(206);
+    expect(Number(tail.headers.get('content-length'))).toBe(5);
+
+    // Past EOF must be 416 — answering 200 makes a player re-request forever.
+    const bad = await fetch(url, { headers: { Range: `bytes=${total + 10}-` } });
+    expect(bad.status).toBe(416);
+    expect(bad.headers.get('content-range')).toBe(`bytes */${total}`);
+  });
+
+  test('image derivatives are range-capable too', async ({ guest, db }) => {
+    // Same helper serves all four media routes, so the guarantee is uniform.
+    const g = await guest('RangeImages');
+    const res = await uploadRaw(
+      g.jwt,
+      readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg')),
+      { filename: 'r.jpg', contentType: 'image/jpeg' }
+    );
+    const { id } = (await res.json()) as { id: string };
+    await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
+
+    const preview = await fetch(`${BASE}/api/v1/upload/${id}/preview`, {
+      headers: { Range: 'bytes=0-9' },
+    });
+    expect(preview.status).toBe(206);
+    expect(Number(preview.headers.get('content-length'))).toBe(10);
+  });
+});

frontend/src/lib/components/LightboxModal.svelte

@@ -41,7 +41,20 @@
 	let heartBurst = $state(false);
 	let burstTimer: ReturnType<typeof setTimeout> | null = null;
 
-	const mediaSrc = $derived(pickMediaUrl($dataMode, upload));
+	// Videos always play the ORIGINAL. `pickMediaUrl` is mime-agnostic and compression only
+	// ever produces a *thumbnail* for a video (one ffmpeg frame), so in the default saver
+	// mode it hands back `/thumbnail` — a JPEG, served as image/jpeg with nosniff. Feeding
+	// that to <video> is why every video failed with DEMUXER_ERROR_COULD_NOT_OPEN. There is
+	// no smaller video derivative to offer, so `preload="none"` keeps saver-mode users on
+	// cellular from fetching anything until they actually press play; the poster is the
+	// thumbnail, which is what they saw in the feed anyway.
+	// Fixed here rather than in `pickMediaUrl` because FeedListCard shares that helper and
+	// legitimately wants the thumbnail for its <img>. Same rule as the diashow.
+	const mediaSrc = $derived(
+		isVideo(upload.mime_type)
+			? `/api/v1/upload/${upload.id}/original`
+			: pickMediaUrl($dataMode, upload)
+	);
 
 	function triggerHeartBurst() {
 		heartBurst = true;
@@ -177,6 +190,8 @@
 					<video
 						src={mediaSrc}
 						controls
+						preload="none"
+						playsinline
 						class="max-h-[60vh] w-full object-contain"
 						poster={upload.thumbnail_url ?? undefined}
 					></video>