Skip to content

fix(image): reject images whose frames decode to too many pixels - #4520

Merged
badGarnet merged 8 commits into
mainfrom
fix/image-pixel-limit
Oct 2, 2026
Merged

badGarnet merged 8 commits into
mainfrom
fix/image-pixel-limit

Conversation

@badGarnet

@badGarnet badGarnet commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Every frame of a multi-frame image (e.g. TIFF) is decoded to RGB, and hi_res (DocumentLayout.from_image_file in unstructured-inference) holds every frame in memory at once. Blank frames compress to almost nothing, and PIL's per-frame limit (raised to 5e8 in pdf.py) does not bound the total.

  • Check before partitioning, without decoding pixels. Once the strategy is resolved, partition_pdf_or_image(is_image=True) measures the image and raises UnprocessableEntityError above IMAGE_MAX_TOTAL_PIXELS (new env setting, default 500,000,000, matching the per-frame limit pdf.py already sets).
    • With hi_res, which decodes every frame, every frame is charged, without allocating or decoding any. Some formats' seek() allocates or decodes the frame (pi-heif allocates it; APNG and GIF load it), so frames are visited only where seek() is metadata-only:
      • TIFF and MPO frames, which can differ in size, are summed from each frame's header (IFD or MP entry).
      • HEIF images are summed from the sizes pi-heif reads from the container when opening it.
      • Canvas-composited frames (APNG, animated WebP, GIF) are charged canvas size × the frame count Pillow reads from metadata, computed in constant time. That count can be one the file merely declares (APNG's acTL allows 2^31), so it is never iterated; the work is bounded by the frames physically in the file.
    • Instrumenting PIL's allocator (Image.core.new, ImageFile.load) confirms none of these measurements allocates a pixel buffer or loads a frame. That instrumentation can't see allocations inside native decoders: libwebp, for example, may allocate its canvas when the file is opened, exactly as it does on main.
    • This bounds the pixels partitioning will decode. It is not a limit on process-wide memory or concurrency.
    • ocr_only uses only the first frame, so only the first frame is charged, from the size Image.open() reads from the header. It still touches only the first frame, as before.
    • It accepts a filename, a file or bytes, and leaves a borrowed file open at its read position, including when it rejects the image.
  • Errors. PIL's own DecompressionBombError for a single oversized frame is reported as UnprocessableEntityError. A file PIL can't identify is left to the existing error paths.

Measurements (blank group4 TIFF frames, 10,000 × 10,000 each)

File Before After
19 KB, 3 frames (3e8 px) hi_res: 3.5 GiB peak, 12s unchanged (under the limit)
195 KB, 30 frames (3e9 px) ~9 GB of RGB decoded before inference starts rejected in <10 ms

hi_res measured about 11 B per pixel including the model, so the 5e8 default bounds a single image to roughly 6 GB. That's about 59 pages of a 300-DPI letter-size scan.

Tests

  • Pixels are summed across TIFF frames, from a filename, a file and bytes (exact-limit pass, one-under fail).
  • Measuring a mixed-size TIFF, an APNG, an animated WebP, a GIF, an MPO and a two-image HEIF (committed 867-byte fixture) charges every frame exactly and allocates no pixel buffer.
  • The scan stops at the frame that passes the limit, without reading the next frame's header.
  • A rejected borrowed file stays open at its position.
  • A 1×1 APNG whose acTL declares 2^31 − 1 frames is rejected in constant time (it took 14 s on the previous commit), allocating nothing.
  • A borrowed file stays open at its position when PIL fails while reading a frame header.
  • Dispatch: hi_res and auto measure every frame and ocr_only only the first, including when a missing dependency makes hi_res fall back to ocr_only or ocr_only fall back to hi_res; PDFs bypass the image limit.
  • partition_image(strategy="hi_res") rejects a 6-frame file before any frame is converted.
  • A decompression-bomb frame is reported as unprocessable, and non-image bytes are ignored.

Full test_image.py: 53 passed, 2 skipped (48 passed, 2 skipped on main).

🤖 Generated with Claude Code

Review in cubic

Every frame of a multi-frame image (e.g. TIFF) is decoded to RGB, and
hi_res holds all frames in memory at once, but blank frames compress to
almost nothing: a ~195 KB TIFF of 30 blank 10,000 x 10,000 frames decodes
to ~9 GB of RGB. PIL's per-frame limit does not bound the total.

Read each frame's size from its header in partition_pdf_or_image() before
any strategy runs, without decoding pixels, and raise
UnprocessableEntityError once the frames exceed IMAGE_MAX_TOTAL_PIXELS in
total (default 500,000,000, matching the per-frame limit pdf.py sets).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic

Comment thread unstructured/partition/pdf.py
PIL raises DecompressionBombError at open or seek for a single frame over
twice MAX_IMAGE_PIXELS, before the frame loop can total the pixels, so
the caller got PIL's error instead of UnprocessableEntityError. Translate
it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.

Re-trigger cubic

@cragwolfe cragwolfe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GPT Pro review of exact head 176f06a, verified against source, main and upstream dependency implementation. The TIFF aggregate guard is a meaningful improvement, but generic frame iteration introduces allocation/decoding of secondary frames into ocr_only before the budget check. That concrete regression prevents approval; see inline finding. Exact-head test run 36953097240 completed successfully (53 reported successful checks, one skipped). No local tests or source changes.

Nonblocking follow-ups: assert borrowed-stream ownership and position after rejection (the current assertion is skipped when the helper raises); cover mixed-size TIFF frames and early exit; cover OCR dispatch and PDF exclusion. Existing first-frame-only OCR and the fact that a pixel limit is not a process/concurrency memory limit are preexisting limitations.

(authored by codex)

Comment thread unstructured/partition/pdf.py Outdated
with PILImage.open(file if file is not None else filename) as image:
total_pixels = 0
# -- seeking to a frame parses its header; pixels are decoded only on `.load()` --
for n_frames, frame in enumerate(ImageSequence.Iterator(image), start=1):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] ImageSequence.Iterator is not a generic metadata-only admission boundary. It invokes plugin seek() before the dimensions are charged on the next line. The permitted pi-heif 1.2.0 plugin's seek() sets the new frame dimensions then calls Image.core.new(self._mode, self._size), allocating a pixel buffer before returning. Pillow APNG seeking also calls self.load() and copies the previous core. For ocr_only, main opens only the initial/primary image (see the unchanged one-item images list in _partition_pdf_or_image_with_ocr); this unconditional preflight newly processes unused secondary frames. A small primary HEIF image plus a 10,000x10,000 secondary image can therefore allocate the secondary buffer even when IMAGE_MAX_TOTAL_PIXELS is much lower, before rejection; long APNG sequences also introduce an extra decoding pass. This is source-derived evidence, not a local memory benchmark. Preserve TIFF protection using an allocation-free format-specific dimension path, or narrow/document the incremental guard to formats whose seek is metadata-only while preserving prior behavior elsewhere. Add small fixtures instrumenting plugin allocation/load, not just Image.convert, and pin OCR-only behavior.

