feat(images): turn a stored photo a quarter turn (#301)

The file half of the remedy for what #300 could only stop. Fixing the EXIF strip means new uploads arrive the way the sender saw them; it cannot repair what is already stored, because the tag that said which way up the pixels went is gone. Those photos need a person to look at each one and turn it. Rewrites the pixels rather than recording an angle, because an angle obliges every consumer to honour it — the storefront, both admin screens, the drafting worker's photo reader, and the rembg sidecar — and any one that forgets shows the photo sideways. The sidecar is not ours to teach. Left is anticlockwise and right is clockwise, which is rotate(-90) and rotate(90); sharp reads a positive angle as clockwise. The direction test asserts a pixel rather than a dimension, because dimensions swap whichever way the turn goes — a reversed sign would pass every size assertion and ship a control that does the opposite of its label. The animated WebP case is the one that could destroy someone's file quietly. Reading such a file without the animated flag succeeds and hands back the first frame alone, so a rotation that omitted it would write a still back over the animation and report success. Passing the same flag reencodeInPlace passes makes sharp refuse instead — multi-page images turn only by 180° — which is the honest answer and leaves the file untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-04 13:37:41 -05:00
parent df508b3783
commit 5c0c7186ff
2 changed files with 217 additions and 0 deletions
@@ -126,3 +126,66 @@ export async function reencodeInPlace(filePath: string, mimetype: string): Promi
    throw err;
  }
 }
