fix(images): respect EXIF orientation in thumbnails, heroes, previews and watermarks (#1185) (#1202)

Stable twin of #1194.

generateThumbnail, generateHeroImage and generatePreviewImage went straight
from sharp(imagePath) to .resize(), so a photo whose Orientation tag is not 1 —
routine for portrait shots on bodies that tag rather than rotate the sensor
data — was resized from the raw frame and came out sideways. The same pipelines
then call .withMetadata(false), stripping the tag from the output, so nothing
downstream could correct it either. The download path already had this right,
which is why the same photo looked correct on download and rotated in the
gallery.

watermarkService had it too, and it is the one a guest actually sees:
gallery.js serves photos.watermark_path ahead of the original when branding
watermarking is on. Two details there — metadata() is read from a separate
unrotated handle, because .rotate() does not change what it reports and every
use of those numbers is positioning; and the composite offsets are floored,
because getPositionCoordinates returns fractional pixels, sharp rejects them,
and applyWatermark catches its own error and silently returns the image
unwatermarked.

The rotate is unconditional in the thumbnail and hero generators — neither
passes `animated: true`, so both already flatten a multi-frame source and
guarding there would protect an animation that was being discarded anyway.
generatePreviewImage keeps the guard, since it genuinely does preserve
animation.

photos.width/height were stored from sharp's metadata, which reports pixels as
STORED, not displayed. For orientation 5-8 those are swapped, so a portrait
photo landed in the database as landscape and masonry sized its tile with the
wrong aspect ratio on top of the image being unrotated. A shared
orientedDimensions() helper now does the conversion at all eight image ingest
sites. The video path is deliberately untouched: its dimensions come from
ffprobe, where EXIF orientation does not apply.

Existing rows keep their pre-rotation dimensions until reprocessed; the backfill
for those is #1199 on main and is not ported here yet.

Co-authored-by: Paul Nothaft <[email protected]>

backend/__tests__/services/imageProcessorOrientation.test.js

@@ -0,0 +1,159 @@
+/**
+ * EXIF orientation in the gallery-facing generators (#1185).
+ *
+ * sharp decodes the pixels as stored, not as displayed. A photo whose
+ * Orientation tag is not 1 — routine for portrait shots on bodies that tag
+ * rather than rotate the sensor data — therefore resizes from the raw frame
+ * and comes out sideways. The generators then call `.withMetadata(false)`,
+ * which strips the tag from the output, so the browser has no hint left to
+ * correct it either: nothing downstream can recover it.
+ *
+ * The download path (`resizeToBox`) always got this right. These three did
+ * not, which is why the same photo looked correct on download and rotated in
+ * the gallery.
+ *
+ * Every assertion here fails on the unfixed generators.
+ */
+
+const fs = require('fs').promises;
+const path = require('path');
+const os = require('os');
+const sharp = require('sharp');
+
+// imageProcessor reads its thumbnail settings from app_settings, so without a
+// db the require() alone hangs the run. Same shape the other generator tests
+// use (ensureHeroImage.external.test.js).
+jest.mock('../../src/database/db', () => {
+  const api = (table) => {
+    if (table === 'app_settings') {
+      return { where: () => ({ whereIn: () => [], first: async () => null }), whereIn: async () => [] };
+    }
+    if (table === 'events') return { where: () => ({ first: async () => null }) };
+    if (table === 'photos') return { where: () => ({ update: async () => 1, first: async () => null }) };
+    return { where: () => ({ first: async () => null }) };
+  };
+  return { db: api };
+});
+
+const LocalFsStorage = require('../../src/services/storage/LocalFsStorage');
+const storageModule = require('../../src/services/storage');
+
+describe('EXIF orientation in thumbnails, heroes and previews (#1185)', () => {
+  let tmpDir;
+  let imageProcessor;
+  let landscapeTaggedPortrait;
+
+  // 400x200 as stored, Orientation 6 (90° CW) — so it DISPLAYS as 200x400.
+  // This is exactly the shape the reporter's Sony bodies produce: the sensor
+  // data is landscape and the tag carries the rotation.
+  const W = 400;
+  const H = 200;
+
+  let storageRoot;
+
+  beforeAll(async () => {
+    tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-orient-'));
+    storageRoot = path.join(tmpDir, 'storage');
+    await fs.mkdir(storageRoot, { recursive: true });
+    process.env.STORAGE_PATH = storageRoot;
+
+    const storage = new LocalFsStorage({ root: storageRoot });
+    await storage.init();
+    storageModule.setStorageForTesting(storage);
+
+    landscapeTaggedPortrait = path.join(tmpDir, 'portrait-tagged.jpg');
+    await sharp({
+      create: { width: W, height: H, channels: 3, background: { r: 120, g: 80, b: 40 } },
+    })
+      .withMetadata({ orientation: 6 })
+      .jpeg()
+      .toFile(landscapeTaggedPortrait);
+
+    delete require.cache[require.resolve('../../src/services/imageProcessor')];
+    imageProcessor = require('../../src/services/imageProcessor');
+  }, 60000);
+
+  afterAll(async () => {
+    await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
+  });
+
+  test('the fixture really is stored landscape with a rotation tag', async () => {
+    const m = await sharp(landscapeTaggedPortrait).metadata();
+    expect(m.width).toBe(W);
+    expect(m.height).toBe(H);
+    expect(m.orientation).toBe(6);
+  });
+
+  test('orientedDimensions reports what a viewer sees, not what is stored', async () => {
+    const m = await sharp(landscapeTaggedPortrait).metadata();
+    // Swapped: this is what the grid needs to size a tile, and what the
+    // database should hold.
+    expect(imageProcessor.orientedDimensions(m)).toEqual({ width: H, height: W });
+  });
+
+  test('orientedDimensions leaves an untagged image alone', async () => {
+    const plain = path.join(tmpDir, 'plain.jpg');
+    await sharp({ create: { width: W, height: H, channels: 3, background: { r: 1, g: 2, b: 3 } } })
+      .jpeg().toFile(plain);
+    const m = await sharp(plain).metadata();
+    expect(imageProcessor.orientedDimensions(m)).toEqual({ width: W, height: H });
+  });
+
+  test('orientedDimensions survives metadata it cannot use', () => {
+    expect(imageProcessor.orientedDimensions(null)).toEqual({ width: null, height: null });
+    expect(imageProcessor.orientedDimensions({})).toEqual({ width: null, height: null });
+  });
+
+  test('the thumbnail comes out portrait, not sideways', async () => {
+    const rel = await imageProcessor.generateThumbnail(landscapeTaggedPortrait, {
+      outputBasename: 'orient-thumb.jpg',
+      regenerate: true,
+    });
+    expect(rel).toBeTruthy();
+
+    const out = await sharp(path.join(storageRoot, rel)).metadata();
+    // Unfixed, this came back wider than tall — the raw frame, resized.
+    expect(out.height).toBeGreaterThan(out.width);
+  });
+
+  test('the preview tier comes out portrait too', async () => {
+    const rel = await imageProcessor.generatePreviewImage(landscapeTaggedPortrait, {
+      outputBasename: 'orient-preview.jpg',
+    });
+    expect(rel).toBeTruthy();
+
+    const out = await sharp(path.join(storageRoot, rel)).metadata();
+    expect(out.height).toBeGreaterThan(out.width);
+  });
+
+  test('the watermarked rendition is oriented too', async () => {
+    // gallery.js serves photos.watermark_path ahead of the original when
+    // branding watermarking is on, so this is the rendition a guest actually
+    // sees — and it went through its own sharp pipeline that nobody had
+    // rotated.
+    const watermarkService = require('../../src/services/watermarkService');
+    const buf = await watermarkService.applyWatermark(landscapeTaggedPortrait, {
+      enabled: true, position: 'bottom-right', opacity: 50, size: 15,
+      // companyName, not text — the SVG branch reads this one, and without it
+      // the service falls through without compositing anything.
+      companyName: 'PicPeak',
+    });
+    expect(Buffer.isBuffer(buf)).toBe(true);
+
+    const out = await sharp(buf).metadata();
+    // 400x200 stored, tagged 6 — so the watermarked output must be portrait.
+    expect(out.height).toBeGreaterThan(out.width);
+  });
+
+  test('the orientation tag is gone from the output, so nothing double-rotates', async () => {
+    // The pixels are corrected now, so a surviving tag would make a viewer
+    // rotate an already-rotated image. withMetadata(false) strips it; this
+    // pins that the two changes agree.
+    const rel = await imageProcessor.generateThumbnail(landscapeTaggedPortrait, {
+      outputBasename: 'orient-thumb-tag.jpg',
+      regenerate: true,
+    });
+    const out = await sharp(path.join(storageRoot, rel)).metadata();
+    expect(out.orientation === undefined || out.orientation === 1).toBe(true);
+  });
+});

backend/__tests__/services/photoProcessor.processPhoto.test.js

@@ -72,6 +72,15 @@ jest.mock('../../src/services/imageProcessor', () => {
   return {
     generateThumbnail: mockGenerateThumbnail,
     extractCaptureDate: mockExtractCaptureDate,
+    // processPhoto routes stored dimensions through this to get the displayed
+    // ones (#1185). Mirrored rather than requireActual'd, because pulling the
+    // real module in here would drag its database dependency into the mock
+    // factory. Kept faithful to imageProcessor.orientedDimensions.
+    orientedDimensions: jest.fn((m) => {
+      if (!m || !m.width || !m.height) return { width: null, height: null };
+      const swap = m.orientation >= 5 && m.orientation <= 8;
+      return { width: swap ? m.height : m.width, height: swap ? m.width : m.height };
+    }),
     withLocalCopy: jest.fn(async (key, fn) =>
       fn(`/tmp/local-copy-${require('path').basename(key)}`)
     ),

backend/src/routes/adminExternalMedia.js

@@ -156,8 +156,8 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
         let height = null;
         try {
           const metadata = await sharp(f.full).metadata();
-          width = metadata.width || null;
-          height = metadata.height || null;
+          // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+          ({ width, height } = require('../services/imageProcessor').orientedDimensions(metadata));
         } catch (dimErr) {
           logger.warn(`Could not extract dimensions for ${f.rel}: ${dimErr.message}`);
         }

backend/src/routes/adminPhotoDimensions.js

@@ -163,13 +163,15 @@ router.post('/repair-dimensions', adminAuth, requirePermission('photos.edit'), a
             }
 
             const metadata = await sharp(fullPath).metadata();
+            // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+            const dims = require('../services/imageProcessor').orientedDimensions(metadata);
 
-            if (metadata.width && metadata.height) {
+            if (dims.width && dims.height) {
               await db('photos')
                 .where({ id: photo.id })
                 .update({
-                  width: metadata.width,
-                  height: metadata.height
+                  width: dims.width,
+                  height: dims.height
                 });
               successCount++;
 

backend/src/routes/v1/events.js

@@ -609,8 +609,8 @@ router.post(
       let height = null;
       try {
         const meta = await sharp(tempPath).metadata();
-        width = meta.width || null;
-        height = meta.height || null;
+        // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+        ({ width, height } = require('../../services/imageProcessor').orientedDimensions(meta));
       } catch { /* non-fatal */ }
 
       let thumbRel = null;

backend/src/services/fileWatcher.js

@@ -101,8 +101,10 @@ async function processNewPhoto(filePath) {
   if (!isVideo) {
     try {
       const metadata = await sharp(filePath).metadata();
-      if (metadata.width && metadata.height) {
-        dimensions = { width: metadata.width, height: metadata.height };
+      // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+      const dims = require('./imageProcessor').orientedDimensions(metadata);
+      if (dims.width && dims.height) {
+        dimensions = { width: dims.width, height: dims.height };
       }
     } catch (err) {
       logger.debug(`Could not read image dimensions for ${filename}: ${err.message}`);

backend/src/services/imageProcessor.js

@@ -123,6 +123,31 @@ const contentTypeFor = (format) => {
  * import path passes a per-photo unique basename so two events both
  * referencing `IMG_0001.jpg` don't clobber each other's thumbnail.
  */
+/**
+ * The dimensions a viewer actually sees, given EXIF orientation (#1185).
+ *
+ * sharp reports `metadata.width`/`height` as the pixels are stored, not as
+ * they are displayed. Orientation values 5-8 carry a 90° rotation, so for
+ * those the two are swapped — which is why a portrait photo from a body that
+ * tags rather than rotates was landing in the database as landscape, and why
+ * masonry and justified layouts sized its tile with the wrong aspect ratio on
+ * top of the image itself being unrotated.
+ *
+ * Everything that renders these photos now applies `.rotate()`, so the stored
+ * numbers have to describe the rotated result to match.
+ *
+ * @param {Object} metadata - a sharp metadata object
+ * @returns {{ width: number|null, height: number|null }}
+ */
+function orientedDimensions(metadata) {
+  if (!metadata || !metadata.width || !metadata.height) return { width: null, height: null };
+  const swap = metadata.orientation >= 5 && metadata.orientation <= 8;
+  return {
+    width: swap ? metadata.height : metadata.width,
+    height: swap ? metadata.width : metadata.height,
+  };
+}
+
 async function generateThumbnail(imagePath, options = {}) {
   const sourceBasename = path.basename(imagePath);
   const outputBasename = options.outputBasename || sourceBasename;
@@ -160,6 +185,18 @@ async function generateThumbnail(imagePath, options = {}) {
       failOn: 'none'
     });
 
+
+    // Apply EXIF orientation before resizing (#1185). Without this a photo
+    // whose Orientation tag is not 1 — routine for portrait shots on bodies
+    // that tag rather than rotate the sensor data — is resized from the raw
+    // pixels and comes out sideways. `.withMetadata(false)` below then strips
+    // the tag, so the browser has no hint left to correct it either.
+    //
+    // Unconditional: this pipeline never passes `animated: true`, so it
+    // already flattens a multi-frame source. Guarding on `pages` would protect
+    // an animation that was being discarded anyway while leaving the output in
+    // raw orientation against corrected dimensions.
+    sharpInstance = sharpInstance.rotate();
     // Strip EXIF/metadata from thumbnails (privacy: prevent GPS leak etc.)
     sharpInstance = sharpInstance.withMetadata(false);
 
@@ -406,6 +443,10 @@ async function generateHeroImage(imagePath, options = {}) {
       failOn: 'none'
     });
 
+
+    // EXIF orientation, same reasoning as generateThumbnail (#1185) — and
+    // unconditional for the same reason: no `animated: true` on the input.
+    sharpInstance = sharpInstance.rotate();
     // Strip EXIF/metadata from hero images (privacy: prevent GPS leak etc.)
     sharpInstance = sharpInstance.withMetadata(false);
 
@@ -619,6 +660,19 @@ async function generatePreviewImage(imagePath, options = {}) {
       animated: isAnimated,
     });
 
+
+    // EXIF orientation (#1185). Guarded here and not in the thumbnail/hero
+    // generators because this one DOES open multi-frame sources with
+    // `animated: true` above, and `.rotate()` would flatten them to a single
+    // frame — trading an animation for an orientation.
+    //
+    // Which leaves one corner unsolved: a multi-frame source that also carries
+    // an orientation tag keeps its raw orientation here while the thumbnail
+    // and the stored dimensions describe the rotated one. GIF has no EXIF at
+    // all and animated WebP effectively never sets it.
+    if (!isAnimated) {
+      sharpInstance = sharpInstance.rotate();
+    }
     // Strip EXIF — same privacy reasoning as thumbnails/heroes.
     sharpInstance = sharpInstance.withMetadata(false);
 
@@ -797,6 +851,7 @@ async function extractCaptureDate(imagePath) {
 }
 
 module.exports = {
+  orientedDimensions,
   generateThumbnail,
   isThumbnailValid,
   ensureThumbnail,

backend/src/services/photoProcessor.js

@@ -149,11 +149,10 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ
         try {
           const sharp = require('sharp');
           const metadata = await sharp(tempPath).metadata();
-          if (metadata.width && metadata.height) {
-            imageMetadata = {
-              width: metadata.width,
-              height: metadata.height
-            };
+          // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+          const dims = require('./imageProcessor').orientedDimensions(metadata);
+          if (dims.width && dims.height) {
+            imageMetadata = { width: dims.width, height: dims.height };
           }
         } catch (metadataError) {
           logger.warn(`Could not extract image dimensions for ${file.originalname}:`, metadataError.message);
@@ -468,9 +467,11 @@ async function processPhoto(photoId) {
       try {
         const sharp = require('sharp');
         const metadata = await sharp(localPath).metadata();
-        if (metadata.width && metadata.height) {
-          updateData.width = metadata.width;
-          updateData.height = metadata.height;
+        // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+        const dims = require('./imageProcessor').orientedDimensions(metadata);
+        if (dims.width && dims.height) {
+          updateData.width = dims.width;
+          updateData.height = dims.height;
         }
       } catch (e) {
         logger.warn(`processPhoto: dimensions extraction failed for ${photoId}`, { error: e.message });

backend/src/services/photoReplacementService.js

@@ -65,8 +65,8 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename,
     let height = null;
     try {
       const metadata = await sharp(newFileTempPath).metadata();
-      width = metadata.width || null;
-      height = metadata.height || null;
+      // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+      ({ width, height } = require('./imageProcessor').orientedDimensions(metadata));
     } catch {
       // Non-image or corrupt
     }

backend/src/services/s3AutoImporter.js

@@ -110,8 +110,10 @@ async function processEvent(event, storage) {
         try {
           dimensions = await withLocalCopy(entry.key, async (localPath) => {
             const metadata = await sharp(localPath).metadata();
-            if (metadata.width && metadata.height) {
-              return { width: metadata.width, height: metadata.height };
+            // Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
+            const dims = require('./imageProcessor').orientedDimensions(metadata);
+            if (dims.width && dims.height) {
+              return { width: dims.width, height: dims.height };
             }
             return null;
           });

backend/src/services/watermarkService.js

@@ -1,4 +1,5 @@
 const sharp = require('sharp');
+const { orientedDimensions } = require('./imageProcessor');
 const path = require('path');
 const fs = require('fs').promises;
 const { db } = require('../database/db');
@@ -105,8 +106,21 @@ class WatermarkService {
       }
 
       // Load the main image
-      const image = sharp(imagePath);
-      const metadata = await image.metadata();
+      // .rotate() for the same reason as the other generators (#1185): sharp
+      // decodes pixels as stored, so an orientation-tagged photo would be
+      // composited and re-encoded sideways — and gallery.js serves
+      // watermark_path ahead of the original, so this is exactly what a guest
+      // sees on a watermarked gallery.
+      const image = sharp(imagePath).rotate();
+
+      // Deliberately NOT `await image.metadata()`: .rotate() does not change
+      // what metadata() reports — a 400x200 source tagged orientation 6 still
+      // reads 400x200 there, even though the pipeline now emits 200x400. Every
+      // use below is positioning, so it has to be the DISPLAYED size or the
+      // mark lands against the wrong axis.
+      const rawMetadata = await sharp(imagePath).metadata();
+      const oriented = orientedDimensions(rawMetadata);
+      const metadata = { ...rawMetadata, width: oriented.width, height: oriented.height };
 
       let watermarkBuffer;
       let watermarkMetadata;
@@ -180,10 +194,15 @@ class WatermarkService {
       );
 
       // Apply watermark with high quality output to preserve original image quality
+      // Floored: getPositionCoordinates derives from the SVG's estimated text
+      // extent, which is fractional, and sharp rejects a non-integer offset
+      // outright — applyWatermark then catches its own error and silently
+      // returns the unwatermarked original. Whether it landed on a whole pixel
+      // was previously luck.
       let watermarkedImage = image.composite([{
         input: watermarkBuffer,
-        top: position.top,
-        left: position.left
+        top: Math.max(0, Math.floor(position.top)),
+        left: Math.max(0, Math.floor(position.left))
       }]);
 
       // Preserve original format with high quality settings