(authored by codex)

Review follow-ups:

- ImageSequence.Iterator calls each format's seek(), which allocates the
  frame (pi-heif) or decodes it (APNG, GIF) before its size can be
  charged, so the guard could allocate a large secondary frame before
  rejecting it, and ocr_only, which uses only the first frame, now
  processed every frame. Iterate frames only for TIFF, whose seek() just
  reads the frame's IFD; for other formats charge the first frame, whose
  size Image.open() reads from the header.
- Apply the guard after the strategy is resolved: only hi_res, which
  decodes every frame, is charged for all of a TIFF's frames.
- Cover: no pixel allocation or frame load while measuring (TIFF and
  APNG), mixed frame sizes, stopping at the frame that passes the limit,
  a rejected borrowed file left open at its position, the hi_res and
  ocr_only dispatch, and PDFs bypassing the image limit.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread unstructured/partition/pdf.py Outdated
…coding

hi_res decodes every frame of any multi-frame image, but the guard charged
only the first frame of formats other than TIFF, so many frames under the
limit could add up past it. Every frame is now charged for hi_res, each
measured without allocating or decoding it:

- TIFF and MPO frames, which can differ in size, from each frame's header
  (their seek() is metadata-only);
- HEIF images from the sizes pi-heif reads from the container on open
  (its seek() allocates the frame, so it is not used);
- canvas-composited frames (APNG, animated WebP, GIF) as canvas size times
  the frame count Pillow reads from metadata.

Covered for each format with an instrumented allocator, using a small
two-image HEIF fixture since pi-heif cannot encode.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files (changes from recent commits).

Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread test_unstructured/partition/pdf_image/test_image.py Outdated
Pillow accepts a bare int for an RGB putpixel() (setting only the red
band), but a tuple states the intent and does not rely on that leniency.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.

Re-trigger cubic

@cragwolfe cragwolfe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh full-context GPT Pro re-review of 1ecd9cf, verified against source and main: COMMENT. The prior secondary-frame decode/allocation-before-admission finding is resolved: OCR-only measures the initial frame, and hi_res uses format-specific metadata paths instead of allocating HEIF/APNG secondary seeks. The new tests meaningfully cover those repairs.

One new High defect prevents approval: fixed-canvas accounting expands an untrusted APNG frame count into a synthetic Python loop. A tiny 1x1 APNG can force 500,000,001 additions before default-limit rejection. Details and acceptance criteria are inline. This is a different introduced defect, rather than repetition of the resolved prior finding.

Nonblocking follow-ups: qualify the blanket “no pixel allocations” claim—WebP's native decoder constructor allocates canvas buffers that the Image.core.new/ImageFile.load instrumentation does not observe, as it already does on main. Add AUTO/dependency-fallback policy coverage and injected metadata/seek-failure ownership checks. The pixel estimate is not a process-wide memory/concurrency limit.

