Merge branch 'fix/export-exif-orientation'

backend/src/services/compression.rs

@@ -3,7 +3,6 @@ use std::sync::Arc;
 use std::sync::atomic::{AtomicU64, Ordering};
 
 use anyhow::{Context, Result};
-use image::ImageDecoder;
 use sqlx::PgPool;
 use tokio::sync::{Semaphore, broadcast};
 use uuid::Uuid;
@@ -205,38 +204,9 @@ impl CompressionWorker {
 
         // Run blocking image operations in a spawn_blocking task
         tokio::task::spawn_blocking(move || -> Result<()> {
-            // Reject decompression bombs *before* fully decoding: the upload body
+            // Decompression-bomb limits + EXIF orientation, both in one place — see
-            // cap bounds the file size on disk, but a small file can still decode to
+            // services::imaging for why neither may be skipped.
-            // enormous dimensions (e.g. a ~1 MB image expanding to 50k×50k px →
+            let img = crate::services::imaging::decode_oriented(&original)?;
-            // gigabytes), OOM-ing the box during decode/resize. 12000×12000 covers
-            // any real phone photo; max_alloc hard-caps the decode allocation.
-            let mut reader = image::ImageReader::open(&original)
-                .context("failed to open image")?
-                .with_guessed_format()
-                .context("failed to read image header")?;
-            let mut limits = image::Limits::default();
-            limits.max_image_width = Some(12_000);
-            limits.max_image_height = Some(12_000);
-            limits.max_alloc = Some(256 * 1024 * 1024);
-            reader.limits(limits);
-
-            // Apply the EXIF orientation. Phones do not rotate the sensor data — they record
-            // the physical camera orientation in a tag and store the pixels as shot. `decode()`
-            // hands back those raw pixels, and the JPEG re-encode below writes no EXIF at all,
-            // so skipping this stores EVERY portrait photo sideways in the feed preview, the
-            // 2048px diashow display and the keepsake — while the untouched original still
-            // renders upright, which is why it looks like a viewer bug rather than a pipeline
-            // one. `into_decoder` carries the limits set above through to the decoder, so the
-            // decompression-bomb guard is unaffected.
-            let mut decoder = reader.into_decoder().context("failed to decode image")?;
-            // A missing or malformed tag is not a failure: most images simply have none.
-            let orientation = decoder
-                .orientation()
-                .unwrap_or(image::metadata::Orientation::NoTransforms);
-            let mut img =
-                image::DynamicImage::from_decoder(decoder).context("failed to decode image")?;
-            img.apply_orientation(orientation);
-            let img = img;
 
             // Preview: max 800px, preserving aspect ratio (data-saver feed).
             img.resize(

backend/src/services/export.rs

@@ -791,7 +791,12 @@ async fn run_html_export_inner(
             let thumb_path_clone = thumb_path.clone();
 
             let thumb_result = tokio::task::spawn_blocking(move || -> Result<()> {
-                let img = image::open(&src_clone).context("failed to open image for thumbnail")?;
+                // `decode_oriented`, not `image::open`: the latter ignores the EXIF
+                // orientation tag AND applies no decode limits. Using it here is why every
+                // portrait photo came out sideways in the keepsake's HTML viewer grid — the
+                // re-encode below drops the tag, so the viewer cannot recover it.
+                let img = crate::services::imaging::decode_oriented(&src_clone)
+                    .context("failed to open image for thumbnail")?;
                 let resized = img.resize(400, 400, image::imageops::FilterType::Lanczos3);
                 resized
                     .save_with_format(&thumb_path_clone, image::ImageFormat::Jpeg)
@@ -812,8 +817,12 @@ async fn run_html_export_inner(
                 let full_path_clone = full_path.clone();
 
                 let compress_result = tokio::task::spawn_blocking(move || -> Result<()> {
-                    let img =
+                    // Same reason as the thumbnail above. This branch only runs for originals
-                        image::open(&src_clone).context("failed to open image for compression")?;
+                    // over 5 MB, which is why the viewer's full image looked correct for small
+                    // photos and sideways for large ones — an inconsistency that reads as a
+                    // viewer bug rather than an export one.
+                    let img = crate::services::imaging::decode_oriented(&src_clone)
+                        .context("failed to open image for compression")?;
                     let resized = img.resize(2000, 2000, image::imageops::FilterType::Lanczos3);
                     resized
                         .save_with_format(&full_path_clone, image::ImageFormat::Jpeg)

backend/src/services/imaging.rs

@@ -0,0 +1,52 @@
+//! Shared image decoding.
+//!
+//! Exists so there is exactly ONE way to turn a file on disk into a `DynamicImage` in this
+//! codebase. Two properties have to hold everywhere an image is decoded, and both were
+//! previously re-derived per call site — which is how they drifted apart:
+//!
+//! - **EXIF orientation must be applied.** Phones do not rotate sensor data; they record how
+//!   the camera was held in a tag and store the pixels as shot. `image::open` and
+//!   `ImageReader::decode` both hand back the raw pixels and ignore that tag, and re-encoding
+//!   to JPEG writes no EXIF, so the derivative is permanently sideways while the untouched
+//!   original still renders upright. The compression worker was fixed; the export worker was
+//!   not, so every portrait photo came out sideways in the keepsake's HTML viewer.
+//! - **Decode limits must be set.** The upload body cap bounds the file on disk, but a small
+//!   file can decode to enormous dimensions (a ~1 MB image expanding to 50k×50k px), OOM-ing
+//!   the box. `image::open` applies NO limits at all, so the export path was also decoding
+//!   arbitrary user-supplied images unbounded.
+
+use anyhow::{Context, Result};
+use image::{DynamicImage, ImageDecoder};
+use std::path::Path;
+
+/// Bounds for any decode of user-supplied image data. 12000×12000 covers any real phone
+/// photo; `max_alloc` hard-caps the decode allocation.
+fn decode_limits() -> image::Limits {
+    let mut limits = image::Limits::default();
+    limits.max_image_width = Some(12_000);
+    limits.max_image_height = Some(12_000);
+    limits.max_alloc = Some(256 * 1024 * 1024);
+    limits
+}
+
+/// Decode an image from disk with decompression-bomb limits applied and its EXIF
+/// orientation baked into the pixels.
+///
+/// Blocking — call inside `spawn_blocking`.
+pub fn decode_oriented(path: &Path) -> Result<DynamicImage> {
+    let mut reader = image::ImageReader::open(path)
+        .context("failed to open image")?
+        .with_guessed_format()
+        .context("failed to read image header")?;
+    reader.limits(decode_limits());
+
+    // `into_decoder` carries the limits above through, so reading the tag costs nothing in
+    // safety. A missing or malformed tag is not an error — most images simply have none.
+    let mut decoder = reader.into_decoder().context("failed to decode image")?;
+    let orientation = decoder
+        .orientation()
+        .unwrap_or(image::metadata::Orientation::NoTransforms);
+    let mut img = DynamicImage::from_decoder(decoder).context("failed to decode image")?;
+    img.apply_orientation(orientation);
+    Ok(img)
+}

backend/src/services/mod.rs

@@ -2,6 +2,7 @@ pub mod compression;
 pub mod config;
 pub mod disk;
 pub mod export;
+pub mod imaging;
 pub mod maintenance;
 pub mod rate_limiter;
 pub mod sse_tickets;

e2e/specs/06-export/exif-orientation.spec.ts

@@ -0,0 +1,117 @@
+/**
+ * Regression guard — the keepsake must not be sideways.
+ *
+ * Round 1 taught the compression worker to apply EXIF orientation, which fixed the live app
+ * (feed preview + diashow display). The export worker was missed: it does NOT reuse those
+ * derivatives — it re-decodes the originals itself with `image::open`, which ignores the
+ * orientation tag — and then re-encodes to JPEG, which drops the tag, so the viewer has no
+ * way to recover it.
+ *
+ * The resulting damage was oddly shaped, which is what made it read as a viewer bug:
+ *   - Gallery.zip originals            → correct (byte-copied, EXIF intact)
+ *   - Memories viewer grid thumbnails  → ALWAYS sideways
+ *   - Memories viewer full image >5 MB → sideways (re-encoded at 2000px)
+ *   - Memories viewer full image ≤5 MB → correct (streamed byte-for-byte)
+ *
+ * So clicking a small photo silently "fixed" it and a large one didn't. This pins the
+ * thumbnail, which is the path every photo takes.
+ */
+import { test, expect } from '../../fixtures/test';
+import { uploadRaw } from '../../helpers/upload-client';
+import { BASE } from '../../helpers/env';
+import { execFileSync } from 'node:child_process';
+import { mkdtempSync, readFileSync, writeFileSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+// 40x20 landscape pixels tagged Orientation=6 ("rotate 90° CW to display"), so anything
+// that honours the tag emits a PORTRAIT derivative.
+const EXIF_FIXTURE = join(process.cwd(), 'fixtures', 'media', 'portrait-exif6.jpg');
+
+/** Pixel dimensions from a JPEG's SOF marker — avoids an image dep for one assertion. */
+function jpegSize(buf: Buffer): { width: number; height: number } {
+  let i = 2;
+  while (i < buf.length) {
+    if (buf[i] !== 0xff) {
+      i++;
+      continue;
+    }
+    const marker = buf[i + 1];
+    if (marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc) {
+      return { height: buf.readUInt16BE(i + 5), width: buf.readUInt16BE(i + 7) };
+    }
+    i += 2 + buf.readUInt16BE(i + 2);
+  }
+  throw new Error('no SOF marker found — not a JPEG?');
+}
+
+test.describe('Export — EXIF orientation in the keepsake', () => {
+  test('the Memories viewer thumbnail of a rotated photo is upright', async ({ host, db }) => {
+    test.setTimeout(90_000);
+    const bearer = { Authorization: `Bearer ${host.jwt}` };
+
+    const src = readFileSync(EXIF_FIXTURE);
+    // Sanity: the SOURCE really is stored landscape, or this test proves nothing.
+    const srcSize = jpegSize(src);
+    expect(srcSize.width).toBeGreaterThan(srcSize.height);
+
+    const up = await uploadRaw(host.jwt, src, {
+      filename: 'hochkant.jpg',
+      contentType: 'image/jpeg',
+      caption: 'hochkant',
+    });
+    expect(up.status).toBe(201);
+    const { id } = (await up.json()) as { id: string };
+    await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
+
+    const rel = await fetch(`${BASE}/api/v1/host/gallery/release`, {
+      method: 'POST',
+      headers: bearer,
+    });
+    expect(rel.status).toBe(204);
+
+    await expect
+      .poll(
+        async () => {
+          const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
+          return (await res.json()).html?.status;
+        },
+        { timeout: 60_000, intervals: [500] }
+      )
+      .toBe('done');
+
+    const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
+      method: 'POST',
+      headers: bearer,
+    });
+    const { ticket } = (await ticketRes.json()) as { ticket: string };
+    const dl = await fetch(`${BASE}/api/v1/export/html?ticket=${encodeURIComponent(ticket)}`);
+    expect(dl.status).toBe(200);
+
+    const dir = mkdtempSync(join(tmpdir(), 'eventsnap-exif-'));
+    try {
+      const zipPath = join(dir, 'Memories.zip');
+      writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
+
+      const entries = execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' })
+        .split('\n')
+        .filter(Boolean);
+      const thumbEntry = entries.find((e) => e.includes(`${id}_thumb`));
+      expect(thumbEntry, `no thumbnail for ${id} in Memories.zip`).toBeTruthy();
+
+      // `-p` streams the entry to stdout. Extracting to disk instead fails with EACCES:
+      // the archive preserves the container's file mode, which the test user can't read.
+      const thumb = execFileSync('unzip', ['-p', zipPath, thumbEntry!], {
+        maxBuffer: 64 * 1024 * 1024,
+      });
+      const { width, height } = jpegSize(thumb);
+
+      expect(
+        height,
+        `the keepsake grid thumbnail is ${width}x${height} — EXIF orientation was not applied`
+      ).toBeGreaterThan(width);
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});