diff --git a/backend/src/imageProcessing.ts b/backend/src/imageProcessing.ts index df593c0..f96dbab 100644 --- a/backend/src/imageProcessing.ts +++ b/backend/src/imageProcessing.ts @@ -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 = { 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 { + 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; + } +} diff --git a/backend/tests/unit/imageRotation.test.ts b/backend/tests/unit/imageRotation.test.ts new file mode 100644 index 0000000..a42c0ce --- /dev/null +++ b/backend/tests/unit/imageRotation.test.ts @@ -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 { + 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 { + 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 { + 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); + }); +});