Validation: complete untruncated production bundle, fresh holistic Pro response and source/dependency verification. Exact-head checks show two successful non-test checks and one neutral security result; no Actions test workflow is recorded for this SHA. Previous-head green tests and historical author test counts do not establish current-head test execution. No local tests or source changes.

(authored by codex)

Comment thread unstructured/partition/pdf.py Outdated
for heif_image in heif_file:
yield heif_image.size
return
yield from itertools.repeat(image.size, getattr(image, "n_frames", 1))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Account for uniform canvas frames without a header-count-sized loop. _iter_frame_sizes() repeats image.size image.n_frames times, and the caller performs one Python addition per synthetic frame. Pillow reads APNG n_frames directly from the acTL uint32 (accepting values up to 0x80000000) before validating the actual frame sequence. A tiny 1x1 APNG with a valid first frame and a forged count above 500,000,000 therefore causes 500,000,001 iterations before the default pixel limit rejects it. Main has no such synthetic replay and reaches actual frame EOF instead. This creates a new CPU amplification in the admission guard despite resolving the prior allocation/decode-before-admission defect. Source-verified; no local timing/test execution.

For formats with a shared known canvas, compute width * height * n_frames in constant time and compare directly to the limit; keep per-frame iteration for varying-size TIFF/MPO/HEIF metadata. Preserve the OCR-only initial-frame behavior, exact-budget admission, error reporting and borrowed-stream restoration. Add a small APNG fixture with a forged large acTL count, verifying bounded admission work and rejection without allocating/loading pixels; retain exact/one-under tests for valid animations.

(authored by codex)

badGarnet and others added 2 commits October 2, 2026 12:32
…rame

The guard charged canvas-composited formats by repeating the canvas size
n_frames times, one addition per frame. Pillow takes an APNG's n_frames
straight from its acTL chunk, which may declare up to 2^31 frames, so a
few-hundred-byte 1x1 APNG cost 500 million iterations before rejection.

Measure frames as runs of same-sized frames: TIFF, MPO and HEIF yield one
run per frame physically in the file, and canvas formats one run of
n_frames, compared against the limit in constant time. The error still
names the frame that takes the total past the limit.

Also cover: auto and the dependency fallbacks resolving which frames are
charged, and a borrowed file left open at its position when reading a
frame header fails. Document that the limit bounds decoded pixels, not
allocations inside native decoders or process-wide memory.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	CHANGELOG.md
#	unstructured/__version__.py
#	unstructured/partition/utils/config.py

@cragwolfe cragwolfe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE the source change at d617e79 after a fresh holistic, full-context GPT Pro review and independent source verification against freshly fetched main. Both prior material findings are resolved: format-specific metadata accounting avoids allocating/decoding secondary frames before admission and leaves unused OCR-only frames untouched; uniform canvas runs now use constant-time multiplication rather than expanding APNG's untrusted declared count into a synthetic loop. Exact-budget admission, first-over-limit reporting, resolved-strategy fallbacks and borrowed-stream restoration are preserved. The checked-in forged-count, fallback and metadata-failure regressions meaningfully cover the repairs.

Nonblocking follow-ups:

  • pdf.py:_iter_frame_runs assumes the initial canvas size for GIF. Pillow's metadata-only frame-count scan does not incorporate later descriptors that grow the canvas, so a 10x10 initial frame followed by a 1000x1000 frame can be undercounted. Main already decodes these inputs without an aggregate guard; this is incomplete hardening, not an introduced regression. Account for effective canvas growth from metadata or reject out-of-canvas descriptors, without restoring decoding seeks. Cover growing and normal canvases with pre-conversion rejection and allocation instrumentation.
  • Extend the unexpected TIFF-header failure regression through partition_image, after one successful frame measurement, proving no partitioner/OCR/conversion runs and the original exception escapes with the borrowed stream open and restored.
  • Add a poisoned n_frames/secondary-metadata negative control for OCR-only and hi_res-to-OCR fallback to prove unused frames remain untouched. Prefer a deterministic work-count assertion alongside the forged-APNG timing bound.

Validation: complete untruncated production context, actual Pro answer with streaming tier evidence, source/dependency verification, and 53 successful checks at this exact head, including CI and security workflows. No source changes or local tests.

Integration qualification: GitHub currently reports CONFLICTING/DIRTY against newer main. This is source approval, not a claim that the branch is currently mergeable. Conflict resolution must retain main's DOCX_TABLE_MAX_CELLS and CSV_MAX_CELLS alongside IMAGE_MAX_TOTAL_PIXELS and preserve intervening release history; obtain green CI for the integrated result.

(authored by codex)

# Conflicts:
#	CHANGELOG.md
#	unstructured/__version__.py
#	unstructured/partition/utils/config.py
@badGarnet
badGarnet enabled auto-merge October 2, 2026 21:03
@badGarnet
badGarnet added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit 0242693 Oct 2, 2026
53 checks passed
@badGarnet
badGarnet deleted the fix/image-pixel-limit branch October 2, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants