A photo tagged to be turned is stored turned (#1079) #1086

Merged
claude-bot merged 1 commit from fix/1079-exif-orientation into main 2026-10-04 21:25:58 +00:00
Collaborator

Closes #1079.

Summary

The re-encode stripped a picture's EXIF without applying its orientation first, so a phone photo held as landscape pixels plus a "rotate" tag was stored on its side. Reproduced first: a 400×300 JPEG with Orientation 6 came out 400×300.

  • The orientation is applied before the re-encode, for all eight values, in JPEG, PNG and WebP. Metadata is still stripped.
  • The tag is read by a small bounded parser from the EXIF block's first directory: at most 512 entries, never the data an entry points to. An invalid value is ignored. Pillow's getexif is not called (see "Found on the way").
  • The turn costs no extra memory. The picture is turned in strips through a spooled temp file, and the original is closed before the turned one is built.
  • The header check reports upright sizes, so width and height swap for orientations 5 to 8.
  • Pictures already stored are not touched. Their tag was stripped at upload, so the orientation is gone; a sideways one needs uploading again. The changelog says so.
  • No migration, bot, compose or frontend change.

One change to every still picture, tagged or not

_reencode now collects its output in pieces and joins them after the image is closed, in place of io.BytesIO. Output bytes are identical (a test compares against a direct Pillow save). Without it the turned worst file cost +74 MB in a fresh process, because BytesIO is copied as it grows.

Measured

docker run -m 512m, the real validator, peak over the state before the call. All files tagged Orientation 6.

File Not turned (main) transpose, original closed exif_transpose(in_place=True) This branch Same file untagged, this branch
4032×3024 photo JPEG +53 MB +93 MB +93 MB +51 MB +51 MB
4080×3072 noise JPEG (worst still) +63 MB +96 MB +96 MB +58 MB +58 MB
4080×3072 RGBA PNG +53 MB +96 MB +96 MB +52 MB +52 MB
Map 8192×6144 JPEG, in 1 GB +210 MB +384 MB not run +209 MB +209 MB

The spill file is page cache, not process memory: up to 38 MB in the backend and 151 MB in the worker. Under pressure it is reclaimed: the photo passed in a 113 MiB container with oom_kill 0, where transpose was killed in 130 MiB.

Through nginx on a warm backend (512 MB, two processes):

  • A 4032×3024 Orientation 6 JPEG with GPS: stored and fetched back as 3024×4032, upright, with no EXIF or GPS. Backend cgroup peak 489 MiB, oom_kill 0.
  • The worst file at the bound with the same tag: stored 3072×4080, peak 496 MiB, oom_kill 0.
  • Those peaks include the spill's page cache; anonymous memory stayed at 382 to 390 MiB.

Limits

  • Only the EXIF orientation is honoured. An XMP orientation, or a PNG's "Raw profile" text chunk, is ignored, as browsers do.
  • A PNG whose eXIf chunk follows its pixel data is turned on store, but the header check reports its unturned size. Stored dimensions are right.
  • Not measured: a CMYK picture with a rotating tag.

Found on the way, not fixed here

On main today, Image.open parses a JPEG's EXIF and reads the data of every directory entry. A 6.3 MB JPEG with 60 entries each claiming the same 6 MB took Image.open alone to 429 MB. That runs before the pixel check and outside the lock. It gets its own issue.

Changelog

  • changelog.d/1079-exif-orientation.fixed.md

Test plan

  • New tests/wiki/test_exif_orientation.py, 222 tests. Against main's image_service.py: 61 failed, 50 passed, 96 errors, the reproduction among the failures.
  • The new file with the pixel-bound and damaged-image tests, at the head commit: 308 passed.
  • Full backend suite: 7627 passed, 14 skipped, 107 deselected. It ran before the last test-only edits; image_service.py did not change after it.
  • A map uploaded as 300×600 pixels with Orientation 6 is stored and reported as 600×300, with tiles 3 by 2.
  • Lint clean; version and changelog checks OK.
  • Not run: frontend, bot, synthetic, journeys (untouched); CI.

🤖 Generated with Claude Code

Closes #1079. ## Summary The re-encode stripped a picture's EXIF without applying its orientation first, so a phone photo held as landscape pixels plus a "rotate" tag was stored on its side. Reproduced first: a 400×300 JPEG with Orientation 6 came out 400×300. - **The orientation is applied before the re-encode**, for all eight values, in JPEG, PNG and WebP. Metadata is still stripped. - **The tag is read by a small bounded parser** from the EXIF block's first directory: at most 512 entries, never the data an entry points to. An invalid value is ignored. Pillow's `getexif` is not called (see "Found on the way"). - **The turn costs no extra memory.** The picture is turned in strips through a spooled temp file, and the original is closed before the turned one is built. - **The header check reports upright sizes**, so width and height swap for orientations 5 to 8. - **Pictures already stored are not touched.** Their tag was stripped at upload, so the orientation is gone; a sideways one needs uploading again. The changelog says so. - No migration, bot, compose or frontend change. ## One change to every still picture, tagged or not `_reencode` now collects its output in pieces and joins them after the image is closed, in place of `io.BytesIO`. Output bytes are identical (a test compares against a direct Pillow save). Without it the turned worst file cost +74 MB in a fresh process, because `BytesIO` is copied as it grows. ## Measured `docker run -m 512m`, the real validator, peak over the state before the call. All files tagged Orientation 6. | File | Not turned (main) | `transpose`, original closed | `exif_transpose(in_place=True)` | This branch | Same file untagged, this branch | |---|---|---|---|---|---| | 4032×3024 photo JPEG | +53 MB | +93 MB | +93 MB | **+51 MB** | +51 MB | | 4080×3072 noise JPEG (worst still) | +63 MB | +96 MB | +96 MB | **+58 MB** | +58 MB | | 4080×3072 RGBA PNG | +53 MB | +96 MB | +96 MB | **+52 MB** | +52 MB | | Map 8192×6144 JPEG, in 1 GB | +210 MB | +384 MB | not run | **+209 MB** | +209 MB | The spill file is page cache, not process memory: up to 38 MB in the backend and 151 MB in the worker. Under pressure it is reclaimed: the photo passed in a 113 MiB container with `oom_kill 0`, where `transpose` was killed in 130 MiB. **Through nginx on a warm backend** (512 MB, two processes): - A 4032×3024 Orientation 6 JPEG with GPS: stored and fetched back as 3024×4032, upright, with no EXIF or GPS. Backend cgroup peak 489 MiB, `oom_kill 0`. - The worst file at the bound with the same tag: stored 3072×4080, peak 496 MiB, `oom_kill 0`. - Those peaks include the spill's page cache; anonymous memory stayed at 382 to 390 MiB. ## Limits - Only the EXIF orientation is honoured. An XMP orientation, or a PNG's "Raw profile" text chunk, is ignored, as browsers do. - A PNG whose `eXIf` chunk follows its pixel data is turned on store, but the header check reports its unturned size. Stored dimensions are right. - Not measured: a CMYK picture with a rotating tag. ## Found on the way, not fixed here On main today, `Image.open` parses a JPEG's EXIF and reads the data of every directory entry. A 6.3 MB JPEG with 60 entries each claiming the same 6 MB took `Image.open` alone to 429 MB. That runs before the pixel check and outside the lock. It gets its own issue. ## Changelog - [x] `changelog.d/1079-exif-orientation.fixed.md` ## Test plan - [x] New `tests/wiki/test_exif_orientation.py`, 222 tests. Against main's `image_service.py`: `61 failed, 50 passed, 96 errors`, the reproduction among the failures. - [x] The new file with the pixel-bound and damaged-image tests, at the head commit: `308 passed`. - [x] **Full backend suite:** `7627 passed, 14 skipped, 107 deselected`. It ran before the last test-only edits; `image_service.py` did not change after it. - [x] A map uploaded as 300×600 pixels with Orientation 6 is stored and reported as 600×300, with tiles 3 by 2. - [x] Lint clean; version and changelog checks OK. - [ ] Not run: frontend, bot, synthetic, journeys (untouched); CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
A photo is stored the way up its EXIF orientation says (#1079)
All checks were successful
CI / Does this change touch the recording path? (pull_request) Successful in 26s
CI / Synthetic session harness (no GPU, no LLM) (pull_request) Has been skipped
CI / Bot/backend version sync (pull_request) Successful in 29s
CI / Backend lint (ruff) (pull_request) Successful in 30s
CI / Docker image build (pull_request) Successful in 34s
CI / Feedback site tests (pull_request) Successful in 51s
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 1m4s
CI / Bot tests and audit (pull_request) Successful in 2m0s
CI / Frontend (shard 2/2) (pull_request) Successful in 7m16s
CI / End-to-end journeys (keyboard) (pull_request) Successful in 13m27s
CI / End-to-end journeys (desktop) (pull_request) Successful in 13m31s
CI / Frontend (shard 1/2) (pull_request) Successful in 14m40s
CI / End-to-end journeys (phone) (pull_request) Successful in 14m37s
CI / Backend migration, tests, and audit (pull_request) Successful in 30m8s
07d05402a3
A phone stores a portrait photo as landscape pixels plus an EXIF
Orientation tag. The picture check re-encoded the pixels and stripped the
tag without applying it, so such a photo was stored on its side as a
page's picture, an image attachment or a map.

The turn is now made before the metadata is stripped, for all eight
orientations, for a JPEG, a PNG and a WebP, on upload and on import. A
picture with no tag, or Orientation 1, is stored byte for byte as before.

Memory is unchanged, so no weight was added and the pixel bounds stand:

- The turn never holds two copies. Image.transpose and
  ImageOps.exif_transpose (in_place or not) build the turned picture
  beside the original: +96 MB for the worst file at the bound against
  +63. _upright turns a strip at a time into a spooled temporary file,
  closes the original, and reads the turned picture back.
- A still picture's output is collected in pieces and joined after the
  picture is let go, in place of a BytesIO that is copied as it grows.
  That copy cost up to 10 MB in a process that had freed a large picture
  before, which a turn always has. Every case now peaks at +58 MB for
  the worst file, tagged or not.

The tag is read by hand from the raw EXIF block Pillow leaves in
Image.info, at most 512 entries of the first directory and nothing they
point to. Image.getexif reads the data of every entry: a 7 MB PNG whose
directory claims 5,000 entries of 6 MB each ended a 512 MB container. A
tag that is not a plain 2 to 8 is ignored, never refused.

read_header, and so check_pixels, header_pixels, map_picture_size and
every refusal sentence, give the upright width and height. A map's
stored width and height come from the stored picture, which is upright.
Pictures already stored are not touched: their orientation went with
their EXIF.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rbrooks referenced this pull request from a commit 2026-10-05 00:39:29 +00:00
Sign in to join this conversation.
No description provided.