diff --git a/backend/src/imageProcessing.ts b/backend/src/imageProcessing.ts index 0314fb2..df593c0 100644 --- a/backend/src/imageProcessing.ts +++ b/backend/src/imageProcessing.ts @@ -89,12 +89,32 @@ export async function reencodeInPlace(filePath: string, mimetype: string): Promi // uploader's image while reporting success. It is not set unconditionally // because it changes how `resize` interprets height (the full frame // strip, not one frame), which would be wrong for the other two. - sharp(filePath, mimetype === 'image/webp' ? { animated: true } : {}).resize({ - width: MAX_DIMENSION, - height: MAX_DIMENSION, - fit: 'inside', - withoutEnlargement: true - }), + sharp(filePath, mimetype === 'image/webp' ? { animated: true } : {}) + // Applies the EXIF orientation to the pixels, and must come before + // resize (#300). + // + // A camera does not turn its sensor data round. It writes the pixels as + // the sensor read them and sets an Orientation tag saying which way up + // they go, and every viewer honours that — which is why a portrait + // photograph looks upright to the person who took it and to the person + // who attached it. Stripping the tag without applying it does not leave + // the photo alone: it leaves the pixels sideways with nothing left to + // explain them, and the sender's upright photo arrives on its side. + // + // Before resize because the resize bounds are width and height, and for + // a portrait photo those are the wrong way round until the rotation has + // happened. A 3000x4000 photograph stored as 4000x3000 would otherwise + // be bounded on the wrong axis. + // + // No argument: that is what makes it read the tag rather than turn the + // image by a fixed amount. + .rotate() + .resize({ + width: MAX_DIMENSION, + height: MAX_DIMENSION, + fit: 'inside', + withoutEnlargement: true + }), mimetype // No withMetadata(): omitting it is what drops EXIF, ICC and everything // else. Calling it would put the metadata back. diff --git a/backend/tests/integration/exifStripping.integration.test.ts b/backend/tests/integration/exifStripping.integration.test.ts index 6a529ed..6bbc879 100644 --- a/backend/tests/integration/exifStripping.integration.test.ts +++ b/backend/tests/integration/exifStripping.integration.test.ts @@ -42,6 +42,30 @@ async function photoWithLocation(): Promise { .toBuffer(); } +/** + * A photo the way a phone actually writes one: pixels in the sensor's own + * orientation, and an EXIF tag saying which way up to display them. + * + * Orientation 6 means "rotate 90° clockwise to show this". So these 400x200 + * landscape pixels are what a portrait photograph looks like on disk, and every + * viewer that honours the tag — the camera roll, the mail client, the browser + * the sender attached it from — shows it as 200x400. That is why the sender + * sees an upright photo and has no idea the file is sideways. + */ +async function portraitPhoto(): Promise { + return sharp({ + create: { width: 400, height: 200, channels: 3, background: { r: 20, g: 60, b: 120 } } + }) + // withMetadata rather than withExif, and the difference is not cosmetic: + // withExif({ IFD0: { Orientation: '6' } }) writes a tag that sharp reads + // back as orientation 1, so a fixture built that way carries no orientation + // at all and would let this test pass against the unfixed code. Checked + // rather than assumed — the first version of this test did exactly that. + .withMetadata({ orientation: 6 }) + .jpeg() + .toBuffer(); +} + async function storedPathFor(itemId: number): Promise { const { rows } = await pool.query<{ image_path: string }>( `SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`, @@ -108,6 +132,37 @@ describe('an uploaded photo does not keep where it was taken', () => { expect(stored.height).toBe(200); }); + /** + * The other half of stripping metadata, and the half #226 missed (#300). + * + * Dropping EXIF is right — a product photo should not publish where it was + * taken. But orientation lives in EXIF too, and deleting it without first + * applying it to the pixels does not leave the photo alone: it leaves the + * pixels sideways with nothing left to explain them. The sender sees an + * upright photo, uploads it, and finds it rotated. + */ + it('applies the orientation before throwing the tag away', async () => { + const itemId = await createItemWith(await portraitPhoto(), 'portrait.jpg'); + + const stored = await sharp(await storedPathFor(itemId)).metadata(); + + // 400x200 on disk with Orientation 6 is a 200x400 photograph. Stored + // upright, the dimensions swap. + expect(stored.width).toBe(200); + expect(stored.height).toBe(400); + }); + + // And the tag itself still goes, so nothing rotates it a second time. + it('does not leave the orientation tag behind after applying it', async () => { + const itemId = await createItemWith(await portraitPhoto(), 'portrait.jpg'); + + const stored = await sharp(await storedPathFor(itemId)).metadata(); + + expect(stored.exif).toBeUndefined(); + // sharp reports 1 — the identity — for a file with nothing to say about it. + expect(stored.orientation ?? 1).toBe(1); + }); + it('leaves no temporary re-encoding files on the volume', async () => { await createItemWith(await photoWithLocation(), 'vase.jpg');