fix(miniflare): apply EXIF orientation in local Images binding and cf.image transforms - #15031
fix(miniflare): apply EXIF orientation in local Images binding and cf.image transforms#15031vdanielb wants to merge 1 commit into
Conversation
….image transforms Production auto-orients images per their EXIF orientation flag before applying transforms, but the local sharp-based polyfill decoded the raw pixels as stored, so phone photos (stored sideways + rotation flag) came back rotated 90deg in local dev while rendering correctly when deployed. Decode with sharp's autoOrient option in both the Images binding and cf.image local fetchers, baking the EXIF rotation into the pixels before user transforms, matching production. The /info path is unchanged. Fixes cloudflare#15029 Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 1219831 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| test("EXIF orientation is applied before transforms (matches production)", async ({ | ||
| expect, | ||
| }) => { | ||
| // Production auto-orients per EXIF before transforming; without it the | ||
| // 200x100 source would pass through sideways as 200x100. | ||
| const { body } = await transform({}, "image/png", await exifRotatedJpeg()); | ||
| const meta = await sharp(body).metadata(); | ||
| expect(meta.width).toBe(100); | ||
| expect(meta.height).toBe(200); | ||
|
|
||
| // After the 90° CW rotation the source's red bottom half lands on the | ||
| // left and the white top half on the right. JPEG encoding is lossy, so | ||
| // sample deep inside each half and allow small artifacts. | ||
| const left = await pixelAt(body, 25, 100); | ||
| expect(left.r).toBeGreaterThan(240); | ||
| expect(left.g).toBeLessThan(15); | ||
| expect(left.b).toBeLessThan(15); | ||
| const right = await pixelAt(body, 75, 100); | ||
| expect(right.r).toBeGreaterThan(240); | ||
| expect(right.g).toBeGreaterThan(240); | ||
| expect(right.b).toBeGreaterThan(240); | ||
| }); |
There was a problem hiding this comment.
📝 Info: No regression test added for the cf.image path
The PR changes both imagesLocalFetcher and cfImageLocalFetcher, but the added regression tests only exercise the env.IMAGES binding path in packages/miniflare/test/plugins/images/transform.spec.ts. The cf.image auto-orient behaviour (including the format: "json" branch) is untested; test/plugins/core/cf-image.spec.ts would be the natural home for an equivalent EXIF orientation case.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async function exifRotatedJpeg() { | ||
| return sharp(sourcePng) | ||
| .jpeg({ quality: 95 }) | ||
| .withMetadata({ orientation: 6 }) | ||
| .toBuffer(); | ||
| } |
There was a problem hiding this comment.
📝 Info: Test fixture relies on withMetadata's orientation option
withMetadata({ orientation: 6 }) is the older sharp API (superseded by keepMetadata()/withExif()/withMetadata variants in recent versions). It still works in sharp 0.35.x but is a deprecation candidate; if it ever becomes a no-op the tests would silently pass against the un-fixed code path since a JPEG without the orientation tag decodes identically with and without autoOrient. Asserting the fixture actually carries orientation: 6 (via sharp(buf).metadata()) would make the regression tests self-verifying.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| if (options.format === "json") { | ||
| const jsonTransformer = sharp(source); | ||
| const jsonTransformer = sharp(source, { autoOrient: true }); |
There was a problem hiding this comment.
🔍 cf.image format=json reports stored (non-oriented) original dimensions alongside oriented output dimensions
The format: "json" response mixes two coordinate systems: width/height come from the auto-oriented transformer, but original.width/original.height come from sharp(source).metadata() at packages/miniflare/src/plugins/images/fetcher.ts:397, which is not auto-oriented. For an EXIF orientation-6 photo this yields e.g. {width: 100, height: 200, original: {width: 200, height: 100}}. Production's cf.image JSON almost certainly reports the display-oriented original dimensions; if so this is a remaining fidelity gap in the same area this PR is fixing.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| if (url.pathname == "/info") { | ||
| return runInfo(transformer); | ||
| return runInfo(sharp(source, {})); |
There was a problem hiding this comment.
🔍 Transform output dimensions now disagree with info() for EXIF-rotated images
/info intentionally keeps the non-auto-oriented sharp(source, {}) (packages/miniflare/src/plugins/images/fetcher.ts:69) while the transform path now decodes with autoOrient: true. For a JPEG with EXIF orientation 6, env.IMAGES.info() will report 200x100 while .transform({}).output(...) now yields 100x200. Worth confirming against production: if the production info endpoint reports the display (EXIF-applied) dimensions, local dev will now be inconsistent with production in the opposite direction from the bug this PR fixes. No test covers /info with an EXIF-tagged image.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I've manually tested this and I can confirm that in production env.IMAGES.info() differ between production and the local simulation
in production the original dimensions are shown while locally those after the EXIF
@vdanielb could you address fix and also add test to cover /info? 🙏
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
3 participants
Fixes #15029
What this PR solves / how to test
The local (low-fidelity) Images binding polyfill decoded images as stored, without applying their EXIF orientation flag. Production auto-orients before transforming, so phone photos (stored as landscape pixels + EXIF
Orientation: 6) rendered sideways inwrangler devbut upright when deployed. Downstream, this made Astro's@astrojs/cloudflareadapter show EXIF-rotated photos sideways inastro dev.This PR decodes with sharp's
autoOrientinput option in both local fetchers:imagesLocalFetcher(env.IMAGES.input(...).transform(...))cfImageLocalFetcher(fetch(url, { cf: { image } })), including theformat: "json"pathautoOrientbakes the EXIF rotation into the pixels at decode and clears the flag; an explicit userrotatestill composes on top. The/infopath is intentionally unchanged (it keeps reporting the stored dimensions from metadata).To test: run any Worker with an
imagesbinding and transform a JPEG that carries EXIFOrientation: 6(e.g. an iPhone portrait photo). Before this change the local output is sideways; after, it is upright and matcheswrangler dev --remote/ production.Includes regression tests in
transform.spec.tsthat tag the existing fixture with EXIF orientation 6 and assert the output is upright (dimensions swapped, rotated pixel layout) and that resize applies to the upright image.Author has addressed the following
packages/miniflare/test/plugins/images/transform.spec.ts; rantest/plugins/images/andtest/plugins/core/cf-image.spec.tslocally (34 passed)