A picture is checked by its pixels before it is opened, and a map upload is checked in the worker (#1074) #1078

Merged

claude-bot merged 7 commits from fix/1074-image-pixel-bounds into main 2026-10-04 16:37:48 +00:00

Conversation 3 Commits 7 Files changed 24 +3232 -255

claude-bot commented 2026-10-04 13:12:43 +00:00

Collaborator

Copy link

Closes #1074.

Summary

• A pixel bound before any decode. validate_and_reencode takes a keyword-only max_pixels with no default and checks the header first. A test asserts every call in app/ passes it.
• Bounds.
  • A picture or image attachment: 4080 × 3072 (12.53 million pixels). It takes the common 4032×3024 phone photo. Uploads and imports share it; it replaces the 4000×3000 that #1076 shipped for imports.
  • A map: 8192 × 6144.
  • No side over 16,384 pixels.
• A map upload is checked in the worker. The route streams the file to disk, checks its size, type and header, answers with the map "cutting", and enqueues the same task an imported map uses. A failed replacement keeps the picture the map had.
• Cheaper decode. A 4032×3024 JPEG costs +53 MB, down from +95 MB.
• One picture at a time per container: a cross-process lock, so the backend's two processes do not decode together.
• The picture and attachment routes refuse on the declared size before reading the body. They used to read up to nginx's 110 MB first.
• No migration, no bot change, no compose change.

Behaviour that changes for a person uploading

What	Before	Now
12000×12000 PNG, 450 KB	the backend was killed	refused in words
WebP	stored as WebP	stored as JPEG, or PNG if it has transparency or is lossless
WebP over 3.13 MP (2040×1536)	taken	refused; the sentence says a JPEG or PNG may be larger
CMYK JPEG	taken	counts twice against the bound
24 MP phone photo (5712×4284)	taken, at about +190 MB	refused
Animated GIF	taken	bounded by 6 × frame + total / 4 ≤ bound, at most 1,000 frames

Animated GIF examples: 480×270 up to 362 frames, 800×600 up to 80, 1280×720 up to 30. The largest frame is 1.93 MP.

Measured

Validator alone, docker run -m 512m, peak over the state before the call:

File	main	this branch
4032×3024 JPEG	+95 MB	+53 MB
4032×3024 PNG RGBA	+94 MB	+56 MB
4032×3024 WebP	+257 MB	refused
12000×12000 PNG, 450 KB	exit 137, at 1 GB too	refused, +1 MB
Worst still at the bound (JPEG of noise)	n/a	+62 MB
Worst accepted animated GIF	n/a	+62 MB, 7.1 s

Through nginx on a warm backend, own compose project:

Step	Result	Backend	Worker peak
12000×12000 PNG, three times to each of three routes	9 × 422	329 to 350 MB	n/a
The worst files at the bound, as picture and attachment	all taken and served	347 to 426 MB	n/a
73.5 MB PNG map	200 in 1.4 s, ready at 11 s	372 to 388 MB	501 MB
60 MB RGBA map, 8192×6144	ready at 11 s	378 to 389 MB	626 MB

oom_kill stayed 0 throughout.

Known limits

• The margin is thin. A backend already at 425 MB would reach about 505 of 512 on the worst file.
• The slowest accepted GIF holds the lock for 7.1 s, and other picture uploads to that container wait.
• A failed replacement's banner title still reads "The map couldn't be cut into tiles"; the reason under it is right. No screen offers replacement today.

Changelog

• changelog.d/1074-image-pixel-bounds.security.md

Test plan

• Full backend suite at the first commit: 7348 passed, 14 skipped, 107 deselected.
• At the head commit: tests/export, tests/wiki, the damaged-image, session-hygiene, enqueue and nginx tests: 1123 passed. Lint clean. Version and changelog checks OK.
• Frontend (first commit; untouched since): Map.test.jsx and MapCanvas.test.jsx, 17 passed; eslint 0 errors.
• All 16 coverage scripts report unclassified: none.
• Not run: the end-to-end pass at the head commit (the GIF figures are from the validator alone); a replacement upload through nginx; the full frontend suite, journeys, bot and synthetic (CI).
• Failing first is only partly shown: the new tests use the new signature, so on main they fail at import. The behavioural "before" is main's exit 137.

🤖 Generated with Claude Code

Closes #1074. ## Summary - **A pixel bound before any decode.** `validate_and_reencode` takes a keyword-only `max_pixels` with no default and checks the header first. A test asserts every call in `app/` passes it. - **Bounds.** - A picture or image attachment: `4080 × 3072` (12.53 million pixels). It takes the common 4032×3024 phone photo. Uploads and imports share it; it replaces the 4000×3000 that #1076 shipped for imports. - A map: `8192 × 6144`. - No side over 16,384 pixels. - **A map upload is checked in the worker.** The route streams the file to disk, checks its size, type and header, answers with the map "cutting", and enqueues the same task an imported map uses. A failed replacement keeps the picture the map had. - **Cheaper decode.** A 4032×3024 JPEG costs +53 MB, down from +95 MB. - **One picture at a time per container:** a cross-process lock, so the backend's two processes do not decode together. - **The picture and attachment routes refuse on the declared size before reading the body.** They used to read up to nginx's 110 MB first. - No migration, no bot change, no compose change. ## Behaviour that changes for a person uploading | What | Before | Now | |---|---|---| | 12000×12000 PNG, 450 KB | the backend was killed | refused in words | | WebP | stored as WebP | stored as JPEG, or PNG if it has transparency or is lossless | | WebP over 3.13 MP (2040×1536) | taken | refused; the sentence says a JPEG or PNG may be larger | | CMYK JPEG | taken | counts twice against the bound | | 24 MP phone photo (5712×4284) | taken, at about +190 MB | refused | | Animated GIF | taken | bounded by `6 × frame + total / 4 ≤ bound`, at most 1,000 frames | Animated GIF examples: 480×270 up to 362 frames, 800×600 up to 80, 1280×720 up to 30. The largest frame is 1.93 MP. ## Measured Validator alone, `docker run -m 512m`, peak over the state before the call: | File | main | this branch | |---|---:|---:| | 4032×3024 JPEG | +95 MB | +53 MB | | 4032×3024 PNG RGBA | +94 MB | +56 MB | | 4032×3024 WebP | +257 MB | refused | | 12000×12000 PNG, 450 KB | exit 137, at 1 GB too | refused, +1 MB | | Worst still at the bound (JPEG of noise) | n/a | +62 MB | | Worst accepted animated GIF | n/a | +62 MB, 7.1 s | Through nginx on a warm backend, own compose project: | Step | Result | Backend | Worker peak | |---|---|---|---| | 12000×12000 PNG, three times to each of three routes | 9 × 422 | 329 to 350 MB | n/a | | The worst files at the bound, as picture and attachment | all taken and served | 347 to 426 MB | n/a | | 73.5 MB PNG map | 200 in 1.4 s, ready at 11 s | 372 to 388 MB | 501 MB | | 60 MB RGBA map, 8192×6144 | ready at 11 s | 378 to 389 MB | 626 MB | `oom_kill` stayed 0 throughout. ## Known limits - **The margin is thin.** A backend already at 425 MB would reach about 505 of 512 on the worst file. - **The slowest accepted GIF holds the lock for 7.1 s**, and other picture uploads to that container wait. - **A failed replacement's banner title** still reads "The map couldn't be cut into tiles"; the reason under it is right. No screen offers replacement today. ## Changelog - [x] `changelog.d/1074-image-pixel-bounds.security.md` ## Test plan - [x] **Full backend suite** at the first commit: `7348 passed, 14 skipped, 107 deselected`. - [x] **At the head commit:** `tests/export`, `tests/wiki`, the damaged-image, session-hygiene, enqueue and nginx tests: `1123 passed`. Lint clean. Version and changelog checks OK. - [x] **Frontend** (first commit; untouched since): `Map.test.jsx` and `MapCanvas.test.jsx`, `17 passed`; eslint 0 errors. - [x] All 16 coverage scripts report `unclassified: none`. - [ ] **Not run:** the end-to-end pass at the head commit (the GIF figures are from the validator alone); a replacement upload through nginx; the full frontend suite, journeys, bot and synthetic (CI). - [ ] **Failing first is only partly shown:** the new tests use the new signature, so on main they fail at import. The behavioural "before" is main's exit 137. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 2 commits 2026-10-04 13:12:43 +00:00

A picture's pixels are counted before it is decoded, and a map's picture is checked in the worker (#1074) ... 716cf2ee61

A 450 KB PNG of 12000x12000 in one flat colour ended the backend: the
validator decoded any picture with no bound on its pixels, and decoding
takes memory by pixels, not by file size. Anyone signed in who could add a
picture to a page could do it.

Part 1. image_service.validate_and_reencode now reads the header first and
refuses a picture over the bound with a 422 that says its size and the
largest taken, before anything is decoded. The bound is a required keyword
argument, so no caller can decode a picture nobody has counted: a page's
picture, an attachment and a picture from a web address at 4080x3072 (over
all frames), a map at 8192x6144. One constant for uploads and imports; the
import's 4000x3000 refused an iPhone's 4032x3024.

The decode is cheaper, which is what lets the bound take a 12 MP photograph
beside a warm backend (4032x3024 JPEG: 95 MB before, 53 MB now):
- one decoded copy, not two (Pillow copied an unloaded picture on save, and
  an RGB JPEG was converted to RGB);
- a WebP is stored as a JPEG, or as a PNG with transparency or without
  loss: the WebP encoder took 6 to 35 bytes a pixel;
- a GIF is saved straight from the file, not from a list of every frame;
- one picture at a time in the container, by a lock file in its own /tmp,
  with the memory handed back after each.
A WebP counts four times, and a CMYK JPEG or an animated GIF twice, as
measured. A JPEG with a second picture behind it (a phone's HDR gain map)
counts once. An upload over the size cap is refused before it is read.

Part 2. The map upload route no longer decodes: it stores the upload in
pieces, refuses what the header can tell, commits the map as "cutting" and
queues restore_imported_map, as an import does. The worker checks the
picture and cuts the tiles; a failure is said on the map's page. A map that
had a picture keeps it until the new one has passed. The hourly sweep
leaves an upload that is still waiting its turn.

No migration, no bot change, no compose change. The frontend already waits
on a map that is "cutting" with no picture; a test holds that for an upload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

An animated GIF is counted by what it costs: six frames and a quarter of them all (#1074) ... aef9ee2607

The first cut counted an animated GIF twice over all its frames, which
refused an ordinary one (480x270 at 60 frames, 7.8 MP) to keep out the
expensive kind. Measured apart, writing one takes 15 to 26 bytes a pixel of
one frame (the most with transparency) while it is compared with the frame
before, and 1.25 bytes a pixel of all the frames, which the writer keeps to
the end. So many small frames are cheap and two large ones are not.

The rule is 6 x frame + total / 4 <= the picture bound, and at most 1,000
frames. At the bound that is 50 MP over all frames: the worst accepted GIF
found took +62 MB (220x220 at 1,000 frames) and the slowest 7.1 s under the
lock, on a host at load 5. 480x270 takes 362 frames; the largest frame is
1.93 MP.

The GIF is written to a temporary file, not into memory beside the frames,
through a counter that refuses it past twice the upload cap as it is
written. The refusal says how many frames the picture's size may have, or
the largest frame.

APNG and animated WebP are left as they were: only their first frame is
decoded and stored, so they do not cost what a GIF does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented 2026-10-04 13:28:25 +00:00

Author

Collaborator

Copy link

Not to be merged yet: two things.

1. Independent review of aef9ee26: one confirmed bypass and seven smaller defects. Fixes are in progress on this branch.

• High, confirmed by running: the bound can be bypassed with an animated GIF. A GIF's later frames can be larger than its logical screen, and the header check sees only the screen. A 48 KB file reporting 1×1 with two frames was accepted and peaked at 1,007 MB; a 125 KB one peaked at 2,606 MB. Fix: read the frame descriptors from the raw bytes before any decode.
• Medium, confirmed by running: a progressive JPEG costs about 2.4 times what the bound assumes. At 4080×3072: +86 MB (4:2:0) and +122 MB (4:4:4), against +50 MB for a baseline file. Fix: measure and weight it.
• Medium: a replacement upload can leave a map "cutting" forever if the process dies between the commit and the enqueue. The sweep covers only maps with no picture.
• Low to medium: a second task after a failed upload erases the failure, or writes the import's reason on it.
• Low to medium: the decode and the lock wait run on the event loop, so both backend processes stop answering during a slow decode.
• Low: two crash windows in the worker leave a map's file and row out of step.
• Low: the output cap refuses a whole import for a page picture; it should leave the picture out.
• Low: a cancelled upload can leave an uploaded file behind.

Found sound

• The type allow-list runs before any open.
• A GIF's first frame beyond its screen is refused.
• 550,000 frames cost 1.75 s and no memory.
• PNG text chunks are bounded.
• No Pillow error becomes a 500.
• The temp file and the lock are released on every path.
• The map upload's path is built from ids only, and its size cap is enforced while streaming.

Not checked by the review: metadata stripping on re-encode, whether a pending file can be served, and WebP edge cases. The builder is answering the first two by running.

Pre-existing, to be filed separately: EXIF orientation is never applied, so a photo that relies on the tag is stored sideways.

2. This run's CI failure is the runner's disk, not the code

All four failed jobs stopped on no space left on device:

• the backend job after 500 passed, on a DiskFullError from Postgres;
• the three journeys jobs while extracting image layers.

The next push will start a fresh run.

**Not to be merged yet: two things.** ## 1. Independent review of `aef9ee26`: one confirmed bypass and seven smaller defects. Fixes are in progress on this branch. - **High, confirmed by running: the bound can be bypassed with an animated GIF.** A GIF's later frames can be larger than its logical screen, and the header check sees only the screen. A 48 KB file reporting 1×1 with two frames was accepted and peaked at 1,007 MB; a 125 KB one peaked at 2,606 MB. Fix: read the frame descriptors from the raw bytes before any decode. - **Medium, confirmed by running: a progressive JPEG costs about 2.4 times what the bound assumes.** At 4080×3072: +86 MB (4:2:0) and +122 MB (4:4:4), against +50 MB for a baseline file. Fix: measure and weight it. - **Medium: a replacement upload can leave a map "cutting" forever** if the process dies between the commit and the enqueue. The sweep covers only maps with no picture. - **Low to medium: a second task after a failed upload erases the failure**, or writes the import's reason on it. - **Low to medium: the decode and the lock wait run on the event loop**, so both backend processes stop answering during a slow decode. - **Low: two crash windows** in the worker leave a map's file and row out of step. - **Low: the output cap refuses a whole import** for a page picture; it should leave the picture out. - **Low: a cancelled upload can leave an `uploaded` file behind.** **Found sound** - The type allow-list runs before any open. - A GIF's first frame beyond its screen is refused. - 550,000 frames cost 1.75 s and no memory. - PNG text chunks are bounded. - No Pillow error becomes a 500. - The temp file and the lock are released on every path. - The map upload's path is built from ids only, and its size cap is enforced while streaming. **Not checked by the review:** metadata stripping on re-encode, whether a pending file can be served, and WebP edge cases. The builder is answering the first two by running. **Pre-existing, to be filed separately:** EXIF orientation is never applied, so a photo that relies on the tag is stored sideways. ## 2. This run's CI failure is the runner's disk, not the code All four failed jobs stopped on `no space left on device`: - the backend job after `500 passed`, on a `DiskFullError` from Postgres; - the three journeys jobs while extracting image layers. The next push will start a fresh run.

claude-bot referenced this pull request 2026-10-04 13:28:31 +00:00

A photo that relies on its EXIF orientation is stored sideways #1079

rbrooks added 1 commit 2026-10-04 14:12:09 +00:00

Close eight defects the PR 1078 review found, with a test each (#1074) ... 6d03120bc2

1 (HIGH). A GIF's later frames can be larger than its logical screen, and
the header check never saw it: n_frames steps over frames without updating
size, and on decode Pillow grows the image to each frame and allocates at
that size. A 48 KB file whose screen is 1x1 and whose second frame is
8000x8000 reported (1,1,2) and peaked over 1 GB. The frame extents are now
walked in the raw bytes (read_header._gif_screen_and_frames) before any
decode; a frame past the screen is refused, peak 24 MB. APNG (Pillow's
chunk_fcTL rejects frames past the canvas) and animated WebP (libwebp
rejects them at demux) do not have the hole; left as they are.

2 (MEDIUM). A progressive JPEG costs ~2.4x a baseline (4080x3072 4:4:4:
+54 MB baseline, +122 MB progressive). It is now counted twice in the
backend (largest page picture 6.27M pixels; the worst accepted took +62 MB),
and once for a map, decoded in the 1 GB worker (a 4:4:4 map at the bound took
+482 MB, fits). A JPEG with more than 32 scans is refused (a crafted file can
have hundreds, slow however few its pixels; a normal progressive has ten).

3 (MEDIUM). The stuck-map sweep only looked at maps with no image_path, so a
replacement upload lost between commit and enqueue left a map "cutting"
forever. It now also recovers a cutting map that still has its old picture:
clear_stuck_upload removes the stale file and restores "ready".

4 (LOW-MEDIUM). A second worker task after a failed or finished upload fell
into the import branch and wrote the import's reason over the upload's, or
re-cut a map to "ready". The worker now returns without touching the row when
there is no file to process and no committed-but-uncut picture.

5 (LOW-MEDIUM). save_uploaded_image, save_attachment, the URL import and the
import's restore_exported_image decoded synchronously on the event loop, so a
held lock froze the process (health checks included). They run on one
dedicated decode thread now (image_service.run_off_loop); one thread, not a
pool, so RSS does not ratchet per PR 1072.

6 (LOW). save_map_image writes original.<hash>.<ext>, not a bare name, so a
crash before the row commits cannot leave the row pointing at a file whose
bytes are already the new ones; and a redelivery that finds the checked file
after a committed success reads the row (a picture newer than the file) as
done instead of marking the map failed.

7 (LOW). The re-encode output cap (ImageTooLarge) reaching the import's page
picture path refused the whole import; it now leaves the picture out and
lists it, as the attachment path does.

8 (LOW). A map upload cancelled mid-copy left an orphan file; the route
removes it on CancelledError, and fix 3's sweep is the backstop.

Also, answering the review: metadata is now stripped on every path (EXIF
incl. GPS, XMP, ICC, JPEG/GIF comments, PNG text) by clearing img.info before
the save, which a JPEG and a GIF comment had survived. And no route or nginx
can serve the pending uploaded/imported/.partial/checking files: their names
never reach a served DB column, /internal-files/ is internal, and nginx
refuses /media/private/.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented 2026-10-04 14:51:20 +00:00

Author

Collaborator

Copy link

All eight review findings fixed in 6d03120b, each with a test. The CI runner's disk was full for the last run, so the evidence below is from the dev host.

Finding	Fix
1. GIF frames larger than the screen	The frame descriptors are read from the raw bytes before any decode; a frame past the screen is refused. The reviewer's hostile GIF: 1,007 MB before, refused at 24 MB now
2. Progressive JPEG	Counts twice in the backend, and a JPEG with more than 32 scans is refused. Not weighted for maps: the worker has room (+482 MB over about 207 MB, 3.9 s)
3. A replacement stuck "cutting"	The hourly sweep now covers maps that have a picture: the stale file is removed and the map returns to "ready" on the picture it kept
4. A second task erasing a failure	The worker returns without touching the row when there is nothing to do
5. Decode on the event loop	One dedicated thread for picture decodes; the loop answers /api/me while a decode blocks
6. Two crash windows	The new picture is written as original.<hash>.<ext> and switched by the row; a redelivery after success is read as done
7. The output cap refusing an import	The picture is left out and listed
8. A cancelled upload's orphan file	Removed on cancellation; the sweep in 3 is the backstop

APNG and animated WebP do not have hole 1: Pillow refuses an APNG frame past the canvas, and libwebp refuses a forged oversized frame.

What changes for a person, beyond the table in the description

• A progressive JPEG may be 6.27 million pixels (2890×2168), half a baseline one. A progressive 4032×3024 is refused, and the sentence says a baseline JPEG may be 12.53 million. Phone cameras write baseline files; photos saved again by an editor or a website are often progressive.
• A replacement upload that is lost leaves the map "ready" on its old picture, with no banner. There is no screen state for "your replacement did not apply".

Two answers the review asked for

• Metadata. JPEG and GIF comments survived the re-encode, on main as well. They are now stripped. EXIF (GPS included), XMP, ICC profiles and PNG text were already dropped. Tested per format, including WebP converted to JPEG and to PNG.
• Serving. A pending file (uploaded, imported, .partial and their .checking forms) cannot be served. They sit under private/, nginx answers 404 there, and the backend redirects only to paths held in a row.

Ran at 6d03120b

• Full backend suite: 7387 passed, 14 skipped, 107 deselected.
• Lint clean; version and changelog checks OK.
• Through nginx on a warm backend:
  • the hostile GIF: 422 on the picture, attachment and map routes; backend flat at 374 MB;
  • a progressive 4032×3024: 422 with the sentence;
  • a progressive 2890×2168: stored and served; backend peak 415 MB;
  • oom_kill 0 and no restart throughout.

Not run: journeys, the bot and synthetic suites, and the frontend (untouched since the first commit). The progressive figures in finding 2 are from the validator alone.

**All eight review findings fixed in `6d03120b`, each with a test.** The CI runner's disk was full for the last run, so the evidence below is from the dev host. | Finding | Fix | |---|---| | 1. GIF frames larger than the screen | The frame descriptors are read from the raw bytes before any decode; a frame past the screen is refused. The reviewer's hostile GIF: 1,007 MB before, refused at 24 MB now | | 2. Progressive JPEG | Counts twice in the backend, and a JPEG with more than 32 scans is refused. Not weighted for maps: the worker has room (+482 MB over about 207 MB, 3.9 s) | | 3. A replacement stuck "cutting" | The hourly sweep now covers maps that have a picture: the stale file is removed and the map returns to "ready" on the picture it kept | | 4. A second task erasing a failure | The worker returns without touching the row when there is nothing to do | | 5. Decode on the event loop | One dedicated thread for picture decodes; the loop answers `/api/me` while a decode blocks | | 6. Two crash windows | The new picture is written as `original.<hash>.<ext>` and switched by the row; a redelivery after success is read as done | | 7. The output cap refusing an import | The picture is left out and listed | | 8. A cancelled upload's orphan file | Removed on cancellation; the sweep in 3 is the backstop | **APNG and animated WebP do not have hole 1:** Pillow refuses an APNG frame past the canvas, and libwebp refuses a forged oversized frame. ## What changes for a person, beyond the table in the description - **A progressive JPEG may be 6.27 million pixels** (2890×2168), half a baseline one. A progressive 4032×3024 is refused, and the sentence says a baseline JPEG may be 12.53 million. Phone cameras write baseline files; photos saved again by an editor or a website are often progressive. - **A replacement upload that is lost** leaves the map "ready" on its old picture, with no banner. There is no screen state for "your replacement did not apply". ## Two answers the review asked for - **Metadata.** JPEG and GIF comments survived the re-encode, on main as well. They are now stripped. EXIF (GPS included), XMP, ICC profiles and PNG text were already dropped. Tested per format, including WebP converted to JPEG and to PNG. - **Serving.** A pending file (`uploaded`, `imported`, `.partial` and their `.checking` forms) cannot be served. They sit under `private/`, nginx answers 404 there, and the backend redirects only to paths held in a row. ## Ran at `6d03120b` - **Full backend suite:** `7387 passed, 14 skipped, 107 deselected`. - Lint clean; version and changelog checks OK. - **Through nginx on a warm backend:** - the hostile GIF: 422 on the picture, attachment and map routes; backend flat at 374 MB; - a progressive 4032×3024: 422 with the sentence; - a progressive 2890×2168: stored and served; backend peak 415 MB; - `oom_kill 0` and no restart throughout. **Not run:** journeys, the bot and synthetic suites, and the frontend (untouched since the first commit). The progressive figures in finding 2 are from the validator alone.

rbrooks added 2 commits 2026-10-04 14:57:56 +00:00

Every JPEG that libjpeg buffers whole counts as progressive (#1074) ... 78357d49b2

The weight keyed on the SOF2 frame alone. libjpeg holds the whole picture's
coefficients for every progressive frame type (SOF2, SOF6, and the
arithmetic-coded SOF10 and SOF14) and for a sequential file whose scans do
not each carry every component, so those cost the same and were weighted
as baseline. They now count as progressive, as does any file with more
than one scan. From libjpeg's source (jdinput.c, has_multiple_scans); not
measured, since nothing here writes such files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Format the JPEG profile tests (#1074) ... 3fc0c32a57

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented 2026-10-04 14:58:08 +00:00

Author

Collaborator

Copy link

One more fix, 3fc0c32a: the progressive-JPEG weight was too narrow.

A second, narrow review of the two raw parsers was started and did not complete, so I read them myself.

The GIF walk: I found no disagreement with Pillow's reader, by reading both.

• Both stop at the trailer.
• Both consume extensions and sub-blocks the same way.
• Both size the colour tables the same way.
• The walk refuses an unknown block, where Pillow skips it.
• The frame cap ends the walk above 1,000 frames, which is then refused.

This was not tested with crafted files beyond the ones the builder and the first reviewer ran.

The JPEG profile: one gap, fixed.

• The weight keyed on the SOF2 frame alone. libjpeg holds the whole picture's coefficients for more than that:
  • every progressive frame type (SOF2, SOF6, and the arithmetic-coded SOF10 and SOF14);
  • a sequential file whose scans do not each carry every component (jdinput.c: comps_in_scan < num_components sets has_multiple_scans).
• All of those now count as progressive, as does any file with more than one scan.
• From libjpeg's source, not measured. Nothing here writes such files, so the cost of an arithmetic-progressive or non-interleaved file was not measured. The change only makes the check stricter.

Ran at 3fc0c32a

• tests/wiki/test_image_pixel_bounds.py: 77 passed, with ten new cases for the frame types and scan shapes.
• Lint clean.
• Locally, with the real Pillow: a baseline RGB, greyscale and 4:4:4 JPEG profile as not progressive with one scan; a progressive one as progressive with ten; a 4032×3024 baseline photo is still accepted.

Not re-run: the full backend suite (CI) and the end-to-end pass.

**One more fix, `3fc0c32a`: the progressive-JPEG weight was too narrow.** A second, narrow review of the two raw parsers was started and did not complete, so I read them myself. **The GIF walk: I found no disagreement with Pillow's reader, by reading both.** - Both stop at the trailer. - Both consume extensions and sub-blocks the same way. - Both size the colour tables the same way. - The walk refuses an unknown block, where Pillow skips it. - The frame cap ends the walk above 1,000 frames, which is then refused. This was not tested with crafted files beyond the ones the builder and the first reviewer ran. **The JPEG profile: one gap, fixed.** - The weight keyed on the `SOF2` frame alone. libjpeg holds the whole picture's coefficients for more than that: - every progressive frame type (`SOF2`, `SOF6`, and the arithmetic-coded `SOF10` and `SOF14`); - a sequential file whose scans do not each carry every component (`jdinput.c`: `comps_in_scan < num_components` sets `has_multiple_scans`). - All of those now count as progressive, as does any file with more than one scan. - **From libjpeg's source, not measured.** Nothing here writes such files, so the cost of an arithmetic-progressive or non-interleaved file was not measured. The change only makes the check stricter. **Ran at `3fc0c32a`** - `tests/wiki/test_image_pixel_bounds.py`: `77 passed`, with ten new cases for the frame types and scan shapes. - Lint clean. - Locally, with the real Pillow: a baseline RGB, greyscale and 4:4:4 JPEG profile as not progressive with one scan; a progressive one as progressive with ten; a 4032×3024 baseline photo is still accepted. **Not re-run:** the full backend suite (CI) and the end-to-end pass.

rbrooks added 1 commit 2026-10-04 15:27:17 +00:00

Re-run CI: Frontend shard 1 failed on AppFrame.test.jsx, which this branch does not touch (#1074) ... ae4e5796c6

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot referenced this pull request 2026-10-04 15:27:29 +00:00

Flaky test: AppFrame "choosing a rule opens it in place" does not find its dialog under load #1080

rbrooks added 1 commit 2026-10-04 16:08:04 +00:00

Merge remote-tracking branch 'origin/main' into fix/1074-image-pixel-bounds d3ea9881c5

claude-bot merged commit 66f2c935e0 into main 2026-10-04 16:37:48 +00:00

claude-bot referenced this pull request from a commit 2026-10-04 16:37:49 +00:00

Merge PR #1078: a picture is checked by its pixels before it is opened, and a map upload is checked in the worker (#1074)

claude-bot deleted branch fix/1074-image-pixel-bounds 2026-10-04 16:37:49 +00:00

claude-bot referenced this pull request 2026-10-04 16:37:57 +00:00

[Security] An uploaded image can exhaust the backend's memory: the validator decodes it with no pixel bound #1074

rbrooks referenced this pull request from a commit 2026-10-04 16:38:11 +00:00

chore(release): v4.5.0-rc4

claude-bot referenced this pull request 2026-10-04 16:38:16 +00:00

chore(release): v4.5.0-rc4 #1083

claude-bot referenced this pull request 2026-10-04 17:19:41 +00:00

CI fills the runners' disks: each journeys run leaves its images behind #1081

claude-bot referenced this pull request 2026-10-04 20:54:56 +00:00

[Security] A JPEG's EXIF can exhaust the backend's memory when the file is opened #1087

rbrooks referenced this pull request from a commit 2026-10-04 22:35:57 +00:00

Strip a picture's metadata before it is opened, not only before it is saved (#1087)

claude-bot referenced this pull request 2026-10-04 22:37:31 +00:00

A picture's metadata is removed before the picture is opened (#1087) #1088

claude-bot referenced this pull request 2026-10-05 05:46:00 +00:00

A picture too large for the server is shrunk on the way in, not refused #1096

rbrooks referenced this pull request from a commit 2026-10-05 07:19:24 +00:00

feat(webapp): a JPEG too large for the server is made smaller on the way in (#1096)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!1078

No description provided.