+
+/** Which way a quarter turn goes, in the words the buttons use. */
+export type RotateDirection = 'left' | 'right';
+
+/**
+ * sharp reads a positive angle as clockwise, so the sign is the whole of the
+ * mapping — and getting it backwards produces a control that works perfectly
+ * and does the opposite of what its label says, which no dimension assertion
+ * would ever catch.
+ */
+const QUARTER_TURN: Record<RotateDirection, number> = { left: -90, right: 90 };
+
+/**
+ * Turns the file at `filePath` a quarter turn, in its own format.
+ *
+ * The remedy for photos uploaded before #300 taught the re-encode to apply EXIF
+ * orientation instead of discarding it. Those files cannot be repaired
+ * automatically — the tag that said which way up they went is gone — so a
+ * person has to look at each one and decide.
+ *
+ * Rewrites the pixels rather than recording an angle. An angle would keep the
+ * bytes pristine, but it would put an obligation on every consumer — the
+ * storefront, both admin screens, the drafting worker's photo reader, and the
+ * rembg sidecar — and any one that forgot would show the photo sideways. The
+ * sidecar in particular is not ours to teach.
+ *
+ * Same temporary-file-and-rename shape as `reencodeInPlace`, for the same two
+ * reasons: sharp cannot read and write one path in a single pass, and a process
+ * that dies mid-write must leave the old photo intact rather than half a new
+ * one.
+ *
+ * No `resize`. The file went through `reencodeInPlace` on upload and is already
+ * within bounds, so re-applying the cap would be a second lossy pass buying
+ * nothing. No metadata handling either: the re-encode already stripped it, and
+ * there is nothing left to strip.
+ */
+export async function rotateInPlace(
+  filePath: string,
+  mimetype: string,
+  direction: RotateDirection
+): Promise<void> {
+  const temporary = `${filePath}.rotating`;
+  try {
+    await encoderFor(
+      // Exactly the `animated` argument reencodeInPlace uses, and the reason is
+      // sharper here. Reading an animated WebP without it decodes the first
+      // frame alone, so omitting it would silently write back a still and
+      // destroy the animation while reporting success. With it, sharp refuses
+      // the rotation outright — multi-page images can only be turned 180° —
+      // which is the honest answer and surfaces as a 500 with the file
+      // untouched. A still WebP has one page and turns normally.
+      sharp(filePath, mimetype === 'image/webp' ? { animated: true } : {}).rotate(
+        QUARTER_TURN[direction]
+      ),
+      mimetype
+    ).toFile(temporary);
+
+    await fs.rename(temporary, filePath);
+  } catch (err) {
+    await fs.unlink(temporary).catch(() => undefined);
+    throw err;
+  }
+}
@@ -0,0 +1,154 @@
+import { promises as fs } from 'fs';
+import os from 'os';
+import path from 'path';
+import sharp from 'sharp';
+import { rotateInPlace } from '../../src/imageProcessing';
+
+let dir = '';
+
+beforeEach(async () => {
+  dir = await fs.mkdtemp(path.join(os.tmpdir(), 'rotate-'));
+});
+
+afterEach(async () => {
+  await fs.rm(dir, { recursive: true, force: true });
+});
+
+/**
+ * A square image, left half red and right half blue.
+ *
+ * Square on purpose. Dimensions swap whichever way a quarter turn goes, so a
+ * rectangle proves that something rotated but says nothing about which way —
+ * only the pixels can say that, and this is the smallest picture that makes
+ * the direction visible.
+ */
+async function halvesPng(): Promise<Buffer> {
+  const red = await sharp({
+    create: { width: 100, height: 200, channels: 3, background: { r: 255, g: 0, b: 0 } }
+  })
+    .png()
+    .toBuffer();
+  const blue = await sharp({
+    create: { width: 100, height: 200, channels: 3, background: { r: 0, g: 0, b: 255 } }
+  })
+    .png()
+    .toBuffer();
+  return sharp({
+    create: { width: 200, height: 200, channels: 3, background: { r: 0, g: 0, b: 0 } }
+  })
+    .composite([
+      { input: red, left: 0, top: 0 },
+      { input: blue, left: 100, top: 0 }
+    ])
+    .png()
+    .toBuffer();
+}
+
+/** The colour a little way in from the top-left corner, clear of any edge. */
+async function topLeft(file: string): Promise<string> {
+  const { data, info } = await sharp(file).raw().toBuffer({ resolveWithObject: true });
+  const at = (10 * info.width + 10) * info.channels;
+  return [data[at], data[at + 1], data[at + 2]].join(',');
+}
+
+/**
+ * A two-frame animated WebP, built rather than committed as a binary fixture so
+ * what it contains is readable here. `pageHeight` is what makes libvips treat
+ * one tall buffer as a strip of frames.
+ */
+async function animatedWebp(): Promise<Buffer> {
+  const width = 40;
+  const height = 40;
+  const raw = Buffer.alloc(width * height * 2 * 4);
+  for (let i = 0; i < width * height * 4; i += 4) {
+    raw[i] = 255;
+    raw[i + 3] = 255;
+  }
+  for (let i = width * height * 4; i < raw.length; i += 4) {
+    raw[i + 2] = 255;
+    raw[i + 3] = 255;
+  }
+  return sharp(raw, { raw: { width, height: height * 2, channels: 4, pageHeight: height } })
+    .webp()
+    .toBuffer();
+}
+
+describe('rotateInPlace', () => {
+  it('turns a landscape photo into a portrait one', async () => {
+    const file = path.join(dir, 'wide.jpg');
+    await fs.writeFile(
+      file,
+      await sharp({
+        create: { width: 400, height: 200, channels: 3, background: { r: 20, g: 60, b: 120 } }
+      })
+        .jpeg()
+        .toBuffer()
+    );
+
+    await rotateInPlace(file, 'image/jpeg', 'right');
+
+    const after = await sharp(file).metadata();
+    expect(after.width).toBe(200);
+    expect(after.height).toBe(400);
+  });
+
+  // The direction mapping, which is the one thing here that can be exactly
+  // backwards while every dimension assertion still passes. Turning clockwise
+  // moves the left edge to the top, so the red half ends up along the top.
+  it('turns right clockwise', async () => {
+    const file = path.join(dir, 'halves.png');
+    await fs.writeFile(file, await halvesPng());
+
+    await rotateInPlace(file, 'image/png', 'right');
+
+    expect(await topLeft(file)).toBe('255,0,0');
+  });
+
+  it('turns left anticlockwise', async () => {
+    const file = path.join(dir, 'halves.png');
+    await fs.writeFile(file, await halvesPng());
+
+    await rotateInPlace(file, 'image/png', 'left');
+
+    expect(await topLeft(file)).toBe('0,0,255');
+  });
+
+  // Format is preserved for the reason #226 preserved it: image_path carries
+  // the extension, so changing the format here would leave the row pointing at
+  // a file that no longer exists.
+  it('keeps the file in its own format', async () => {
+    const file = path.join(dir, 'halves.png');
+    await fs.writeFile(file, await halvesPng());
+
+    await rotateInPlace(file, 'image/png', 'left');
+
+    expect((await sharp(file).metadata()).format).toBe('png');
+  });
+
+  // The whole reason for the temp-file-and-rename: a failure must leave the
+  // photo exactly as it was, and must not leave a stray file behind either.
+  it('leaves the file untouched when sharp cannot read it', async () => {
+    const file = path.join(dir, 'broken.jpg');
+    await fs.writeFile(file, 'not an image');
+
+    await expect(rotateInPlace(file, 'image/jpeg', 'right')).rejects.toThrow();
+
+    expect(await fs.readFile(file, 'utf8')).toBe('not an image');
+    expect(await fs.readdir(dir)).toEqual(['broken.jpg']);
+  });
+
+  // The trap this guards. Reading an animated WebP without `animated: true`
+  // succeeds and yields the first frame alone, so a rotation that omitted the
+  // flag would write back a still and destroy the uploader's animation while
+  // reporting success. With the flag, sharp refuses — which is the correct
+  // answer, because a quarter turn of a multi-page image is not something it
+  // can do.
+  it('refuses an animated WebP rather than flattening it', async () => {
+    const file = path.join(dir, 'moving.webp');
+    await fs.writeFile(file, await animatedWebp());
+
+    await expect(rotateInPlace(file, 'image/webp', 'right')).rejects.toThrow(/multi-page/i);
+
+    expect((await sharp(file, { animated: true }).metadata()).pages).toBe(2);
+  });
+});