Files
pdf-inspector/src
36dd5fa426 fix(structure-tree): bound recursive /K parsing with cycle detection and a node budget (#322)
* fix(structure-tree): bound tagged /K parsing against alias/cycle DoS

A struct element that references itself (or an ancestor) through /K — e.g.
/K [n 0 R n 0 R] — made parse_struct_element_dict branch exponentially:
the depth cap (64) alone still permits 2^depth materialized nodes, so a
~830-byte PDF exhausts memory (OOM, exit 134).

Add a StructWalk carrying (1) an active-path set of object IDs so a node
that references itself/an ancestor is not re-expanded (breaks self- and
mutual-reference cycles cheaply), and (2) a global node budget
(MAX_STRUCT_NODES) that caps total materialization for aliased/DAG-shaped
graphs of distinct objects the path guard cannot catch.

Adds regression tests for self-alias, mutual-alias, and the aliased-DAG
budget cap.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): charge /K content refs against the node budget

The per-node budget only covered materialized struct elements and child
recursion; bare MCIDs and MCR dicts in a /K array append to content_refs
without charging it, so one element with a very wide /K array could still
allocate content_refs without bound. Charge every /K array item before
handling it, and stop the top-level /K loop once the budget is spent, so
content refs and loop work are bounded too. Adds a wide-MCID-array test.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): charge /K budget per materialized item, not per array entry

Charging every /K array item double-counted structural children (charged
here and again at their node entry) and charged cycle-skipped references
that materialize nothing, draining the budget up to ~2x faster than the
per-node semantics and risking early truncation of large legitimate trees.
Charge only the unbounded content-ref items (bare MCIDs and MCR dicts);
structural children remain charged once at their node entry.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* refactor(structure-tree): charge every content ref uniformly via helper

Route all budget charges through StructWalk::charge() so every
marked-content reference is charged once, including the single-value /K
branches (bare integer and MCR dict) that previously appended without
charging. charge() also guards against underflow, so charging after the
node-entry charge (which can leave the budget at 0) is safe. Makes the
documented per-item budget contract hold uniformly across all branches.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* feat(structure-tree): log once when the node budget truncates parsing

Add a one-shot truncation flag on StructWalk, set the first time the
budget is exhausted, and emit a single warn! after parsing so an operator
can tell when a (very large or malformed) tagged tree was cut off. Avoids
per-item log spam; negligible overhead on the normal path.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): flag truncation at budget guards, not just in charge

The truncation flag was only set inside charge() on the budget==0 branch,
but the dominant skip paths use budget==0 guards that break/return before
charge() is ever called with an empty budget, so the flag (and the warn!)
almost never fired. Route those guards through a new exhausted() that sets
the flag when it skips remaining work. Adds a parser-level test that would
have caught the missed warning.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): flag cycle/depth skips and charge bare MCIDs fully

Two review follow-ups:
- Cycle-broken and depth-capped /K skips dropped tagged content without
  setting the truncation flag, so the one-shot warning never fired for
  malformed/over-deep trees. Mark those skips via note_skipped() and
  broaden the warning to cover non-budget truncation.
- A bare /K MCID materializes a wrapper node AND a content reference but
  charged only one budget unit, allowing ~2x the advertised budget for
  such content; charge both.

Adds tests: cycle-skip flags truncation, and bare MCID charges two units.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): charge MCR-dict wrappers the same two units as bare MCIDs

A top-level MCR /K dict flows through parse_kid -> parse_struct_element_dict
and materializes a Span node + one content ref (two items) but was charged
only one unit at node entry, while the bare-MCID path charges two. Charge
the content reference in the MCR branch too so the per-item budget is
uniform across both wrapper paths. Adds a symmetric test.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): reserve leaf-wrapper budget units atomically

A leaf MCID wrapper (bare MCID or MCR dict) materializes a node + one
content ref and charged the two units via separate charge() calls. At the
last unit the first charge succeeded and the second failed, consuming a
unit without emitting the wrapper and denying it to a later element that
would have fit. Add charge_n() to reserve both units atomically (or
neither), and detect MCR before the node charge so it reserves both up
front. Adds a boundary test asserting the leftover unit is preserved.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): stop scanning wide /K once a leaf reservation stalls

The atomic charge_n(2) left budget nonzero (==1) when it failed, so
exhausted() (budget==0) never broke the root /K loop and a crafted wide
array of leaf wrappers was scanned in full after no leaf could fit. Add a
stalled flag set on an insufficient reservation and fold it into
exhausted(); charge()-based (one-unit) loops are unaffected since they
reach budget 0 exactly. Adds a test that a one-unit budget still allows a
one-unit item but a failed two-unit reservation stops the scan.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* fix(structure-tree): add traversal budget and stop charging non-materializing dicts

Two review follow-ups on budget accounting:
- Wide /K arrays of non-materializing items (unsupported value types, OBJR
  dicts, cycle back-edges) consumed no node budget, so the loop scanned the
  whole array. Add a separate work budget charged per examined /K item and
  break the loops when it is spent, bounding traversal even when nothing
  materializes.
- OBJR dicts and dicts without a valid /S were charged the node budget before
  being recognized and skipped, draining the shared budget and truncating
  real content later. Hoist the OBJR check and /S validation above the node
  charge so only materializing nodes consume it (matching the MCR hoisting).

Adds tests for the work-budget bound, wide unsupported /K, and non-materializing
dicts not charging the node budget.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

* docs(structure-tree): mention traversal budget in truncation warning

The one-shot truncation warning listed the node budget, cycle, and depth
as causes but not the new traversal (work) budget, so a work-budget
truncation printed a misleading message. Include MAX_STRUCT_WORK so
malformed-PDF debugging identifies the actual limit hit.

Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
2026-08-10 13:36:40 -07:00
..