From adaee001c591adb1854210c424adac3c9631f944 Mon Sep 17 00:00:00 2001 From: thisismana Date: Thu, 30 Jul 2026 16:14:35 +0200 Subject: [PATCH] fix: bake EXIF orientation into pixels for all outputs Auto-converted images (esp. WebP) kept the source's EXIF Orientation tag instead of physically rotating the pixels. WebKit honors that tag, Blink ignores it, so rotated phone photos rendered sideways in Chrome but upright in Safari. The old pipeline never auto-oriented and actively re-injected the stale tag via `.withMetadata({ orientation })`. Call `.autoOrient()` on the pipeline before any resize/crop so the pixels are rotated once and the tag is normalized, consistently across WebP/JPEG/AVIF/PNG. `autoOrient()` wins over `keepMetadata()`, so no stale Orientation is written back. While here, rebase metadata handling onto the modern sharp API to match upstream (keepIccProfile/keepMetadata/withExif/withIccProfile), replacing the `withMetadata()/rotate:null` pattern. Behavior changes: - `strip_exif` and `strip_icc` are now independent: `strip_exif` keeps the ICC profile, `strip_icc` keeps EXIF (previously both dropped all metadata via the `rotate:null` hack). - Default outputs no longer carry sharp's injected sRGB ICC profile; sRGB is the assumed default, so this is visually identical and slightly smaller. - ratio resize now swaps width/height for 90/270 orientations, since `metadata()` reports pre-autoOrient dimensions. Co-Authored-By: Claude Opus 4.8 (1M context) --- source/image-handler/src/image-handler.ts | 27 +++++--- source/image-handler/src/thumbor-mapper.ts | 7 +- .../test/image-handler/rotate.spec.ts | 67 ++++++++++++++++--- .../test/image-handler/standard.spec.ts | 41 ++++++------ source/image-handler/test/index.spec.ts | 2 +- .../test/thumbor-mapper/filter.spec.ts | 4 +- 6 files changed, 104 insertions(+), 44 deletions(-) diff --git a/source/image-handler/src/image-handler.ts b/source/image-handler/src/image-handler.ts index c6ee0f88d..1ab8a09b6 100644 --- a/source/image-handler/src/image-handler.ts +++ b/source/image-handler/src/image-handler.ts @@ -35,15 +35,20 @@ export class ImageHandler { edits: ImageEdits, options: SharpOptions, ): Promise { - let image: sharp.Sharp; + // Bake the EXIF Orientation into the pixels and clear the tag, so output looks upright in + // every engine (Blink ignores WebP EXIF orientation; WebKit honors it). No-op when orientation + // is 1/absent. Must run before resize/crop so those act on oriented dimensions. autoOrient() + // wins over keepMetadata(), so the stale orientation tag is not re-attached. + const image = sharp(originalImage, options).autoOrient().keepIccProfile().keepMetadata(); + + if (edits?.stripExif) { + // Replace EXIF with a minimal tag; leave the ICC profile untouched. + image.keepIccProfile().withExif({ IFD0: { Software: 'ströer image-handler' } }); + } - if (edits && edits.rotate !== undefined && edits.rotate === null) { - image = sharp(originalImage, options); - } else { - const metadata = await sharp(originalImage, options).metadata(); - image = metadata.orientation - ? sharp(originalImage, options).withMetadata({ orientation: metadata.orientation }) - : sharp(originalImage, options).withMetadata(); + if (edits?.stripIcc) { + // Normalize the color profile to sRGB (drops the embedded ICC); leave EXIF untouched. + image.keepExif().withIccProfile('srgb'); } return image; @@ -208,7 +213,11 @@ export class ImageHandler { if (resize.ratio) { const ratio = resize.ratio; - const { width, height } = resize.width && resize.height ? resize : await originalImage.metadata(); + const meta = resize.width && resize.height ? resize : await originalImage.metadata(); + // metadata() reports pre-autoOrient dims, so swap for 90/270 rotations (orientation 5-8). + const swap = typeof meta.orientation === 'number' && meta.orientation >= 5; + const width = swap ? meta.height : meta.width; + const height = swap ? meta.width : meta.height; resize.width = Math.round(width * ratio); resize.height = Math.round(height * ratio); diff --git a/source/image-handler/src/thumbor-mapper.ts b/source/image-handler/src/thumbor-mapper.ts index 13344f4ee..d41902014 100644 --- a/source/image-handler/src/thumbor-mapper.ts +++ b/source/image-handler/src/thumbor-mapper.ts @@ -356,9 +356,12 @@ export class ThumborMapper { this.mapStretch(currentEdits); break; } - case 'strip_exif': + case 'strip_exif': { + currentEdits.stripExif = true; + break; + } case 'strip_icc': { - currentEdits.rotate = null; + currentEdits.stripIcc = true; break; } case 'upscale': { diff --git a/source/image-handler/test/image-handler/rotate.spec.ts b/source/image-handler/test/image-handler/rotate.spec.ts index 2e9108221..f8b9f9580 100644 --- a/source/image-handler/test/image-handler/rotate.spec.ts +++ b/source/image-handler/test/image-handler/rotate.spec.ts @@ -10,14 +10,35 @@ import { ImageHandler } from '../../src/image-handler'; const s3Client = new S3(); describe('rotate', () => { - it('Should pass if rotate is null and return image without EXIF and ICC', async () => { + it('Should keep the ICC profile when stripExif is set (and normalize orientation)', async () => { + // Arrange + const originalImage = fs.readFileSync('./test/image/1x1.jpg'); // has EXIF + ICC + orientation 3 + const request: ImageRequestInfo = { + requestType: RequestTypes.DEFAULT, + bucket: 'sample-bucket', + key: 'test.jpg', + edits: { stripExif: true }, + originalImage: originalImage, + }; + + // Act + const imageHandler = new ImageHandler(s3Client); + const result = await imageHandler.process(request); + + // Assert: EXIF stripping keeps ICC untouched; orientation baked in and cleared + const metadata = await sharp(Buffer.from(result, 'base64')).metadata(); + expect(metadata).toHaveProperty('icc'); + expect(metadata.orientation ?? 1).toBe(1); + }); + + it('Should keep EXIF when stripIcc is set (and normalize orientation)', async () => { // Arrange const originalImage = fs.readFileSync('./test/image/1x1.jpg'); const request: ImageRequestInfo = { requestType: RequestTypes.DEFAULT, bucket: 'sample-bucket', key: 'test.jpg', - edits: { rotate: null }, + edits: { stripIcc: true }, originalImage: originalImage, }; @@ -25,14 +46,13 @@ describe('rotate', () => { const imageHandler = new ImageHandler(s3Client); const result = await imageHandler.process(request); - // Assert + // Assert: ICC stripping keeps EXIF untouched; orientation baked in and cleared const metadata = await sharp(Buffer.from(result, 'base64')).metadata(); - expect(metadata).not.toHaveProperty('exif'); - expect(metadata).not.toHaveProperty('icc'); - expect(metadata).not.toHaveProperty('orientation'); + expect(metadata).toHaveProperty('exif'); + expect(metadata.orientation ?? 1).toBe(1); }); - it('Should pass if the original image has orientation', async () => { + it('Should bake EXIF orientation into pixels and normalize the tag', async () => { // Arrange const originalImage = fs.readFileSync('./test/image/1x1.jpg'); const request: ImageRequestInfo = { @@ -47,9 +67,38 @@ describe('rotate', () => { const imageHandler = new ImageHandler(s3Client); const result = await imageHandler.process(request); - // Assert + // Assert: orientation is auto-applied and cleared (was 3), not re-injected as a tag + const metadata = await sharp(Buffer.from(result, 'base64')).metadata(); + expect(metadata.orientation ?? 1).toBe(1); + }); + + it('Should physically rotate pixels so output dims match the oriented source (all engines)', async () => { + // Arrange: 1366x768 landscape pixels + EXIF Orientation=6 (rotate 90 CW) = a portrait photo + const landscape = await sharp({ + create: { width: 1366, height: 768, channels: 3, background: { r: 200, g: 100, b: 50 } }, + }) + .jpeg() + .toBuffer(); + const originalImage = await sharp(landscape).withMetadata({ orientation: 6 }).jpeg().toBuffer(); + + const request: ImageRequestInfo = { + requestType: RequestTypes.DEFAULT, + bucket: 'sample-bucket', + key: 'test.jpg', + contentType: 'image/webp', + edits: {}, + originalImage, + }; + + // Act + const imageHandler = new ImageHandler(s3Client); + const result = await imageHandler.process(request); + + // Assert: pixels are physically rotated to portrait, and no stale orientation tag remains const metadata = await sharp(Buffer.from(result, 'base64')).metadata(); - expect(metadata.orientation).toEqual(3); + expect(metadata.width).toBe(768); + expect(metadata.height).toBe(1366); + expect(metadata.orientation ?? 1).toBe(1); }); it('Should pass if the original image does not have orientation', async () => { diff --git a/source/image-handler/test/image-handler/standard.spec.ts b/source/image-handler/test/image-handler/standard.spec.ts index e010a98ad..75e823bfd 100644 --- a/source/image-handler/test/image-handler/standard.spec.ts +++ b/source/image-handler/test/image-handler/standard.spec.ts @@ -9,7 +9,10 @@ import { ImageHandler } from '../../src/image-handler'; const s3Client = new S3(); const image = fs.readFileSync('./test/image/25x15.png'); -const withMetatdataSpy = jest.spyOn(sharp.prototype, 'withMetadata'); +const autoOrientSpy = jest.spyOn(sharp.prototype, 'autoOrient'); +const keepMetadataSpy = jest.spyOn(sharp.prototype, 'keepMetadata'); +const withExifSpy = jest.spyOn(sharp.prototype, 'withExif'); +const withIccProfileSpy = jest.spyOn(sharp.prototype, 'withIccProfile'); describe('standard', () => { it('Should pass if a series of standard edits are provided to the function', async () => { @@ -39,50 +42,46 @@ describe('instantiateSharpImage', () => { jest.clearAllMocks(); }); - it('Should not include metadata if the rotation is null', async () => { + it('Should auto-orient and keep metadata by default, without stripping EXIF or ICC', async () => { // Arrange - const edits = { - rotate: null, - }; const options: SharpOptions = { failOn: 'none' }; const imageHandler = new ImageHandler(s3Client); // Act - await imageHandler['instantiateSharpImage'](image, edits, options); + await imageHandler['instantiateSharpImage'](image, {}, options); //Assert - expect(withMetatdataSpy).not.toHaveBeenCalled(); + expect(autoOrientSpy).toHaveBeenCalled(); + expect(keepMetadataSpy).toHaveBeenCalled(); + expect(withExifSpy).not.toHaveBeenCalled(); + expect(withIccProfileSpy).not.toHaveBeenCalled(); }); - it('Should include metadata and not define orientation if the rotation is not null and orientation is not defined', async () => { + it('Should strip EXIF but keep the ICC profile when stripExif is set', async () => { // Arrange - const edits = { - rotate: undefined, - }; const options: SharpOptions = { failOn: 'none' }; const imageHandler = new ImageHandler(s3Client); // Act - await imageHandler['instantiateSharpImage'](image, edits, options); + await imageHandler['instantiateSharpImage'](image, { stripExif: true }, options); //Assert - expect(withMetatdataSpy).toHaveBeenCalled(); - expect(withMetatdataSpy).not.toHaveBeenCalledWith(expect.objectContaining({ orientation: expect.anything })); + expect(autoOrientSpy).toHaveBeenCalled(); + expect(withExifSpy).toHaveBeenCalled(); + expect(withIccProfileSpy).not.toHaveBeenCalled(); }); - it('Should include orientation metadata if the rotation is defined in the metadata', async () => { + it('Should strip the ICC profile but keep EXIF when stripIcc is set', async () => { // Arrange - const edits = { - rotate: undefined, - }; const options: SharpOptions = { failOn: 'none' }; - const modifiedImage = await sharp(image).withMetadata({ orientation: 1 }).toBuffer(); const imageHandler = new ImageHandler(s3Client); // Act - await imageHandler['instantiateSharpImage'](modifiedImage, edits, options); + await imageHandler['instantiateSharpImage'](image, { stripIcc: true }, options); //Assert - expect(withMetatdataSpy).toHaveBeenCalledWith({ orientation: 1 }); + expect(autoOrientSpy).toHaveBeenCalled(); + expect(withIccProfileSpy).toHaveBeenCalledWith('srgb'); + expect(withExifSpy).not.toHaveBeenCalled(); }); }); diff --git a/source/image-handler/test/index.spec.ts b/source/image-handler/test/index.spec.ts index 2b5a1c963..d505ec1fc 100644 --- a/source/image-handler/test/index.spec.ts +++ b/source/image-handler/test/index.spec.ts @@ -39,7 +39,7 @@ describe('index', () => { Vary: 'Accept', 'Last-Modified': undefined, }, - body: '/9j/4QC8RXhpZgAASUkqAAgAAAAGABIBAwABAAAAAQAAABoBBQABAAAAVgAAABsBBQABAAAAXgAAACgBAwABAAAAAgAAABMCAwABAAAAAQAAAGmHBAABAAAAZgAAAAAAAABIAAAAAQAAAEgAAAABAAAABgAAkAcABAAAADAyMTABkQcABAAAAAECAwAAoAcABAAAADAxMDABoAMAAQAAAP//AAACoAQAAQAAAAEAAAADoAQAAQAAAAEAAAAAAAAA/+IB8ElDQ19QUk9GSUxFAAEBAAAB4GxjbXMEIAAAbW50clJHQiBYWVogB+IAAwAUAAkADgAdYWNzcE1TRlQAAAAAc2F3c2N0cmwAAAAAAAAAAAAAAAAAAPbWAAEAAAAA0y1oYW5keem/Vlo+AbaDI4VVRvdPqgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAKZGVzYwAAAPwAAAAkY3BydAAAASAAAAAid3RwdAAAAUQAAAAUY2hhZAAAAVgAAAAsclhZWgAAAYQAAAAUZ1hZWgAAAZgAAAAUYlhZWgAAAawAAAAUclRSQwAAAcAAAAAgZ1RSQwAAAcAAAAAgYlRSQwAAAcAAAAAgbWx1YwAAAAAAAAABAAAADGVuVVMAAAAIAAAAHABzAFIARwBCbWx1YwAAAAAAAAABAAAADGVuVVMAAAAGAAAAHABDAEMAMAAAWFlaIAAAAAAAAPbWAAEAAAAA0y1zZjMyAAAAAAABDD8AAAXd///zJgAAB5AAAP2S///7of///aIAAAPcAADAcVhZWiAAAAAAAABvoAAAOPIAAAOPWFlaIAAAAAAAAGKWAAC3iQAAGNpYWVogAAAAAAAAJKAAAA+FAAC2xHBhcmEAAAAAAAMAAAACZmkAAPKnAAANWQAAE9AAAApb/9sAQwAGBgYGBwYHCAgHCgsKCwoPDgwMDg8WEBEQERAWIhUZFRUZFSIeJB4cHiQeNiomJio2PjQyND5MRERMX1pffHyn/9sAQwEGBgYGBwYHCAgHCgsKCwoPDgwMDg8WEBEQERAWIhUZFRUZFSIeJB4cHiQeNiomJio2PjQyND5MRERMX1pffHyn/8IAEQgAAQABAwEiAAIRAQMRAf/EABUAAQEAAAAAAAAAAAAAAAAAAAAH/8QAFQEBAQAAAAAAAAAAAAAAAAAABQf/2gAMAwEAAhADEAAAAIOA6p//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAE/AH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAECAQE/AH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/AH//2Q==', + body: '/9j/4QC8RXhpZgAASUkqAAgAAAAGABIBAwABAAAAAQAAABoBBQABAAAAVgAAABsBBQABAAAAXgAAACgBAwABAAAAAgAAABMCAwABAAAAAQAAAGmHBAABAAAAZgAAAAAAAABIAAAAAQAAAEgAAAABAAAABgAAkAcABAAAADAyMTABkQcABAAAAAECAwAAoAcABAAAADAxMDABoAMAAQAAAP//AAACoAQAAQAAAAEAAAADoAQAAQAAAAEAAAAAAAAA/9sAQwAGBgYGBwYHCAgHCgsKCwoPDgwMDg8WEBEQERAWIhUZFRUZFSIeJB4cHiQeNiomJio2PjQyND5MRERMX1pffHyn/9sAQwEGBgYGBwYHCAgHCgsKCwoPDgwMDg8WEBEQERAWIhUZFRUZFSIeJB4cHiQeNiomJio2PjQyND5MRERMX1pffHyn/8IAEQgAAQABAwEiAAIRAQMRAf/EABUAAQEAAAAAAAAAAAAAAAAAAAAH/8QAFQEBAQAAAAAAAAAAAAAAAAAABQf/2gAMAwEAAhADEAAAAIOA6p//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAE/AH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAECAQE/AH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/AH//2Q==', }; // Assert diff --git a/source/image-handler/test/thumbor-mapper/filter.spec.ts b/source/image-handler/test/thumbor-mapper/filter.spec.ts index 71da7a219..e52f815a7 100644 --- a/source/image-handler/test/thumbor-mapper/filter.spec.ts +++ b/source/image-handler/test/thumbor-mapper/filter.spec.ts @@ -430,7 +430,7 @@ describe('filter', () => { const edits = thumborMapper.mapFilter(edit, filetype); // Assert - const expectedResult = { rotate: null }; + const expectedResult = { stripExif: true }; expect(edits).toEqual(expectedResult); }); @@ -444,7 +444,7 @@ describe('filter', () => { const edits = thumborMapper.mapFilter(edit, filetype); // Assert - const expectedResult = { rotate: null }; + const expectedResult = { stripIcc: true }; expect(edits).toEqual(expectedResult); });