fix(uploads): apply the EXIF orientation before discarding it (#300) #302
@@ -89,7 +89,27 @@ 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({
|
||||
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',
|
||||
|
||||
@@ -42,6 +42,30 @@ async function photoWithLocation(): Promise<Buffer> {
|
||||
.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<Buffer> {
|
||||
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<string> {
|
||||
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');
|
||||
|
||||
|
||||
Reference in New Issue
Block a user