Compare commits

...
Author SHA1 Message Date
Cursor AgentandAbimael Martell 528829cb6c 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>
2026-08-10 20:25:32 +00:00
Cursor AgentandAbimael Martell 0a47a57147 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>
2026-08-10 20:17:11 +00:00
Cursor AgentandAbimael Martell 311ea9d9fa 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>
2026-08-10 19:46:36 +00:00
Cursor AgentandAbimael Martell 92ee85687f 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>
2026-08-10 19:32:26 +00:00
Cursor AgentandAbimael Martell fdf617409c 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>
2026-08-10 19:08:11 +00:00
Cursor AgentandAbimael Martell 8accfd2b5c 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>
2026-08-10 08:33:02 +00:00
Cursor AgentandAbimael Martell 60c73399fc 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>
2026-08-10 00:45:47 +00:00
Cursor AgentandAbimael Martell 913d41091b 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>
2026-08-10 00:10:36 +00:00
Cursor AgentandAbimael Martell c191f11007 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>
2026-08-09 15:24:49 +00:00
Cursor AgentandAbimael Martell 5b0a85b57a 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>
2026-08-09 14:59:48 +00:00
Cursor AgentandAbimael Martell 1ee5d3c2e7 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>
2026-08-09 14:48:39 +00:00
Cursor AgentandAbimael Martell 2ad08a4e28 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>
2026-08-09 07:10:32 +00:00
493fed498e fix(links): prevent stack-overflow DoS from AcroForm /Kids self-cycle (#314)
* fix(links): guard AcroForm /Kids traversal against cycles and huge trees

A crafted PDF whose AcroForm field lists itself (or another ancestor) in
/Kids caused walk_form_fields to recurse indefinitely, overflowing the
stack and aborting pdf2md (exit 134) — an application-level DoS from a
~730-byte input.

Track visited field object IDs to break /Kids cycles, and cap total
field-node traversal at 100k nodes to bound pathologically large trees.

Adds regression tests for self-cycle and mutual-cycle field graphs.

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

* fix(links): cap AcroForm /Kids recursion depth to stop deep-chain overflow

The visited-set guard stops cyclic /Kids graphs, but a long *acyclic*
chain of distinct fields still recurses to the chain length and overflows
the stack (a ~1.6MB PDF with 20k linked fields aborts pdf2md, exit 134)
before the 100k node budget is reached.

Add an explicit recursion depth cap (100 levels — far above any legitimate
form hierarchy) so stack usage is bounded independently of node count.

Adds a deep-acyclic-chain regression test.

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

* fix(links): enforce form-field node budget before insertion

The node-budget guard inserted each field ID into the visited set before
checking the budget, so the check triggered an early return but never
actually capped the set. A field with a huge /Kids array kept inserting
post-budget IDs, letting visited (memory and work) grow with the crafted
input rather than stopping at MAX_FORM_FIELD_NODES.

Check depth and budget before inserting, so visited can never exceed the
cap. Adds a wide-tree regression test.

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

* fix(links): stop /Fields and /Kids iteration once node budget is spent

Checking the budget before insertion capped the visited set, but callers
still iterated every remaining entry of a wide /Fields or /Kids array
after the budget was exhausted — each walk returned immediately, yet the
O(N) sibling iteration let a single multi-million-entry array burn
extraction CPU unbounded. Break out of both the top-level and recursive
loops once visited reaches the cap, making the budget a true
traversal-work cap. Adds a top-level wide-/Fields regression test.

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

* fix(links): charge examined entries against the field-node budget

The budget counted only distinct visited nodes, so /Fields or /Kids
arrays full of invalid (non-reference) or duplicate entries never grew
visited and ran to completion regardless of size — the node budget did
not actually cap traversal work.

Introduce FieldWalkBudget tracking both visited nodes and total entries
examined; charge every array entry (valid, invalid, or duplicate) and
stop once either hits MAX_FORM_FIELD_NODES. Adds a regression test with a
huge /Kids array of duplicate + null entries.

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

* fix(links): iterate /Fields and /Kids arrays by borrow, not clone

Both arrays were cloned in full before the budget check, so a crafted
oversized /Fields or /Kids array forced an O(n) allocation and copy
regardless of the cap. resolve_array already returns a borrow tied to the
document and the walker only needs a shared &Document, so iterate the
borrowed arrays directly — the early break now bounds how many entries
are even touched, before any per-array allocation.

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

* docs(links): correct wide-array test comments to match range assertions

The two wide-array tests assert item counts within a range near the
budget, not an exact value (charging entries in the entry guard shifts
the boundary by one or two). Fix the stale comments that claimed exact
counts.

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-08 23:43:33 -07:00
Abimael Martell 436af97038 fix(tables): exclude script attachments and tiny numeric fragments from detection (#264)
* fix(tables): exclude script attachments and tiny numeric fragments from detection

Split out of #242 (draft) so it can be reviewed on its own evidence.

Display equations with sub/superscripts form phantom small-font table
regions: the subscripts cluster with nearby small text (footnotes, axis
labels) into fake multi-column grids. Two guards:

- Script attachment: a small-font item horizontally adjacent to a
  larger-font item at a genuine baseline offset is a sub/superscript,
  not a table cell, and is excluded from candidates. A real baseline
  offset is required so a small cell beside a larger same-baseline
  label is never filtered. Attachment targets are indexed by Y and
  scanned through a bounded window rather than a full-page sweep.
  The body-font pass applies the same exclusion but only for
  heading-sized anchors (>= 1.15x base), so body-size table cells
  beside slightly larger labels are untouched.
- Tiny numeric fragments: a <=2-row grid whose every cell is a bare
  1-2 digit number carries no tabular information. Restricted to the
  small-font pass, where the pattern is overwhelmingly exponent
  clusters; body-font numeric grids are unaffected.

Corpus impact: 20 of 186 documents, measured against a control build of
main so main's own drift is excluded. Table rows fall in 17 of 18
inspected documents and no content is lost — 2103_07786 drops all 21
rows, every one a math fragment ('|X 42 43|1|||'); Stijn_SB_doc drops
173 rows of footnote text that had been shredded into cells, with word
count slightly UP and footnote markers intact. M2019_mordeste gains 11
rows from a 3-column grid re-detected as 2-column, neither clearly
better nor worse.

Note the 20 documents is far more than the 3 that per-change ablation
suggested: that figure measured sole-cause attribution inside the
original combined PR, where other heuristics changed the same files and
masked this one. Reach and sole-cause are different measurements.

805 unit + 148 integration tests pass, clippy clean, in-repo snapshots
unchanged.

* fix(tables): suppress script column evidence instead of dropping candidates

Reworked after reviewing the corpus diffs: the first approach removed
sub/superscripts from table candidates entirely, which had two failure
modes beyond the intended fix.

- Legitimate cell content was displaced. citizen-sr-282 is a calculator
  manual whose engineering-notation table lists M = 10^6, k = 10^3.
  Those exponents are superscripts, so they were dropped from the table
  and resurfaced elsewhere in the reading order ('9 mega 6 kilo = 10 3
  milli').
- Removing items changed the candidate geometry, so different spurious
  structure could form from what remained.

Scripts are now kept as candidates and excluded only from the geometry:
they cannot create a column (find_column_boundaries), cannot qualify a
region on their own (find_table_regions / _strict), but are still
assigned to cells. Column alignment is validated against ALL items
including scripts — validating only the non-script subset would let a
region manufacture alignment by ignoring its awkward items, which is
what block-diagram pages did.

Corpus: 20 of 186 documents, net -359 table rows, no content lost.
Token-level comparison shows the only text changes are merges in the
right direction: 'X' + '10' becomes 'X10', 'L' + 'g' becomes 'Lg' —
subscripts joining their base instead of floating free.

Remaining artifact: MCF5235RM (and its _nxp duplicate) gains a small
spurious table from a block-diagram label line, and M2019_mordeste
gains 11 rows from a 3-column grid re-detected as 2-column. Both are
borderline regions where the previous output was also wrong; documented
rather than tuned away.

805 unit + 148 integration tests pass, clippy clean.

* fix(tables): use a heading-anchored script mask in the body-font pass

Cubic review of #264: a single script mask computed with a 0.0 anchor
was applied to both passes, including body-font region qualification and
geometry. The body pass is supposed to require a heading-sized anchor —
that distinction existed before the geometry rework and was lost in it.

Why it matters: body-pass candidates are themselves body-sized
(0.85..1.05x base). A cell at the low end of that band, say 8.5pt,
sitting beside a 10.5pt label clears the inherent 'anchor >= 1.2x cell'
rule (10.2) and so was flagged as a script attachment. At body sizes a
slightly larger neighbour is a bold label or column header, not the base
of a superscript, so flagging it stripped real cells out of the region
evidence and column geometry and could lose the table entirely.

Two masks now: the small-font pass keeps the 0.0 anchor, the body pass
requires >= 1.15x base. Note the threshold only bites below base size —
for a cell at base, 1.2x-of-cell already exceeds 1.15x-of-base — which
is exactly the 0.85..1.0x band cubic identified.

Corpus: 20 documents, net -367 table rows (was -359 with the single
mask), so the body pass now keeps 8 rows of real table it had been
discarding. 958 tests pass, clippy clean.

* fix(markdown): reject headings that end on a relational verb

A heading candidate ending in 'equals', 'denotes', 'implies' and the
like is the first half of a sentence, not a title. This shows up when a
block dissolves and strands its lead-in ahead of the formula it
introduced — opendataloader 01030000000144 produced

    ## Note that the exact error equals
    M - Q(h) = e - 2.7525... = -0.0342....

Deliberately a very short list. Broader variants were tried and
measured, then rejected:

- Function words (of/and/for/the): a heading that WRAPS across lines
  ends on exactly those. Destroyed real IRS Publication 17 headings —
  'Casualty and' -> 'Casualty and Theft Losses', 'Rule 10. You Must Be
  at' -> '... At Least Age 25'. 52 documents affected, -619 headings.
- Copulas and auxiliaries (is/are/be/have): same failure. 'Rule 15.
  Your AGI Must Be', 'What Medical Expenses Are' and 'When Can a Roth
  IRA Be' are real wrapped headings, while 'the tax burden should be'
  is a genuine fragment. The trailing word cannot separate them; that
  needs the next line's context, which this text-only predicate lacks.

The verbs kept never end a heading in any register, so they are safe
without context. Standalone the guard is a no-op on both benchmarks
(0 documents on opendataloader, 4 on pdf-evals with no net heading
change) — its value is as a companion to the table filter in this PR,
which is what strands these lead-ins.

Combined effect on opendataloader (200 docs, vs a control build of
main), where the table filter alone regressed:

                 table filter    + this guard
    overall        -0.0003          +0.0003
    mhs            -0.0019          +0.0003
    doc ...144     -0.063           +0.053
    doc ...144 mhs -0.203           +0.028

* review: gate the dangling-verb veto on sentence case, drop 'yields'

Cubic review of a5a6e8f — both findings valid.

1. 'yields' is also a plural noun. 'Bond Yields', 'Crop Yields' and
   'Dividend Yields' are real section titles in financial documents,
   which this corpus contains. Removed from the list; my claim that
   these verbs 'never end a heading in any register' was wrong for it.

2. A wrapped title-case heading whose first line ends on one of these
   verbs would be suppressed if the heading preprocessor failed to
   merge it.

Both are fixed by the same gate, which is the discriminator I was
missing: case. A heading is title case ('Bond Yields', 'The Theorem
Implies'); a stranded lead-in is sentence case ('Note that the exact
error equals', 'the method yields'). The veto now applies only when
every content word is NOT capitalized, so titles are spared regardless
of their final word.

This is also why the earlier function-word and copula variants failed:
they had no way to tell 'Rule 15. Your AGI Must Be' from 'the tax
burden should be'. Case separates those two as well.

No measured cost. opendataloader is unchanged from the previous
revision — overall +0.0003, mhs +0.0003, doc 01030000000144 still
0.732 -> 0.785 — and pdf-evals still 20 documents. 963 tests pass,
clippy clean.

* review: exempt section-numbered lines from the dangling-verb veto

Valid ordering bug. heading.rs consults is_heading_fragment at line 282
and only applies its numbered-prefix allowance at line 288, so the veto
pre-empted it: '1. What the model implies' is sentence case and ends on
a listed verb, so it was discarded before numbering could vouch for it.

Numbering is independent evidence of a heading, so the veto now skips
any line opening with a section number.

Acceptance is deliberately a little broader than heading::parse_numbering
(which requires a trailing delimiter) because '2.3 Section Title' is
written without one, and being permissive in a veto exemption can only
avoid suppressing headings. Two guards keep it from swallowing prose:

- a bare single number needs a delimiter ('1.' yes, '3 apples' no)
- roman numerals always need one, since a leading 'I' is the pronoun far
  more often than a section number

Not reused from convert::starts_with_section_number, which deliberately
demands two components because it bypasses isolation checks — that would
reject the reviewer's single-'1.' case.

No measured change: opendataloader still overall +0.0003 / mhs +0.0003
with doc 01030000000144 at 0.732 -> 0.785, pdf-evals still 20 documents,
target case still suppressed. 964 tests pass, clippy clean.

* review: share roman_value so the veto exemption matches the parser

Valid. My numbering predicate accepted tokens heading::parse_numbering
rejects — lowercase 'iv)', alphabetical 'd)', over-long 'MMMM.' — because
it case-folded and allowed D and M. Anything the parser rejects is not
numbering, so exempting it let ordinary list items bypass the
dangling-verb veto and reach font-based heading promotion.

Rather than restate the grammar, roman_value is now pub(super) and the
exemption calls it, so the two cannot drift. Its rules apply as written:
uppercase I/V/X/L/C only, at most 8 characters, positive total.

Decimal numbering keeps its slightly broader acceptance (bare '2.3' with
no trailing delimiter), which is deliberate and documented — that form is
common in real headings and being permissive in a veto exemption cannot
manufacture a heading, only decline to suppress one. The roman case is
different because single letters collide with alphabetical list markers.

No measured change: opendataloader overall +0.0003 / mhs +0.0003, doc
01030000000144 still 0.732 -> 0.785. 964 tests pass, clippy clean.

* test: cover the roman length bound with a nine-character token

Valid P3. The 'MMMM.' case fails on the unsupported M, not on length, so
the 8-character bound in roman_value had no coverage and could regress
silently. Added a nine-'I' token, which is rejected only by the bound,
plus an eight-'I' token that must stay exempt to pin the boundary from
both sides.

* fix(tables): stop dropping body-band scripts from the candidate set

Valid: the body-font pass filtered scripts out of body_candidates
itself, so body_script_flags and its two downstream uses were dead. The
mask filters region_evidence and feeds detect_table_in_region's is_script
closure, but neither ever saw a script item because the candidate set no
longer contained any.

Consequences: a body-band sub/superscript attached to a heading-sized
anchor was dropped from the table outright rather than assigned to a
cell, so its text was lost — the opposite of what both the
body_script_flags comment ('they stay candidates') and the
detect_table_in_region docstring ('they remain eligible for cell
assignment') describe, and inconsistent with the small-font pass.

Root cause: the geometry rework removed the candidate-level filter from
the small-font pass, but the body one had been reflowed onto a single
line by rustfmt so the same edit missed it. Adding body_script_flags in
a later review then wired a mask that the surviving filter made
unreachable.

No measured change on either benchmark — pdf-evals still 20 documents
and -367 table rows, opendataloader still overall +0.0003 / mhs +0.0003
with one document changed — because the combination it affects (a
body-sized script attached to a heading-sized anchor) does not occur in
either corpus. The fix is for correctness and consistency between the
two passes, not for a score.

964 tests pass, clippy clean.
2026-08-07 09:41:46 -07:00
Abimael Martell f731e1191c fix(site): refresh benchmark results (#289) 2026-08-06 12:52:00 -07:00
7 changed files with 1522 additions and 61 deletions
+1
View File
@@ -37,6 +37,7 @@ scripts/
# Test output
test_output/
.firecrawl/
# Python
__pycache__/
+7 -7
View File
@@ -858,22 +858,22 @@
<p>Evaluated on the <a class="text-link" href="https://github.com/opendataloader-project/opendataloader-bench">opendataloader-bench</a> corpus of 200 PDFs. This comparison covers local engines without model-based PDF parsing, with OCR disabled. Higher scores are better.</p>
</div>
<div class="benchmark-card">
<div class="benchmark-top"><span><strong>200 PDFs</strong> · OpenDataLoader benchmark</span><span>Apple M4 Pro · median of 3 runs</span></div>
<div class="benchmark-top"><span><strong>200 PDFs</strong> · OpenDataLoader benchmark</span><span>Apple M4 Pro · median of 5 runs</span></div>
<div class="table-scroll">
<table aria-label="PDF extraction benchmark results">
<thead>
<tr><th>Engine</th><th>Overall</th><th>Reading order</th><th>Tables</th><th>Headings</th><th>Complete run</th></tr>
</thead>
<tbody>
<tr class="highlight"><td>pdf-inspector</td><td>0.875</td><td>0.915</td><td>0.814</td><td>0.788</td><td>2.8s</td></tr>
<tr><td>LiteParse</td><td>0.870</td><td>0.908</td><td>0.693</td><td>0.811</td><td>13.9s</td></tr>
<tr><td>OpenDataLoader</td><td>0.843</td><td>0.912</td><td>0.489</td><td>0.760</td><td>9.8s</td></tr>
<tr><td>PyMuPDF4LLM</td><td>0.735</td><td>0.886</td><td>0.401</td><td>0.424</td><td>15.5s</td></tr>
<tr><td>MarkItDown</td><td>0.583</td><td>0.879</td><td>0.000</td><td>0.000</td><td>6.7s</td></tr>
<tr class="highlight"><td>pdf-inspector</td><td>0.875</td><td>0.915</td><td>0.814</td><td>0.788</td><td>0.470s</td></tr>
<tr><td>LiteParse</td><td>0.873</td><td>0.913</td><td>0.693</td><td>0.811</td><td>0.750s</td></tr>
<tr><td>OpenDataLoader</td><td>0.831</td><td>0.902</td><td>0.489</td><td>0.739</td><td>2.569s</td></tr>
<tr><td>PyMuPDF4LLM</td><td>0.735</td><td>0.886</td><td>0.401</td><td>0.424</td><td>17.117s</td></tr>
<tr><td>MarkItDown</td><td>0.589</td><td>0.844</td><td>0.273</td><td>0.000</td><td>16.165s</td></tr>
</tbody>
</table>
</div>
<div class="benchmark-note">Refreshed July 16, 2026. Scores use the benchmarks NID, TEDS, and MHS evaluators.</div>
<div class="benchmark-note">Refreshed July 31, 2026. Scores use the benchmarks NID, TEDS, and MHS evaluators; speed is the median of five alternating or rotating complete corpus runs after an excluded warm-up. <a class="text-link" href="https://github.com/firecrawl/opendataloader-bench/tree/abi/pdf-parser-benchmark-results">Versions and raw artifacts</a>.</div>
</div>
<div class="best-fit">
<strong>Best fit</strong>
+311 -5
View File
@@ -2,11 +2,51 @@
use crate::types::{ItemType, TextItem};
use lopdf::{Document, Object, ObjectId};
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use super::fonts::{resolve_array, resolve_dict};
use super::get_number;
/// Upper bound on the number of form-field nodes visited during a single
/// `extract_form_fields` pass. A crafted PDF can chain thousands of distinct
/// `/Kids` fields to blow the stack even without an outright reference cycle,
/// so we cap total traversal work in addition to detecting cycles.
const MAX_FORM_FIELD_NODES: usize = 100_000;
/// Upper bound on `/Kids` recursion depth. Real AcroForm hierarchies are only
/// a few levels deep (fields → child fields → widgets); a crafted PDF can chain
/// tens of thousands of distinct fields into a linear `/Kids` list that would
/// overflow the stack via depth-first recursion long before the node budget is
/// reached. This depth cap bounds the stack independently of total node count.
const MAX_FORM_FIELD_DEPTH: usize = 100;
/// Traversal budget for the AcroForm field walk. Bounds both the number of
/// distinct nodes visited *and* the total number of `/Fields`/`/Kids` entries
/// examined.
///
/// Counting `visited` alone is not enough: invalid entries (non-references) and
/// duplicate references never grow `visited`, so an oversized array full of them
/// would iterate to completion no matter how large. Charging every examined
/// entry against the same budget makes it a real cap on traversal work.
pub(crate) struct FieldWalkBudget {
visited: HashSet<ObjectId>,
examined: usize,
}
impl FieldWalkBudget {
fn new() -> Self {
Self {
visited: HashSet::new(),
examined: 0,
}
}
/// True once the budget is spent; callers must stop iterating and recursing.
fn exhausted(&self) -> bool {
self.visited.len() >= MAX_FORM_FIELD_NODES || self.examined >= MAX_FORM_FIELD_NODES
}
}
pub fn extract_page_links(doc: &Document, page_id: ObjectId, page_num: u32) -> Vec<TextItem> {
let mut links = Vec::new();
@@ -146,9 +186,12 @@ pub(crate) fn extract_form_fields(
Err(_) => return items,
};
// Borrow the array rather than cloning it: a crafted `/Fields` can be huge,
// and cloning would pay an O(n) allocation/copy before the budget check
// below can stop the work.
let fields = match acroform.get(b"Fields") {
Ok(obj) => match resolve_array(doc, obj) {
Some(arr) => arr.clone(),
Some(arr) => arr,
None => return items,
},
Err(_) => return items,
@@ -158,7 +201,19 @@ pub(crate) fn extract_form_fields(
}
let annotation_pages = annotation_page_map(doc, page_map);
for field_obj in &fields {
// Bound the walk so a crafted PDF cannot send us into unbounded recursion
// via a `/Kids` cycle, a deep chain, or an oversized array of invalid or
// duplicate entries.
let mut budget = FieldWalkBudget::new();
for field_obj in fields {
// Stop once the budget is spent so a `/Fields` array wider than the
// budget can't burn CPU iterating entries whose walk would no-op. Charge
// every entry (including invalid ones) against the budget.
if budget.exhausted() {
break;
}
budget.examined += 1;
if let Ok(field_ref) = field_obj.as_reference() {
walk_form_fields(
doc,
@@ -168,6 +223,8 @@ pub(crate) fn extract_form_fields(
page_map,
&annotation_pages,
&mut items,
&mut budget,
0,
);
}
}
@@ -202,6 +259,7 @@ fn annotation_page_map(
}
/// Recursively walk the form field tree, extracting leaf field values.
#[allow(clippy::too_many_arguments)]
pub(crate) fn walk_form_fields(
doc: &Document,
field_id: ObjectId,
@@ -210,7 +268,22 @@ pub(crate) fn walk_form_fields(
page_map: &HashMap<ObjectId, u32>,
annotation_pages: &HashMap<ObjectId, u32>,
items: &mut Vec<TextItem>,
budget: &mut FieldWalkBudget,
depth: usize,
) {
// Guard against `/Kids` cycles and pathologically large field trees.
// Exceeding the depth cap means the chain is too deep to be a legitimate
// form (and would overflow the stack); an exhausted budget means the tree is
// too large. Both checks run *before* inserting so the visited set can never
// grow past the budget.
if depth > MAX_FORM_FIELD_DEPTH || budget.exhausted() {
return;
}
// Revisiting an object ID means we hit a `/Kids` cycle.
if !budget.visited.insert(field_id) {
return;
}
let field_dict = match doc.get_dictionary(field_id) {
Ok(d) => d,
Err(_) => return,
@@ -241,9 +314,19 @@ pub(crate) fn walk_form_fields(
// Check for /Kids — if present, recurse into children
if let Ok(kids_obj) = field_dict.get(b"Kids") {
// Iterate the borrowed array directly — cloning a crafted, oversized
// `/Kids` would allocate and copy every entry before the budget check
// below could stop the work.
if let Some(kids) = resolve_array(doc, kids_obj) {
let kids = kids.clone();
for kid in &kids {
for kid in kids {
// Stop once the budget is spent so a `/Kids` array wider than the
// budget can't burn CPU iterating entries whose walk would no-op.
// Charge every entry (including invalid/duplicate ones) against
// the budget so this is a true traversal-work cap.
if budget.exhausted() {
break;
}
budget.examined += 1;
if let Ok(kid_ref) = kid.as_reference() {
walk_form_fields(
doc,
@@ -253,6 +336,8 @@ pub(crate) fn walk_form_fields(
page_map,
annotation_pages,
items,
budget,
depth + 1,
);
}
}
@@ -411,4 +496,225 @@ mod tests {
assert_eq!(items[0].page, 2);
assert_eq!(items[0].text, "customer: Alice");
}
#[test]
fn kids_self_cycle_does_not_overflow_stack() {
// A crafted AcroForm field that lists itself in `/Kids` must not send
// the traversal into unbounded recursion.
let mut doc = Document::new();
let field_id = doc.new_object_id();
doc.set_object(
field_id,
dictionary! {
"FT" => "Tx",
"T" => Object::string_literal("loop"),
"Kids" => vec![Object::Reference(field_id)],
},
);
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => vec![Object::Reference(field_id)],
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
// Completes (rather than overflowing the stack) and yields no items.
let items = extract_form_fields(&doc, &page_map);
assert!(items.is_empty());
}
#[test]
fn kids_mutual_cycle_terminates() {
// Two fields that reference each other via `/Kids` form a cycle that
// must also terminate.
let mut doc = Document::new();
let field_a = doc.new_object_id();
let field_b = doc.new_object_id();
doc.set_object(
field_a,
dictionary! {
"T" => Object::string_literal("a"),
"Kids" => vec![Object::Reference(field_b)],
},
);
doc.set_object(
field_b,
dictionary! {
"T" => Object::string_literal("b"),
"Kids" => vec![Object::Reference(field_a)],
},
);
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => vec![Object::Reference(field_a)],
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
let items = extract_form_fields(&doc, &page_map);
assert!(items.is_empty());
}
#[test]
fn deep_acyclic_kids_chain_does_not_overflow_stack() {
// A long chain of *distinct* fields (no cycle) must also terminate:
// the visited set alone would still recurse to the chain length, so
// the depth cap is what prevents a stack overflow here.
let mut doc = Document::new();
let n = MAX_FORM_FIELD_DEPTH * 500;
let ids: Vec<ObjectId> = (0..=n).map(|_| doc.new_object_id()).collect();
for i in 0..n {
doc.set_object(
ids[i],
dictionary! {
"FT" => "Tx",
"Kids" => vec![Object::Reference(ids[i + 1])],
},
);
}
// Leaf carries a value; it sits far below the depth cap so it is never
// reached, proving traversal stops early rather than crashing.
doc.set_object(
ids[n],
dictionary! {
"FT" => "Tx",
"T" => Object::string_literal("leaf"),
"V" => Object::string_literal("x"),
"Rect" => vec![10.into(), 20.into(), 110.into(), 40.into()],
},
);
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => vec![Object::Reference(ids[0])],
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
let items = extract_form_fields(&doc, &page_map);
assert!(items.is_empty());
}
#[test]
fn wide_tree_traversal_stops_at_node_budget() {
// A single field with a `/Kids` array wider than the node budget must
// stop traversal at the cap rather than growing `visited` (and the work)
// without bound. Each processed leaf emits one item, so the item count
// is bounded by the budget and reaches right up to it (a couple of
// slots go to the root and the boundary node charged against the cap).
let mut doc = Document::new();
let fanout = MAX_FORM_FIELD_NODES + 50;
let leaf_ids: Vec<ObjectId> = (0..fanout).map(|_| doc.new_object_id()).collect();
for &leaf in &leaf_ids {
doc.set_object(
leaf,
dictionary! {
"FT" => "Tx",
"V" => Object::string_literal("v"),
"Rect" => vec![10.into(), 20.into(), 110.into(), 40.into()],
},
);
}
let kids: Vec<Object> = leaf_ids.iter().map(|&id| Object::Reference(id)).collect();
let root_id = doc.add_object(dictionary! {
"T" => Object::string_literal("root"),
"Kids" => kids,
});
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => vec![Object::Reference(root_id)],
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
let items = extract_form_fields(&doc, &page_map);
// Extraction stops at the budget: bounded above by the cap, and it gets
// right up to it (allowing a small delta for the root/boundary nodes
// charged against the budget).
assert!(items.len() <= MAX_FORM_FIELD_NODES);
assert!(items.len() >= MAX_FORM_FIELD_NODES - 3);
}
#[test]
fn wide_top_level_fields_stop_at_node_budget() {
// A top-level `/Fields` array wider than the budget must also stop at
// the cap: the item count is bounded by the budget and reaches right up
// to it.
let mut doc = Document::new();
let fanout = MAX_FORM_FIELD_NODES + 50;
let leaf_ids: Vec<ObjectId> = (0..fanout).map(|_| doc.new_object_id()).collect();
for &leaf in &leaf_ids {
doc.set_object(
leaf,
dictionary! {
"FT" => "Tx",
"V" => Object::string_literal("v"),
"Rect" => vec![10.into(), 20.into(), 110.into(), 40.into()],
},
);
}
let fields: Vec<Object> = leaf_ids.iter().map(|&id| Object::Reference(id)).collect();
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => fields,
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
let items = extract_form_fields(&doc, &page_map);
assert!(items.len() <= MAX_FORM_FIELD_NODES);
assert!(items.len() >= MAX_FORM_FIELD_NODES - 3);
}
#[test]
fn duplicate_and_invalid_kids_entries_stop_at_budget() {
// Duplicate references and non-reference junk never grow `visited`, so
// without charging examined entries against the budget an oversized
// array of them would iterate to completion. The walk must still
// terminate and extract the single real leaf exactly once.
let mut doc = Document::new();
let leaf_id = doc.new_object_id();
doc.set_object(
leaf_id,
dictionary! {
"FT" => "Tx",
"V" => Object::string_literal("v"),
"Rect" => vec![10.into(), 20.into(), 110.into(), 40.into()],
},
);
// A `/Kids` array far wider than the budget: half duplicate references
// to the same leaf, half invalid (null) entries.
let mut kids: Vec<Object> = Vec::new();
for i in 0..(MAX_FORM_FIELD_NODES * 2) {
if i % 2 == 0 {
kids.push(Object::Reference(leaf_id));
} else {
kids.push(Object::Null);
}
}
let root_id = doc.add_object(dictionary! {
"T" => Object::string_literal("root"),
"Kids" => kids,
});
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"AcroForm" => dictionary! {
"Fields" => vec![Object::Reference(root_id)],
},
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let page_map = HashMap::new();
let items = extract_form_fields(&doc, &page_map);
assert_eq!(items.len(), 1);
}
}
+192
View File
@@ -171,6 +171,74 @@ pub(crate) fn is_toc_marker_heading(text: &str) -> bool {
/// equation and absent from name-plus-number headings. A bare trailing colon
/// is NOT a fragment signal either: real headings frequently end with colons
/// ("Procedure:", "Steps for Using the Microscope:").
/// True when the line opens with a section number ("3.", "2.1.4", "IV)").
///
/// Mirrors the acceptance of `heading::parse_numbering` rather than the
/// stricter `convert::starts_with_section_number`, which deliberately
/// requires two components because it bypasses isolation checks. Here a
/// single "1." counts: numbering is independent evidence of a heading, and
/// `heading.rs` applies its numbered-prefix allowance *after* consulting
/// `is_heading_fragment`, so without this exemption a numbered
/// sentence-case heading would be vetoed before that allowance can run.
fn starts_with_numbering_prefix(t: &str) -> bool {
let Some(first) = t.split_whitespace().next() else {
return false;
};
let has_delimiter = first.ends_with(['.', ')', ':']);
let token = first.trim_end_matches(['.', ')', ':']);
if token.is_empty() {
return false;
}
let parts: Vec<&str> = token.split('.').collect();
let decimal = parts
.iter()
.all(|p| !p.is_empty() && p.len() <= 3 && p.chars().all(|c| c.is_ascii_digit()));
if decimal {
// "1." / "2.1." carry a delimiter; "2.3 Title" is written without
// one, so a multi-component number is accepted bare. A bare single
// number ("3 apples") is not — that is ordinary prose.
return has_delimiter || parts.len() >= 2;
}
// Roman numerals go through the heading parser's own grammar so the two
// agree: uppercase I/V/X/L/C only, at most 8 characters. A looser rule
// here would exempt markers the parser rejects — "iv)" or "d)" from an
// alphabetical list — letting an ordinary list item bypass the veto and
// reach heading promotion.
//
// A delimiter is also required: a bare leading "I" is the pronoun far
// more often than a section number.
has_delimiter && crate::markdown::heading::roman_value(token).is_some()
}
/// True when the line reads as a title rather than a sentence: every
/// content word (ignoring minor words) starts uppercase. Used to spare real
/// headings from the dangling-verb veto — "Bond Yields" is a section title,
/// "the method yields" is a stranded clause, and only the casing tells them
/// apart.
fn looks_title_case(t: &str) -> bool {
const MINOR: &[&str] = &[
"a", "an", "the", "of", "and", "or", "for", "to", "in", "on", "at", "by", "with", "from",
"as", "is", "are", "that", "than", "into",
];
let mut content = 0usize;
let mut capitalized = 0usize;
for w in t.split_whitespace() {
let cleaned: String = w.chars().filter(|c| c.is_alphabetic()).collect();
if cleaned.is_empty() {
continue;
}
if MINOR.contains(&cleaned.to_lowercase().as_str()) {
continue;
}
content += 1;
if cleaned.chars().next().is_some_and(char::is_uppercase) {
capitalized += 1;
}
}
// A single content word ("Yields") is a title by default.
content == 0 || capitalized == content
}
pub(crate) fn is_heading_fragment(text: &str) -> bool {
let t = text.trim_end();
@@ -244,9 +312,133 @@ pub(crate) fn is_heading_fragment(text: &str) -> bool {
if t.ends_with(':') && t.split_whitespace().any(is_equation_number) {
return true;
}
// Dangling clause: a stranded sentence lead-in ends on a relational
// verb with no terminal punctuation — "Note that the exact error equals"
// left ahead of its formula when a phantom table dissolved.
//
// Gated on the line reading as prose rather than a title. Case is the
// discriminator the trailing word alone cannot provide: a heading is
// title case ("Bond Yields", "The Method Yields") while a stranded
// lead-in is sentence case ("the method yields"). Without this gate the
// veto eats real headings — "Bond Yields", "Crop Yields" and any wrapped
// title-case heading the preprocessor failed to merge.
if !t.ends_with(['.', '!', '?', ':', ';', ')', ']'])
&& !looks_title_case(t)
&& !starts_with_numbering_prefix(t)
{
if let Some(last) = t.split_whitespace().next_back() {
let word: String = last
.trim_matches(|c: char| !c.is_alphanumeric())
.to_lowercase();
// Relational verbs only, and only those with no common noun
// sense. "yields" was dropped for exactly that reason: "Bond
// Yields" is a real section title. Function words, copulas and
// auxiliaries were measured and rejected outright — a heading
// that wraps across lines ends on those, and suppressing them
// destroyed real IRS Publication 17 headings.
const DANGLING_TAIL: &[&str] =
&["equals", "denotes", "implies", "satisfies", "signifies"];
if DANGLING_TAIL.contains(&word.as_str()) {
return true;
}
}
}
false
}
#[cfg(test)]
mod fragment_heading_tests {
use super::is_heading_fragment;
#[test]
fn dangling_tail_marks_stranded_clause() {
// opendataloader 01030000000144: left behind when a phantom table
// dissolved, ahead of its formula on the next line.
assert!(is_heading_fragment("Note that the exact error equals"));
assert!(is_heading_fragment("The remainder term satisfies"));
assert!(is_heading_fragment("we conclude that the sum equals"));
}
#[test]
fn real_headings_survive() {
assert!(!is_heading_fragment("Introduction"));
assert!(!is_heading_fragment("Error Analysis"));
assert!(!is_heading_fragment("Materials and Methods"));
assert!(!is_heading_fragment("Results"));
assert!(!is_heading_fragment("3.2 Richardson Extrapolation"));
assert!(!is_heading_fragment("Discussion and Conclusions"));
// Terminal punctuation means the clause is complete.
assert!(!is_heading_fragment("What is a Derivative?"));
assert!(!is_heading_fragment("Procedure:"));
assert!(!is_heading_fragment("Note that this is important."));
}
#[test]
fn title_case_headings_ending_in_a_verb_survive() {
// "yields" is also a plural noun; these are real section titles.
assert!(!is_heading_fragment("Bond Yields"));
assert!(!is_heading_fragment("Crop Yields"));
assert!(!is_heading_fragment("Dividend Yields"));
assert!(!is_heading_fragment("Yields"));
// A wrapped title-case heading whose first line ends on a listed
// verb must survive even if the preprocessor failed to merge it.
assert!(!is_heading_fragment("The Theorem Implies"));
assert!(!is_heading_fragment("What This Denotes"));
}
#[test]
fn numbered_sentence_case_headings_survive() {
// heading.rs consults is_heading_fragment BEFORE applying its
// numbered-prefix allowance, so the veto must not pre-empt it.
assert!(!is_heading_fragment("1. What the model implies"));
assert!(!is_heading_fragment("2.3 How the estimator satisfies"));
assert!(!is_heading_fragment("IV) What this denotes"));
// Without numbering the same wording is still a stranded clause.
assert!(is_heading_fragment("What the model implies"));
// A bare leading number or pronoun is prose, not numbering.
assert!(is_heading_fragment("3 apples and what that implies"));
assert!(is_heading_fragment("I think the model implies"));
// Markers heading::parse_numbering rejects must not be exempted
// either, or an ordinary list item bypasses the veto: lowercase
// roman, alphabetical markers, and over-long tokens.
assert!(is_heading_fragment("iv) the estimator satisfies"));
assert!(is_heading_fragment("d) the value implies"));
// Unsupported character (M is outside the parser's I/V/X/L/C set).
assert!(is_heading_fragment("MMMM. the value implies"));
// Over-long token: nine valid characters, so this exercises the
// 8-character bound rather than the character set.
assert!(is_heading_fragment("IIIIIIIII. the value implies"));
// Eight is still within the bound and stays exempt.
assert!(!is_heading_fragment("IIIIIIII. What this implies"));
// Uppercase roman within the parser's grammar is still exempt.
assert!(!is_heading_fragment("IV. What this denotes"));
assert!(!is_heading_fragment("XII) What this implies"));
}
#[test]
fn wrapped_headings_are_not_fragments() {
// A heading that wraps across lines ends on a function word. These
// are real headings from IRS Publication 17 and must survive.
assert!(!is_heading_fragment("Casualty and"));
assert!(!is_heading_fragment("Rule 10. You Must Be at"));
assert!(!is_heading_fragment("Higher Standard Deduction for"));
assert!(!is_heading_fragment("Qualifying Child of"));
assert!(!is_heading_fragment("When Can I Withdraw or"));
// Copulas and auxiliaries also end real wrapped headings.
assert!(!is_heading_fragment("Rule 15. Your AGI Must Be"));
assert!(!is_heading_fragment("What Medical Expenses Are"));
assert!(!is_heading_fragment("Rule 13. You Must Have"));
assert!(!is_heading_fragment("When Can a Roth IRA Be"));
}
#[test]
fn dangling_check_is_case_insensitive() {
// All-caps is not sentence case, so the veto must not fire there.
assert!(!is_heading_fragment("THE REMAINDER EQUALS"));
}
}
/// Compute the Y-gap threshold for paragraph break detection.
///
/// Instead of using a fixed multiple of base_size (which fails for double-spaced
+3 -1
View File
@@ -127,7 +127,9 @@ fn visual_style(line: &TextLine) -> Option<VisualStyle> {
})
}
fn roman_value(token: &str) -> Option<u32> {
/// Shared with `analysis::starts_with_numbering_prefix` so the veto
/// exemption and the heading parser agree on what a roman numeral is.
pub(super) fn roman_value(token: &str) -> Option<u32> {
if token.is_empty() || token.len() > 8 {
return None;
}
+700 -38
View File
@@ -9,7 +9,7 @@
use log::debug;
use lopdf::{Document, Object, ObjectId};
use std::borrow::Cow;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
// ─── Standard structure types ────────────────────────────────────────
@@ -252,10 +252,28 @@ impl StructTree {
let role_map = parse_role_map(doc, struct_root);
debug!("structure tree: {} role map entries", role_map.len());
// Seed the cycle guard with the struct-root's own object id so a `/K`
// that points back at the root is treated as a cycle, and bound total
// node materialization with a global budget.
let mut walk = StructWalk::new();
if let Ok(root_id) = struct_root_obj.as_reference() {
walk.active.insert(root_id);
}
// Parse child elements from /K
let children = parse_kids(doc, struct_root, &role_map, None, 0);
let children = parse_kids(doc, struct_root, &role_map, None, 0, &mut walk);
debug!("structure tree: {} top-level elements", children.len());
if walk.truncated {
log::warn!(
"structure tree parsing was truncated (node budget of \
{MAX_STRUCT_NODES} or traversal budget of {MAX_STRUCT_WORK} \
reached, a `/K` reference cycle, or the max nesting depth of \
{MAX_DEPTH}); tagged roles/tables may be incomplete (likely a \
very large or malformed tagged PDF)"
);
}
if children.is_empty() {
return None;
}
@@ -479,6 +497,123 @@ fn parse_role_map(doc: &Document, struct_root: &lopdf::Dictionary) -> HashMap<St
/// malformed PDFs).
const MAX_DEPTH: usize = 64;
/// Global cap on the number of structure-tree nodes materialized in a single
/// parse. Real tagged trees are far smaller; a crafted PDF can alias one struct
/// element into its own `/K` (e.g. `/K [n 0 R n 0 R]`) so the tree branches
/// exponentially (2^depth) before the depth cap is reached, exhausting memory.
/// This budget bounds total work and allocation regardless of tree shape.
const MAX_STRUCT_NODES: usize = 500_000;
/// Cap on the number of `/K` items *examined* during a single parse, regardless
/// of whether they materialize anything. Bounds CPU for crafted wide `/K` arrays
/// of non-materializing entries (unsupported value types, `/OBJR` dicts, cycle
/// back-edges) that would otherwise be scanned in full without ever touching the
/// node budget. Kept well above the node budget so it never truncates content
/// that already fits within `MAX_STRUCT_NODES`.
const MAX_STRUCT_WORK: usize = 2_000_000;
/// Traversal state shared across the recursive structure-tree parse.
///
/// `budget` is a global allowance charged once per materialized item — each
/// struct-element node and each marked-content reference — so total work is
/// bounded even for aliased/DAG-shaped `/K` graphs of distinct objects or a
/// single element with a very wide `/K` array. `active` holds the object IDs
/// currently on the depth-first path so a struct element that references itself
/// (or an ancestor) is not expanded into an unbounded/exponential subtree.
/// `budget` bounds *materialization* (nodes + content refs). `work` separately
/// bounds *traversal* — every `/K` item examined is charged against it, even
/// ones that materialize nothing (unsupported values, `/OBJR`, cycle back-edges)
/// — so a wide malformed array cannot force an unbounded scan, and those skipped
/// items don't drain the materialization budget and truncate real content.
/// `truncated` records whether any parse work was skipped — the budget was
/// exhausted, a `/K` reference cycle was broken, or the depth cap was hit — so
/// the caller can log it once rather than per skipped item. `stalled` is set
/// when an atomic multi-unit reservation could not fit in the remaining budget;
/// it makes [`exhausted`](Self::exhausted) report done so a wide `/K` array is
/// not scanned to the end once no further leaf can be materialized.
struct StructWalk {
budget: usize,
work: usize,
active: HashSet<ObjectId>,
truncated: bool,
stalled: bool,
}
impl StructWalk {
fn new() -> Self {
Self {
budget: MAX_STRUCT_NODES,
work: MAX_STRUCT_WORK,
active: HashSet::new(),
truncated: false,
stalled: false,
}
}
/// Charge one unit of traversal work for an examined `/K` item, whether or
/// not it materializes anything. Returns `false` (flagging truncation) once
/// the traversal budget is spent, so an enclosing loop stops instead of
/// scanning the rest of a wide array of non-materializing entries.
fn spend_work(&mut self) -> bool {
if self.work == 0 {
self.truncated = true;
return false;
}
self.work -= 1;
true
}
/// Record that some parse work was skipped for a non-budget reason (a `/K`
/// reference cycle or the depth cap), so the one-shot truncation warning
/// also covers malformed/over-deep trees, not just budget exhaustion.
fn note_skipped(&mut self) {
self.truncated = true;
}
/// Charge one unit against the budget for a materialized item (a struct
/// element node or a marked-content reference). Returns `false` — without
/// underflowing — once the budget is exhausted, so callers skip the item.
fn charge(&mut self) -> bool {
if self.budget == 0 {
self.truncated = true;
return false;
}
self.budget -= 1;
true
}
/// Atomically charge `n` units for a single item that materializes several
/// budget-counted parts at once (a leaf wrapper node *plus* its content
/// reference). Charges nothing when fewer than `n` units remain — so a
/// partial reservation never wastes capacity — and marks the walk `stalled`
/// so the enclosing loop stops instead of scanning the rest of a wide `/K`
/// array that can no longer fit any leaf.
fn charge_n(&mut self, n: usize) -> bool {
if self.budget < n {
self.truncated = true;
self.stalled = true;
return false;
}
self.budget -= n;
true
}
/// Whether traversal should stop: the budget is spent, or a multi-unit
/// reservation could not fit (`stalled`) so no further leaf will materialize.
/// Use this at the guards that break/return to skip remaining items; it
/// records that truncation occurred (a guard only fires while an item is
/// still pending), so callers that drop work without going through
/// [`charge`](Self::charge) still flag the truncation for logging.
fn exhausted(&mut self) -> bool {
if self.budget == 0 || self.stalled {
self.truncated = true;
true
} else {
false
}
}
}
/// Parse child elements from a `/K` entry.
fn parse_kids(
doc: &Document,
@@ -486,8 +621,13 @@ fn parse_kids(
role_map: &HashMap<String, String>,
inherited_page: Option<ObjectId>,
depth: usize,
walk: &mut StructWalk,
) -> Vec<StructElement> {
if depth >= MAX_DEPTH {
walk.note_skipped();
return Vec::new();
}
if walk.exhausted() {
return Vec::new();
}
@@ -498,22 +638,59 @@ fn parse_kids(
// /Pg on this element (inherited by children)
let page_id = get_page_ref(doc, dict).or(inherited_page);
let mut children = Vec::new();
match k_obj {
Object::Array(arr) => {
let mut children = Vec::new();
for item in arr {
let resolved = resolve_obj(doc, item);
parse_kid(doc, resolved, role_map, page_id, depth, &mut children);
if walk.exhausted() || !walk.spend_work() {
break;
}
process_kid_item(doc, item, role_map, page_id, depth, &mut children, walk);
}
children
}
other => {
let resolved = resolve_obj(doc, other);
let mut children = Vec::new();
parse_kid(doc, resolved, role_map, page_id, depth, &mut children);
children
process_kid_item(doc, other, role_map, page_id, depth, &mut children, walk);
}
}
children
}
/// Resolve one `/K` array item (following at most one level of indirection),
/// guarding against reference cycles and the global node budget, then dispatch
/// it via [`parse_kid`].
fn process_kid_item(
doc: &Document,
item: &Object,
role_map: &HashMap<String, String>,
inherited_page: Option<ObjectId>,
depth: usize,
out: &mut Vec<StructElement>,
walk: &mut StructWalk,
) {
if walk.exhausted() {
return;
}
if depth >= MAX_DEPTH {
walk.note_skipped();
return;
}
// If this child is an indirect reference, track its id on the active path so
// a self/ancestor reference is not expanded into an exponential subtree.
let ref_id = match item {
Object::Reference(id) => Some(*id),
_ => None,
};
if let Some(id) = ref_id {
if !walk.active.insert(id) {
walk.note_skipped();
return; // cycle: this object is already on the current path
}
}
let resolved = resolve_obj(doc, item);
parse_kid(doc, resolved, role_map, inherited_page, depth, out, walk);
if let Some(id) = ref_id {
walk.active.remove(&id);
}
}
/// Parse a single child (either a struct element dict or an MCID integer).
@@ -524,10 +701,16 @@ fn parse_kid(
inherited_page: Option<ObjectId>,
depth: usize,
out: &mut Vec<StructElement>,
walk: &mut StructWalk,
) {
match obj {
// Direct MCID integer — create a leaf wrapper
Object::Integer(mcid) => {
// A wrapper node plus its content reference — two items — reserved
// atomically so we never consume one unit without emitting both.
if !walk.charge_n(2) {
return;
}
// This is a bare MCID at the struct-element level.
// We attach it to the parent element, so we create a wrapper struct element.
// Actually, bare MCIDs inside /K are content refs for the parent,
@@ -546,11 +729,11 @@ fn parse_kid(
});
}
Object::Dictionary(d) => {
parse_struct_element_dict(doc, d, role_map, inherited_page, depth, out);
parse_struct_element_dict(doc, d, role_map, inherited_page, depth, out, walk);
}
Object::Stream(s) => {
// Some PDFs wrap struct elements in streams (rare)
parse_struct_element_dict(doc, &s.dict, role_map, inherited_page, depth, out);
parse_struct_element_dict(doc, &s.dict, role_map, inherited_page, depth, out, walk);
}
_ => {}
}
@@ -565,13 +748,23 @@ fn parse_struct_element_dict(
inherited_page: Option<ObjectId>,
depth: usize,
out: &mut Vec<StructElement>,
walk: &mut StructWalk,
) {
if depth >= MAX_DEPTH {
walk.note_skipped();
return;
}
// Check if this is a marked-content reference dict (has /Type /MCR)
// A marked-content reference dict materializes a wrapper node + one content
// reference (two items). Reserve both atomically *before* the node charge so
// we never consume a unit without emitting the reference — which would also
// deny that unit to a later element that would have fit. This matches the
// bare-MCID path.
if is_mcr_dict(dict) {
if let Ok(Object::Integer(mcid)) = dict.get(b"MCID") {
if !walk.charge_n(2) {
return;
}
let page_id = get_page_ref(doc, dict).or(inherited_page);
out.push(StructElement {
role: StructRole::Span,
@@ -588,12 +781,15 @@ fn parse_struct_element_dict(
return;
}
// Check if this is an object reference dict (has /Type /OBJR) — skip these
// Skip object-reference dicts (`/Type /OBJR`) — they materialize no node, so
// recognize and return *before* charging the budget (otherwise a document
// full of OBJRs would drain the shared budget and truncate real content).
if is_objr_dict(dict) {
return;
}
// It's a struct element — parse its /S (structure type)
// It's a struct element — parse its /S (structure type). A dict without a
// valid /S also materializes nothing, so validate before charging.
let role_name = match dict.get(b"S") {
Ok(s_obj) => {
let resolved = resolve_obj(doc, s_obj);
@@ -605,6 +801,12 @@ fn parse_struct_element_dict(
Err(_) => return,
};
// Charge the node only now that we know it will materialize (bounds
// aliased/DAG-shaped `/K` graphs the per-path cycle guard alone cannot stop).
if !walk.charge() {
return;
}
let role = StructRole::from_name_with_role_map(&role_name, role_map);
let page_id = get_page_ref(doc, dict).or(inherited_page);
@@ -621,51 +823,73 @@ fn parse_struct_element_dict(
let k_resolved = resolve_obj(doc, k_obj);
match k_resolved {
Object::Integer(mcid) => {
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id,
});
if walk.charge() {
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id,
});
}
}
Object::Array(arr) => {
for item in arr {
if walk.exhausted() || !walk.spend_work() {
break;
}
// Only content-ref items (bare MCIDs / MCR dicts) are charged
// here — those are the unbounded allocations. Structural
// children are charged once at their own node entry in the
// recursive call, so charging them here too would double-count
// and drain the budget ~2× faster than the per-node semantics.
let ref_id = match item {
Object::Reference(id) => Some(*id),
_ => None,
};
let resolved = resolve_obj(doc, item);
match resolved {
Object::Integer(mcid) => {
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id,
});
if walk.charge() {
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id,
});
}
}
Object::Dictionary(d) => {
if is_mcr_dict(d) {
if let Ok(Object::Integer(mcid)) = d.get(b"MCID") {
let pg = get_page_ref(doc, d).or(page_id);
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id: pg,
});
if walk.charge() {
let pg = get_page_ref(doc, d).or(page_id);
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id: pg,
});
}
}
} else if is_objr_dict(d) {
// Skip object references
} else {
parse_struct_element_dict(
recurse_struct_child(
doc,
ref_id,
d,
role_map,
page_id,
depth + 1,
depth,
&mut children,
walk,
);
}
}
Object::Stream(s) => {
parse_struct_element_dict(
recurse_struct_child(
doc,
ref_id,
&s.dict,
role_map,
page_id,
depth + 1,
depth,
&mut children,
walk,
);
}
_ => {}
@@ -675,14 +899,29 @@ fn parse_struct_element_dict(
Object::Dictionary(d) => {
if is_mcr_dict(d) {
if let Ok(Object::Integer(mcid)) = d.get(b"MCID") {
let pg = get_page_ref(doc, d).or(page_id);
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id: pg,
});
if walk.charge() {
let pg = get_page_ref(doc, d).or(page_id);
content_refs.push(MarkedContentRef {
mcid: *mcid,
page_id: pg,
});
}
}
} else {
parse_struct_element_dict(doc, d, role_map, page_id, depth + 1, &mut children);
let ref_id = match k_obj {
Object::Reference(id) => Some(*id),
_ => None,
};
recurse_struct_child(
doc,
ref_id,
d,
role_map,
page_id,
depth,
&mut children,
walk,
);
}
}
_ => {}
@@ -699,6 +938,37 @@ fn parse_struct_element_dict(
});
}
/// Recurse into a child struct-element dictionary, guarding against reference
/// cycles (via the active-path object-id set) and the global node budget.
///
/// `ref_id` is the object id of the child when it was reached through an
/// indirect reference (`None` for an inline dictionary, which cannot alias).
#[allow(clippy::too_many_arguments)]
fn recurse_struct_child(
doc: &Document,
ref_id: Option<ObjectId>,
dict: &lopdf::Dictionary,
role_map: &HashMap<String, String>,
inherited_page: Option<ObjectId>,
depth: usize,
out: &mut Vec<StructElement>,
walk: &mut StructWalk,
) {
if walk.exhausted() {
return;
}
if let Some(id) = ref_id {
if !walk.active.insert(id) {
walk.note_skipped();
return; // cycle: this object is already on the current path
}
}
parse_struct_element_dict(doc, dict, role_map, inherited_page, depth + 1, out, walk);
if let Some(id) = ref_id {
walk.active.remove(&id);
}
}
/// Check if dict has `/Type /MCR`.
fn is_mcr_dict(dict: &lopdf::Dictionary) -> bool {
dict.get(b"Type")
@@ -901,6 +1171,7 @@ fn contains_bytes(haystack: &[u8], needle: &[u8]) -> bool {
#[cfg(test)]
mod tests {
use super::*;
use lopdf::dictionary;
#[test]
fn non_heading_content_roles() {
@@ -1231,4 +1502,395 @@ mod tests {
let role_map = tree.mcid_to_roles(&page_ids);
assert!(!role_map.is_empty(), "Should have MCID→role mappings");
}
fn count_nodes(elems: &[StructElement]) -> usize {
elems.iter().map(|e| 1 + count_nodes(&e.children)).sum()
}
/// Wrap already-created struct elements under a `/StructTreeRoot` and
/// `/Catalog`, returning a document ready for [`StructTree::from_doc`].
/// `root_kid` is the top-level element the root's `/K` points at.
fn finalize_tagged_doc(mut doc: Document, root_kid: ObjectId) -> Document {
let root_id = doc.add_object(dictionary! {
"Type" => "StructTreeRoot",
"K" => vec![Object::Reference(root_kid)],
});
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"StructTreeRoot" => Object::Reference(root_id),
});
doc.trailer.set("Root", Object::Reference(catalog_id));
doc
}
#[test]
fn struct_tree_self_alias_kids_terminates() {
// A struct element that lists itself twice in `/K` (`/K [n 0 R n 0 R]`)
// must not expand into an exponential tree.
let mut doc = Document::new();
let elem = doc.new_object_id();
doc.set_object(
elem,
dictionary! {
"Type" => "StructElem",
"S" => "Div",
"K" => vec![Object::Reference(elem), Object::Reference(elem)],
},
);
let doc = finalize_tagged_doc(doc, elem);
let tree = StructTree::from_doc(&doc).expect("tree should parse");
let n = count_nodes(&tree.children);
assert!(
n < 10,
"self-alias must not explode; materialized {n} nodes"
);
}
#[test]
fn struct_tree_mutual_alias_kids_terminates() {
// A → B → A cycle via `/K` must terminate.
let mut doc = Document::new();
let a = doc.new_object_id();
let b = doc.new_object_id();
doc.set_object(
a,
dictionary! {
"Type" => "StructElem",
"S" => "Div",
"K" => vec![Object::Reference(b), Object::Reference(b)],
},
);
doc.set_object(
b,
dictionary! {
"Type" => "StructElem",
"S" => "Div",
"K" => vec![Object::Reference(a), Object::Reference(a)],
},
);
let doc = finalize_tagged_doc(doc, a);
let tree = StructTree::from_doc(&doc).expect("tree should parse");
let n = count_nodes(&tree.children);
assert!(
n < 100,
"mutual alias must terminate small; materialized {n} nodes"
);
}
#[test]
fn struct_tree_aliased_dag_respects_node_budget() {
// Distinct elements, each aliased twice in the next level's `/K`, form a
// DAG that would expand to 2^depth nodes (the per-path cycle guard does
// not catch this since every id is on the path only once). The global
// node budget must cap total materialization.
let mut doc = Document::new();
let levels = 22; // 2^22 ≈ 4.2M unbounded, well past the budget
let ids: Vec<ObjectId> = (0..=levels).map(|_| doc.new_object_id()).collect();
for i in 0..levels {
doc.set_object(
ids[i],
dictionary! {
"Type" => "StructElem",
"S" => "Div",
"K" => vec![Object::Reference(ids[i + 1]), Object::Reference(ids[i + 1])],
},
);
}
doc.set_object(
ids[levels],
dictionary! { "Type" => "StructElem", "S" => "P" },
);
let root_id = doc.add_object(dictionary! {
"Type" => "StructTreeRoot",
"K" => vec![Object::Reference(ids[0])],
});
let catalog_id = doc.add_object(dictionary! {
"Type" => "Catalog",
"StructTreeRoot" => Object::Reference(root_id),
});
doc.trailer.set("Root", Object::Reference(catalog_id));
let tree = StructTree::from_doc(&doc).expect("tree should parse");
let n = count_nodes(&tree.children);
assert!(
n <= MAX_STRUCT_NODES,
"node count {n} exceeded budget {MAX_STRUCT_NODES}"
);
}
#[test]
fn struct_tree_wide_mcid_array_respects_budget() {
// A single struct element with a `/K` array of bare MCIDs wider than the
// budget must not allocate `content_refs` without bound — each array item
// is charged, so materialized marked-content refs stay within the budget.
let mut doc = Document::new();
let elem = doc.new_object_id();
let kids: Vec<Object> = (0..(MAX_STRUCT_NODES as i64 + 100))
.map(Object::Integer)
.collect();
doc.set_object(
elem,
dictionary! {
"Type" => "StructElem",
"S" => "P",
"K" => kids,
},
);
let doc = finalize_tagged_doc(doc, elem);
let tree = StructTree::from_doc(&doc).expect("tree should parse");
assert!(
tree.mcid_count() <= MAX_STRUCT_NODES,
"content_refs unbounded: {} > {MAX_STRUCT_NODES}",
tree.mcid_count()
);
}
#[test]
fn budget_charge_flags_truncation_once_exhausted() {
let mut walk = StructWalk::new();
walk.budget = 1;
assert!(walk.charge(), "should spend the last unit");
assert!(!walk.truncated, "not truncated while budget remained");
assert!(!walk.charge(), "budget exhausted");
assert!(walk.truncated, "exhaustion must set the truncation flag");
// Stays exhausted/flagged on subsequent calls.
assert!(!walk.charge());
assert!(walk.truncated);
}
#[test]
fn exhausted_flags_truncation_after_budget_spent_by_charge() {
// The dominant truncation path: the budget is driven to 0 by a
// successful `charge()` (which does not set the flag), and remaining
// items are then dropped by an `exhausted()` guard — which must flag it.
let mut walk = StructWalk::new();
walk.budget = 1;
assert!(walk.charge());
assert!(
!walk.truncated,
"spending the last unit is not truncation yet"
);
assert!(walk.exhausted(), "budget is now spent");
assert!(
walk.truncated,
"the guard that skips work must flag truncation"
);
}
#[test]
fn wide_kids_array_flags_truncation_via_parser() {
// Reproduce the reviewer's scenario through the real parser: a `/K`
// array wider than the budget drives the budget to 0 via `charge()`,
// then the loop guard drops the rest — the truncation flag must be set
// (so `from_doc` logs it) rather than staying silently false.
let mut doc = Document::new();
let elem = doc.new_object_id();
let kids: Vec<Object> = (0..20i64).map(Object::Integer).collect();
doc.set_object(
elem,
dictionary! { "Type" => "StructElem", "S" => "P", "K" => kids },
);
let dict = doc.get_dictionary(elem).unwrap().clone();
let mut walk = StructWalk::new();
walk.budget = 5; // smaller than the 20-item `/K` array
let role_map = HashMap::new();
let mut out = Vec::new();
parse_struct_element_dict(&doc, &dict, &role_map, None, 0, &mut out, &mut walk);
assert!(
walk.truncated,
"a `/K` array wider than the budget must flag truncation"
);
}
#[test]
fn cycle_skip_flags_truncation() {
// A `/K` reference cycle is dropped rather than expanded; that skip must
// still flag truncation so the one-shot warning fires for malformed
// trees, not only for budget exhaustion.
let mut doc = Document::new();
let elem = doc.new_object_id();
doc.set_object(
elem,
dictionary! {
"Type" => "StructElem",
"S" => "Div",
"K" => vec![Object::Reference(elem), Object::Reference(elem)],
},
);
let dict = doc.get_dictionary(elem).unwrap().clone();
let mut walk = StructWalk::new();
walk.active.insert(elem); // simulate `elem` already on the DFS path
let role_map = HashMap::new();
let mut out = Vec::new();
parse_struct_element_dict(&doc, &dict, &role_map, None, 0, &mut out, &mut walk);
assert!(
walk.truncated,
"a cycle-skipped `/K` child must flag truncation"
);
}
#[test]
fn bare_mcid_charges_node_and_reference() {
// A bare MCID `/K` child becomes a wrapper node carrying one content
// reference — two materialized items — so it must charge two budget
// units, not one.
let doc = Document::new();
let obj = Object::Integer(7);
let role_map = HashMap::new();
let mut out = Vec::new();
let mut walk = StructWalk::new();
let before = walk.budget;
parse_kid(&doc, &obj, &role_map, None, 0, &mut out, &mut walk);
assert_eq!(
out.len(),
1,
"bare MCID should materialize one wrapper node"
);
assert_eq!(
before - walk.budget,
2,
"bare MCID must charge for both the node and its content reference"
);
}
#[test]
fn mcr_dict_charges_node_and_reference() {
// A top-level MCR `/K` dict materializes the same wrapper node + content
// reference as a bare MCID, so it must charge the same two budget units
// (not one), keeping the per-item budgeting uniform.
let doc = Document::new();
let obj = Object::Dictionary(dictionary! { "Type" => "MCR", "MCID" => 3 });
let role_map = HashMap::new();
let mut out = Vec::new();
let mut walk = StructWalk::new();
let before = walk.budget;
parse_kid(&doc, &obj, &role_map, None, 0, &mut out, &mut walk);
assert_eq!(out.len(), 1, "MCR dict should materialize one wrapper node");
assert_eq!(
before - walk.budget,
2,
"MCR dict must charge for both the node and its content reference"
);
}
#[test]
fn leaf_wrappers_reserve_both_units_atomically() {
// With only one unit left, a two-item leaf wrapper (bare MCID or MCR
// dict) must consume nothing and flag truncation, leaving the unit for a
// later single-item element instead of half-charging.
let doc = Document::new();
let role_map = HashMap::new();
// Bare MCID via parse_kid.
let mut walk = StructWalk::new();
walk.budget = 1;
let mut out = Vec::new();
parse_kid(
&doc,
&Object::Integer(5),
&role_map,
None,
0,
&mut out,
&mut walk,
);
assert!(out.is_empty(), "bare MCID must not partially materialize");
assert_eq!(walk.budget, 1, "the leftover unit must be preserved");
assert!(walk.truncated);
// MCR dict via parse_struct_element_dict.
let mcr = dictionary! { "Type" => "MCR", "MCID" => 1 };
let mut walk = StructWalk::new();
walk.budget = 1;
let mut out = Vec::new();
parse_struct_element_dict(&doc, &mcr, &role_map, None, 0, &mut out, &mut walk);
assert!(out.is_empty(), "MCR dict must not partially materialize");
assert_eq!(walk.budget, 1, "the leftover unit must be preserved");
assert!(walk.truncated);
}
#[test]
fn insufficient_reservation_stops_the_scan() {
// A one-unit budget is not "exhausted" for a one-unit item, but once a
// two-unit leaf reservation fails, the walk is stalled so enclosing `/K`
// loops stop instead of scanning the rest of a wide array.
let mut walk = StructWalk::new();
walk.budget = 1;
assert!(
!walk.exhausted(),
"one unit left must still allow a one-unit item"
);
assert!(!walk.charge_n(2), "cannot reserve two units from one");
assert!(
walk.exhausted(),
"an insufficient reservation must stop the loop"
);
assert!(walk.truncated);
}
#[test]
fn work_budget_bounds_examined_items() {
let mut walk = StructWalk::new();
walk.work = 2;
assert!(walk.spend_work());
assert!(walk.spend_work());
assert!(!walk.spend_work(), "traversal budget exhausted");
assert!(walk.truncated);
}
#[test]
fn wide_unsupported_kids_stop_at_work_budget() {
// A wide `/K` array of unsupported values (nulls) materializes nothing;
// it must stop at the traversal budget instead of scanning every entry.
let mut doc = Document::new();
let elem = doc.new_object_id();
let kids: Vec<Object> = (0..1000).map(|_| Object::Null).collect();
doc.set_object(
elem,
dictionary! { "Type" => "StructElem", "S" => "P", "K" => kids },
);
let dict = doc.get_dictionary(elem).unwrap().clone();
let mut walk = StructWalk::new();
walk.work = 10; // far smaller than the 1000-entry array
let role_map = HashMap::new();
let mut out = Vec::new();
parse_struct_element_dict(&doc, &dict, &role_map, None, 0, &mut out, &mut walk);
assert!(
walk.truncated,
"a wide unsupported `/K` array must hit the work budget"
);
}
#[test]
fn non_materializing_dicts_do_not_charge_node_budget() {
let doc = Document::new();
let role_map = HashMap::new();
// OBJR dict: materializes no node, so it must not spend the node budget.
let objr = dictionary! { "Type" => "OBJR" };
let mut walk = StructWalk::new();
let before = walk.budget;
let mut out = Vec::new();
parse_struct_element_dict(&doc, &objr, &role_map, None, 0, &mut out, &mut walk);
assert!(out.is_empty());
assert_eq!(walk.budget, before, "OBJR must not spend the node budget");
// A struct dict without a valid /S also materializes nothing.
let no_s = dictionary! { "Type" => "StructElem" };
let mut walk = StructWalk::new();
let before = walk.budget;
let mut out = Vec::new();
parse_struct_element_dict(&doc, &no_s, &role_map, None, 0, &mut out, &mut walk);
assert!(out.is_empty());
assert_eq!(
walk.budget, before,
"a dict without /S must not spend the node budget"
);
}
}
+308 -10
View File
@@ -435,6 +435,72 @@ fn revised_table_cell_indices(
.collect()
}
/// Index of candidate "body" items (larger-font attachment targets) sorted by
/// Y, so script-attachment checks scan a narrow Y window instead of the whole
/// page per candidate.
struct ScriptBodyIndex<'a> {
/// (y, item), sorted ascending by y
by_y: Vec<(f32, &'a TextItem)>,
/// widest vertical attachment window any body item can produce
max_window: f32,
}
impl<'a> ScriptBodyIndex<'a> {
fn new(items: &'a [TextItem]) -> Self {
// Smallest table-candidate font is 6pt, so any possible attachment
// target is at least 6 x 1.2 pt.
let mut by_y: Vec<(f32, &TextItem)> = items
.iter()
.filter(|i| i.font_size >= 6.0 * 1.2)
.map(|i| (i.y, i))
.collect();
by_y.sort_by(|a, b| a.0.total_cmp(&b.0));
let max_window = by_y
.iter()
.map(|(_, i)| i.font_size * 0.8)
.fold(0.0f32, f32::max);
Self { by_y, max_window }
}
/// True when a small-font item is horizontally attached to a larger-font
/// item at a script baseline offset — a sub/superscript in running text
/// or math (equation subscripts, footnote markers). Script attachments
/// are not table cells; without this filter, display equations with
/// sub/superscripts form phantom small-font table regions (e.g. TeX
/// papers where log subscripts cluster with footnote lines into a fake
/// 3-column table). A genuine baseline offset is required so same-line
/// table neighbours (a small cell beside a larger label cell) are never
/// classified as scripts.
///
/// `min_anchor_size` additionally constrains what counts as an
/// attachment target: the small-font pass accepts any sufficiently
/// larger item (0.0), while the body-font pass requires a heading-sized
/// anchor so a body-size table cell beside a slightly larger label with
/// baseline jitter is never treated as a script.
fn is_script_attachment(&self, small: &TextItem, min_anchor_size: f32) -> bool {
let attach_gap = small.font_size.max(4.0) * 0.6;
let lo = self
.by_y
.partition_point(|(y, _)| *y < small.y - self.max_window);
self.by_y[lo..]
.iter()
.take_while(|(y, _)| *y <= small.y + self.max_window)
.any(|(_, body)| {
let dy = (small.y - body.y).abs();
body.font_size >= small.font_size * 1.2
&& body.font_size >= min_anchor_size
&& dy > body.font_size * 0.05
&& dy <= body.font_size * 0.8
&& {
let gap_after_body = small.x - (body.x + body.width);
let gap_before_body = body.x - (small.x + small.width);
(-attach_gap..=attach_gap).contains(&gap_after_body)
|| (-attach_gap..=attach_gap).contains(&gap_before_body)
}
})
}
}
/// Detect tables in a set of text items from a single page
pub fn detect_tables(items: &[TextItem], base_font_size: f32, skip_body_font: bool) -> Vec<Table> {
detect_tables_with_page_width(items, base_font_size, skip_body_font, content_width(items))
@@ -483,6 +549,27 @@ pub(crate) fn detect_tables_with_page_width(
// === Pass 1: Small-font tables (existing behavior) ===
let table_font_threshold = base_font_size * 0.90;
// Mark sub/superscript attachments once per pass. They stay candidates —
// the masks only remove them from region qualification and column/row
// geometry.
//
// The two passes need different anchor thresholds. In the small-font pass
// any sufficiently larger neighbour is a plausible base for a script. In
// the body-font pass the candidates are themselves body-sized
// (0.85..1.05x), so a merely "slightly larger" neighbour is usually a bold
// label or an adjacent column header, not the base of a superscript —
// treating it as one would strip real cells out of the geometry and lose
// the table. Requiring a heading-sized anchor (>= 1.15x base) keeps the
// body pass to genuine scripts hanging off headings.
let script_index = ScriptBodyIndex::new(items);
let script_flags: Vec<bool> = items
.iter()
.map(|item| script_index.is_script_attachment(item, 0.0))
.collect();
let body_script_flags: Vec<bool> = items
.iter()
.map(|item| script_index.is_script_attachment(item, base_font_size * 1.15))
.collect();
let table_candidates: Vec<(usize, &TextItem)> = items
.iter()
.enumerate()
@@ -494,7 +581,14 @@ pub(crate) fn detect_tables_with_page_width(
.collect();
if table_candidates.len() >= 6 {
let regions = find_table_regions(&table_candidates);
// Qualify regions from non-script items: a cluster of sub/superscripts
// must not, on its own, mark out a table region.
let region_evidence: Vec<(usize, &TextItem)> = table_candidates
.iter()
.filter(|(idx, _)| !script_flags[*idx])
.cloned()
.collect();
let regions = find_table_regions(&region_evidence);
for (y_min, y_max) in regions {
let region_items: Vec<(usize, &TextItem)> = table_candidates
@@ -508,7 +602,9 @@ pub(crate) fn detect_tables_with_page_width(
}
if let Some(mut table) =
detect_table_in_region(&region_items, TableDetectionMode::SmallFont)
detect_table_in_region(&region_items, TableDetectionMode::SmallFont, &|i| {
script_flags[i]
})
{
// Try to recover body-font header row above the small-font table
recover_header_row(&mut table, items, table_font_threshold);
@@ -553,8 +649,20 @@ pub(crate) fn detect_tables_with_page_width(
body_font_low,
body_font_high,
);
// Scripts are NOT filtered out of the candidate set here, mirroring
// the small-font pass: they must stay eligible for cell assignment so
// a sub/superscript that belongs inside a table cell keeps its text.
// The heading-anchored `body_script_flags` mask removes them from
// geometry only.
if body_candidates.len() >= 6 {
let regions = find_table_regions_strict(&body_candidates);
// Same reasoning as the small-font pass: scripts do not qualify
// regions, but remain available for cell assignment within one.
let region_evidence: Vec<(usize, &TextItem)> = body_candidates
.iter()
.filter(|(idx, _)| !body_script_flags[*idx])
.cloned()
.collect();
let regions = find_table_regions_strict(&region_evidence);
log::debug!("body-font: {} strict regions found", regions.len());
for (y_min, y_max, _x_min, _x_max) in &regions {
@@ -580,7 +688,9 @@ pub(crate) fn detect_tables_with_page_width(
}
if let Some(table) =
detect_table_in_region(&region_items, TableDetectionMode::BodyFont)
detect_table_in_region(&region_items, TableDetectionMode::BodyFont, &|i| {
body_script_flags[i]
})
{
tables.push(table);
}
@@ -808,10 +918,30 @@ fn find_table_regions_strict(items: &[(usize, &TextItem)]) -> Vec<(f32, f32, f32
regions
}
/// Detect a table within a specific region
fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode) -> Option<Table> {
// Find column boundaries
let columns = find_column_boundaries(items, mode);
/// Detect a table within a specific region.
///
/// `is_script` marks items that are sub/superscript attachments. Those are
/// excluded from the *geometry* — they must not be able to create a column,
/// which is how equation subscript clusters used to fabricate phantom grids —
/// but they remain eligible for cell assignment, so legitimate cell content
/// (exponents in an engineering-notation table, footnote markers) stays in
/// the cell it belongs to instead of leaking out into the reading order.
fn detect_table_in_region(
items: &[(usize, &TextItem)],
mode: TableDetectionMode,
is_script: &dyn Fn(usize) -> bool,
) -> Option<Table> {
// Column geometry from non-script items only.
let geometry_items: Vec<(usize, &TextItem)> = items
.iter()
.filter(|(idx, _)| !is_script(*idx))
.cloned()
.collect();
// A region that is *entirely* scripts has no table structure at all.
if geometry_items.is_empty() {
return None;
}
let columns = find_column_boundaries(&geometry_items, mode);
let min_cols = 2;
if columns.len() < min_cols || columns.len() > 25 {
log::debug!(
@@ -822,8 +952,8 @@ fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode
return None;
}
// Find row boundaries
let rows = find_row_boundaries(items);
// Find row boundaries (geometry items only, same reasoning)
let rows = find_row_boundaries(&geometry_items);
let min_rows = 2;
if rows.len() < min_rows {
log::debug!(
@@ -842,6 +972,11 @@ fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode
);
// Verify this looks like a table: multiple items should align to columns
// Validate against ALL items, including scripts. Columns are derived from
// non-script geometry so scripts cannot *create* a column, but excluding
// them from validation too would let a region manufacture alignment: drop
// the awkward items and whatever remains looks like a tidy grid. Block
// diagrams did exactly that. Everything in the region must fit.
let col_alignment = check_column_alignment(items, &columns, mode);
let min_alignment = match mode {
TableDetectionMode::SmallFont => 0.5,
@@ -912,6 +1047,29 @@ fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode
cells.push(row_cells);
}
// Validation 0 (small-font pass only): reject tiny all-numeric
// fragments. A <=2-row grid whose every cell is a bare 1-2 digit number
// carries no tabular information — in practice these are
// exponent/subscript clusters from display math that happen to align.
// Body-font tables are not subject to this veto: their cells cannot be
// script glyphs.
if matches!(mode, TableDetectionMode::SmallFont) {
let nonempty_cells: Vec<&String> =
cells.iter().flatten().filter(|c| !c.is_empty()).collect();
if rows.len() <= 2
&& !nonempty_cells.is_empty()
&& nonempty_cells
.iter()
.all(|c| c.len() <= 2 && c.chars().all(|ch| ch.is_ascii_digit()))
{
log::debug!(
" validation 0 fail: tiny all-numeric fragment ({} cells)",
nonempty_cells.len()
);
return None;
}
}
// Validation 1: some rows should have content in first column.
// Use a lower threshold (25%) for tables with wrapped cells where
// continuation lines leave the first column empty.
@@ -1977,6 +2135,146 @@ fn try_add_label_column(
#[cfg(test)]
mod tests {
fn make_item(text: &str, x: f32, y: f32, font_size: f32, width: f32) -> TextItem {
TextItem {
text: text.to_string(),
x,
y,
width,
height: font_size,
font: "TestFont".to_string(),
font_size,
page: 1,
is_bold: false,
is_italic: false,
is_underline: false,
is_strikeout: false,
item_type: ItemType::Text,
mcid: None,
}
}
#[test]
fn script_attachment_detects_subscript_after_body_text() {
let body = make_item("log", 100.0, 500.0, 10.0, 15.0);
let sub = make_item("10", 115.5, 497.0, 7.0, 7.0);
let items = vec![body, sub.clone()];
assert!(ScriptBodyIndex::new(&items).is_script_attachment(&sub, 0.0));
}
#[test]
fn script_attachment_detects_superscript_footnote_marker() {
let body = make_item("Hartley", 200.0, 500.0, 10.0, 35.0);
let sup = make_item("2", 235.8, 504.0, 6.6, 3.5);
let items = vec![body, sup.clone()];
assert!(ScriptBodyIndex::new(&items).is_script_attachment(&sup, 0.0));
}
#[test]
fn script_attachment_ignores_small_cell_far_from_body_text() {
let body = make_item("Revenue", 100.0, 500.0, 10.0, 40.0);
let cell = make_item("1,234", 180.0, 500.0, 7.0, 20.0);
let items = vec![body, cell.clone()];
assert!(!ScriptBodyIndex::new(&items).is_script_attachment(&cell, 0.0));
}
#[test]
fn body_pass_anchor_spares_cells_beside_slightly_larger_labels() {
// A body-font table cell (10pt) sitting beside a slightly larger,
// NON-heading label (12.5pt) with a little baseline jitter. The
// small-font pass treats any larger neighbour as a possible script
// base, but the body pass must not: at body sizes a slightly larger
// neighbour is a bold label or column header, and flagging the cell
// would strip it out of the table geometry and lose the table.
// Cell at the low end of the body band (0.85x base) beside a 10.5pt
// label. 10.5 clears the inherent 1.2x-of-cell rule (10.2) but falls
// below the body pass's heading anchor (11.5), which is exactly the
// band where the two masks must disagree.
let label = make_item("Revenue", 100.0, 500.0, 10.5, 40.0);
let cell = make_item("1,234", 141.0, 496.5, 8.5, 22.0);
let items = vec![label, cell.clone()];
let index = ScriptBodyIndex::new(&items);
let base = 10.0;
assert!(
index.is_script_attachment(&cell, 0.0),
"small-font pass anchor should still see this as an attachment"
);
assert!(
!index.is_script_attachment(&cell, base * 1.15),
"body pass must not treat a cell beside a slightly larger label \
as a script — that removes real cells from the geometry"
);
// A genuine heading-sized anchor still qualifies in the body pass.
let heading = make_item("Section", 100.0, 500.0, 20.0, 60.0);
let sup = make_item("3", 161.0, 508.0, 10.0, 5.0);
let h_items = vec![heading, sup.clone()];
assert!(
ScriptBodyIndex::new(&h_items).is_script_attachment(&sup, base * 1.15),
"script hanging off a heading must still be excluded in the body pass"
);
}
#[test]
fn script_attachment_ignores_same_baseline_neighbor_cell() {
// A small cell beside a larger label on the SAME baseline is a table
// layout, not a subscript — a genuine baseline offset is required.
let label = make_item("Total", 100.0, 500.0, 10.0, 25.0);
let cell = make_item("42", 127.0, 500.0, 7.5, 9.0);
let items = vec![label, cell.clone()];
assert!(!ScriptBodyIndex::new(&items).is_script_attachment(&cell, 0.0));
}
#[test]
fn script_attachment_ignores_neighbor_on_different_line() {
let body = make_item("Header", 100.0, 500.0, 10.0, 30.0);
let cell = make_item("42", 131.0, 486.0, 7.0, 10.0);
let items = vec![body, cell.clone()];
assert!(!ScriptBodyIndex::new(&items).is_script_attachment(&cell, 0.0));
}
/// Equation-subscript + footnote layout from Shannon entropy.pdf page 1,
/// with real coordinates. Without the larger-font anchors the small items
/// alone DO form a phantom table — proving the layout reaches detection —
/// and adding the anchors must suppress it.
fn shannon_page1_small_items() -> Vec<TextItem> {
vec![
make_item("2", 267.4, 133.9, 7.4, 3.7),
make_item("10", 306.2, 133.9, 7.4, 7.4),
make_item("10", 342.7, 133.9, 7.4, 7.4),
make_item("10", 325.0, 118.9, 7.4, 7.4),
make_item("Bell System Technical Journal,", 295.7, 101.9, 8.0, 95.0),
make_item(
"April 1924, p. 324; Certain Topics in",
396.7,
101.9,
8.0,
130.0,
),
make_item("v. 47, April 1928, p. 617.", 250.9, 92.5, 8.0, 90.0),
make_item("Bell System Technical Journal,", 264.2, 82.6, 8.0, 95.0),
make_item("July 1928, p. 535.", 364.3, 82.6, 8.0, 65.0),
]
}
#[test]
fn equation_scripts_do_not_form_phantom_table() {
let bare = shannon_page1_small_items();
assert!(
!detect_tables(&bare, 10.0, false).is_empty(),
"test layout must form a phantom table when the filter cannot fire"
);
let mut items = shannon_page1_small_items();
items.push(make_item("log", 253.0, 137.0, 10.0, 13.5));
items.push(make_item("log", 291.5, 137.0, 10.0, 13.5));
items.push(make_item("log", 328.0, 137.0, 10.0, 13.5));
items.push(make_item("log", 310.3, 122.0, 10.0, 13.5));
let tables = detect_tables(&items, 10.0, false);
assert!(
tables.is_empty(),
"equation scripts + footnotes must not become a table: {tables:?}"
);
}
use super::*;
use crate::types::ItemType;