Compare commits

...
Author SHA1 Message Date
Abimael Martell 3fe9ccee07 chore(release): bump package versions 2026-08-10 14:16:54 -07:00
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
fabec0aec3 feat(napi): add async variants that keep the Node event loop free (#337)
* feat(napi): add processPdfAsync, classifyPdfAsync, extractPagesMarkdownAsync

The Node bindings are synchronous, so every call parses on the event
loop thread — up to hundreds of milliseconds of dead loop per document
in a server. Add additive AsyncTask-based variants that run the same
shared implementations on the libuv thread pool and return promises.

The existing synchronous exports keep their names, signatures, and
behaviour; each sync/async pair shares one implementation. Panics in
compute() are caught and surfaced as rejections, matching the sync
error contract.

Closes #336

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

* fix(napi): read async task buffers in place instead of copying

Review feedback on #337: buffer.to_vec() copied the whole PDF on the
event loop before the task was queued, so large inputs still stalled
the loop and doubled peak memory. The tasks now hold the napi Buffer
itself — its ref pins the JS allocation for the task's lifetime and
the backing store is stable, so compute() reads it directly from the
worker thread. Callers must not mutate the buffer until the promise
settles (same contract as Node's async fs APIs); documented on each
export and in the README.

The suggested removal of ts_return_type was checked and rejected:
without it napi-rs generates Promise<unknown> for AsyncTask returns.
A comment now records that finding.

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

* fix(napi): copy async task input on the JS thread for soundness

Review feedback on #337: holding the napi Buffer and reading it from
the libuv worker was unsound. Buffer derefs straight to the JS-side
allocation, so a caller mutating it before the promise settled would
race the worker's reads — undefined behavior, not a recoverable error,
and the documented don't-mutate contract was unenforceable. Deferring
the copy to compute() would not help: any off-thread read races the
same way. The JS thread is the only race-free place to take the copy,
because JS is single-threaded and nothing can mutate the buffer during
the synchronous part of the call.

Revert to an owned Vec<u8> copied at call time. The cost is one memcpy,
negligible next to the parse the async variants exist to unblock. Docs
now state the buffer may be reused or mutated immediately, and a test
locks in the copy semantics by mutating the input while a parse is in
flight.

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 12:29:23 -07:00
1f28c00a13 Bound column-detection histogram and harden coordinate handling (#328)
* Bound column-detection histogram size

Derive the projection histogram from a clamped bin count and skip
non-finite page widths. Extreme or malformed text-item coordinates
(from the content-stream text matrix) could otherwise drive a very
large allocation. 65,536 bins is ~9x the largest legal page, so real
layouts are unaffected. Adds regression tests.

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

* Exclude non-finite coordinates from page bounds

Items at NaN/inf positions are now skipped when folding the page bounds,
so a malformed coordinate can no longer escape as a ColumnRegion
boundary, and an all-non-finite page returns no columns. Bad items are
dropped individually rather than failing the page, so one stray glyph
does not disable column detection.

Addresses review feedback on the finite-width guard.

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

* Trim far-outlier coordinates from page bounds

Gutter margins, spanning-item width and the XY-cut margin are all
fractions of page_width, so a single far-but-finite item (x=50_000 is
enough) set the scale for the whole page: real gutters fell inside the
rejected margin band and a genuine two-column page collapsed to one
region. When the span exceeds one legal page (14_400 units), re-derive
the bounds from items clustered around the median x. Outliers keep their
text because column assignment buckets by nearest overlap.

The MAX_BINS ceiling stays as an allocation bound that does not depend
on this heuristic.

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

* Harden bounds trimming against widths and wide layouts

Check both item edges when trimming: a malformed width at an ordinary
position poisoned x_max just as a malformed position poisoned x_min, so
a huge width still collapsed a two-column page to one region.

Only trim when the far items are a small minority (<=10%). A genuinely
large-format page has content spread across its full width, so it now
keeps its true bounds instead of being reduced to the median cluster.

Correct the MAX_PAGE_EXTENT comment: 14_400 units is the traditional
Acrobat architectural limit, not a format cap. PDF 2.0 sets no page-size
limit and UserUnit scales physical size, so this is a heuristic.

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

* Scale bin width so the histogram spans the whole page

Clamping the bin count alone left anything past MAX_BINS * BIN_WIDTH
(~131k points) outside the histogram, folded into the final bin. A page
wide enough to hit that lost real gutters: with a visible gutter inside
the covered range the XY-cut fallback never runs, so a three-column
layout silently reported two. Derive bin_width from page_width instead,
keeping the same allocation ceiling and degrading only resolution.

Also anchor the trimming median on the same finite left/right items that
bounds() accepts, so a malformed width cannot shift which items count as
strays.

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

* Require geometric evidence before detaching far content

The median-window trim narrowed any content more than one page from the
centre, so a valid large page with a sparse far sidebar lost the sidebar
from its bounds and its text fell into column 0. An item-count minority
rule cannot tell that layout from malformed coordinates.

Group content into clusters separated by more than a whole page of
continuous emptiness, and only drop a cluster that is both detached by
such a void and a small minority of items. Real content does not leave a
gap that large; a stray coordinate sits alone beyond one.

A single run wider than one page is treated as a malformed width, which
also covers the huge-width case the cluster sweep cannot see (such an
item spans everything and leaves no gap).

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

* Judge run width against page content, not a fixed extent

Treating any run wider than 14_400 units as malformed penalised valid
large pages: one made entirely of such runs reported no columns at all,
and a mixed page lost the right edge of every long run.

Judge width relative to the page's own content instead. Positions cannot
be inflated by a bogus width, so the spread of the core cluster is a
sound scale: a run wider than that spread plus one page is malformed.
A genuinely large page keeps its genuinely long runs, while a 1e12-wide
run beside ordinary text is still rejected.

Cluster on positions rather than filled intervals, so a bogus width can
no longer merge everything into one cluster, and keep ordinary pages on
an O(n) fast path that skips the sort entirely.

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 12:29:12 -07:00
f4b8c9e854 Clarify SECURITY.md reporting channels (#329)
* Update SECURITY.md reporting channels

Clarify that email is the only required channel and point the
alternative at Firecrawl's Bugcrowd disclosure engagement instead of
the private-advisory link, which is not enabled on this repo.

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

* Make Bugcrowd the preferred reporting channel

Bugcrowd's disclosure engagement is the primary channel; email to
help@firecrawl.dev is offered as the alternative.

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-09 16:27:50 -07:00
69039f2728 Fix char-boundary panic in hex_to_unicode_string (#320)
Use hex.get(i..i+2) instead of &hex[i..i+2] so a non-hex, non-ASCII
destination in a /ToUnicode CMap can no longer trigger a UTF-8
char-boundary panic. An even byte length does not guarantee the byte
offset falls on a char boundary; get() returns None on a non-boundary
or out-of-range index, folding cleanly into the existing flow.

Add regression tests covering a multi-byte destination char and a
replacement-char byte.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
2026-08-09 08:21:31 -07:00
3cca6446bd fix(glyph_names): handle non-ASCII input in uniXXXX glyph name parsing (#321)
Use str::get instead of a byte-length check plus slice when parsing the
uniXXXX glyph-name form. The byte-length guard only proved the index was
in bounds, not on a UTF-8 char boundary, so a glyph name containing
non-ASCII bytes could cause a slice on a non-boundary index. Switch to a
checked slice that folds into the existing Option flow, and add tests.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Abimael Martell <abimaelmartell@users.noreply.github.com>
2026-08-09 08:21:22 -07: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
22 changed files with 2199 additions and 147 deletions
+1
View File
@@ -37,6 +37,7 @@ scripts/
# Test output
test_output/
.firecrawl/
# Python
__pycache__/
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "pdf-inspector"
version = "0.1.7"
version = "0.1.8"
edition = "2021"
autobins = false
authors = ["Firecrawl Team"]
+4 -3
View File
@@ -5,14 +5,15 @@
If you believe you've found a security vulnerability in pdf-inspector, please
report it privately so we can fix it before public disclosure.
**Preferred:** Email **help@firecrawl.dev** with:
**Preferred:** Submit through Firecrawl's Bugcrowd vulnerability disclosure
program at <https://bugcrowd.com/engagements/firecrawl-vdp-ess>. Please include:
- A description of the issue and its impact
- Steps to reproduce (a minimal PDF or input that triggers the bug is ideal)
- The version or commit hash of pdf-inspector you tested against
**Alternative:** Use GitHub's private vulnerability reporting under the
[Security tab](https://github.com/firecrawl/pdf-inspector/security/advisories/new).
**Alternative:** If you'd rather not use Bugcrowd, email
**help@firecrawl.dev** with the same details.
We'll acknowledge your report in a timely manner and keep you updated on
remediation progress. Please do not open a public GitHub issue for security
+2 -2
View File
@@ -851,7 +851,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe"
[[package]]
name = "pdf-inspector"
version = "0.1.7"
version = "0.1.8"
dependencies = [
"env_logger",
"include_dir",
@@ -867,7 +867,7 @@ dependencies = [
[[package]]
name = "pdf-inspector-napi"
version = "0.2.2"
version = "0.2.3"
dependencies = [
"napi",
"napi-build",
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "pdf-inspector-napi"
version = "0.2.2"
version = "0.2.3"
edition = "2021"
[lib]
+16
View File
@@ -83,6 +83,22 @@ for (const region of result[0].regions) {
}
```
### Async variants
`processPdf`, `classifyPdf`, and `extractPagesMarkdown` are synchronous and parse on the calling thread — in Node, that's the event loop. For a one-off call in a script that's fine, but in a server a large document can hold the loop for tens to hundreds of milliseconds.
`processPdfAsync`, `classifyPdfAsync`, and `extractPagesMarkdownAsync` take the same arguments and produce the same results, but run the parse on the libuv thread pool and return a promise, keeping the event loop free. The input buffer is copied before the call returns, so it's safe to reuse or mutate immediately:
```typescript
import { classifyPdfAsync, extractPagesMarkdownAsync } from '@firecrawl/pdf-inspector'
const classification = await classifyPdfAsync(pdf)
if (classification.pdfType === 'TextBased') {
const { pages } = await extractPagesMarkdownAsync(pdf)
// ...
}
```
## Types
```typescript
+6 -6
View File
@@ -8,12 +8,12 @@
"@napi-rs/cli": "^3.4.1",
},
"optionalDependencies": {
"@firecrawl/pdf-inspector-darwin-arm64": "1.12.0",
"@firecrawl/pdf-inspector-linux-arm64-gnu": "1.12.0",
"@firecrawl/pdf-inspector-linux-arm64-musl": "1.12.0",
"@firecrawl/pdf-inspector-linux-x64-gnu": "1.12.0",
"@firecrawl/pdf-inspector-linux-x64-musl": "1.12.0",
"@firecrawl/pdf-inspector-win32-x64-msvc": "1.12.0",
"@firecrawl/pdf-inspector-darwin-arm64": "1.13.0",
"@firecrawl/pdf-inspector-linux-arm64-gnu": "1.13.0",
"@firecrawl/pdf-inspector-linux-arm64-musl": "1.13.0",
"@firecrawl/pdf-inspector-linux-x64-gnu": "1.13.0",
"@firecrawl/pdf-inspector-linux-x64-musl": "1.13.0",
"@firecrawl/pdf-inspector-win32-x64-msvc": "1.13.0",
},
},
},
+7 -7
View File
@@ -1,6 +1,6 @@
{
"name": "@firecrawl/pdf-inspector",
"version": "1.12.0",
"version": "1.13.0",
"description": "Fast PDF classification and text extraction. Detect text-based vs scanned PDFs, extract text by region with quality checks. Native Rust performance via napi-rs.",
"main": "index.js",
"types": "index.d.ts",
@@ -52,11 +52,11 @@
"@napi-rs/cli": "^3.4.1"
},
"optionalDependencies": {
"@firecrawl/pdf-inspector-linux-x64-gnu": "1.12.0",
"@firecrawl/pdf-inspector-linux-x64-musl": "1.12.0",
"@firecrawl/pdf-inspector-linux-arm64-gnu": "1.12.0",
"@firecrawl/pdf-inspector-linux-arm64-musl": "1.12.0",
"@firecrawl/pdf-inspector-darwin-arm64": "1.12.0",
"@firecrawl/pdf-inspector-win32-x64-msvc": "1.12.0"
"@firecrawl/pdf-inspector-linux-x64-gnu": "1.13.0",
"@firecrawl/pdf-inspector-linux-x64-musl": "1.13.0",
"@firecrawl/pdf-inspector-linux-arm64-gnu": "1.13.0",
"@firecrawl/pdf-inspector-linux-arm64-musl": "1.13.0",
"@firecrawl/pdf-inspector-darwin-arm64": "1.13.0",
"@firecrawl/pdf-inspector-win32-x64-msvc": "1.13.0"
}
}
+182 -41
View File
@@ -153,9 +153,7 @@ fn to_napi_result(r: pdf_inspector::PdfProcessResult) -> PdfResult {
}
}
fn to_napi_page_ocr_reasons(
reasons: Vec<pdf_inspector::PageOcrReasons>,
) -> Vec<PageOcrReasons> {
fn to_napi_page_ocr_reasons(reasons: Vec<pdf_inspector::PageOcrReasons>) -> Vec<PageOcrReasons> {
reasons
.into_iter()
.map(|reason| PageOcrReasons {
@@ -202,6 +200,31 @@ where
}
}
// ---------------------------------------------------------------------------
// Shared implementations (single body behind sync and async entry points)
// ---------------------------------------------------------------------------
fn process_pdf_impl(bytes: &[u8], pages: Option<Vec<u32>>) -> Result<PdfResult> {
let mut opts = pdf_inspector::PdfOptions::new();
if let Some(p) = pages {
opts = opts.pages(p);
}
let result = pdf_inspector::process_pdf_mem_with_options(bytes, opts)
.map_err(|e| to_napi_err(e, "process_pdf"))?;
Ok(to_napi_result(result))
}
fn classify_pdf_impl(bytes: &[u8]) -> Result<PdfClassification> {
let result =
pdf_inspector::classify_pdf_mem(bytes).map_err(|e| to_napi_err(e, "classify_pdf"))?;
Ok(PdfClassification {
pdf_type: convert_pdf_type(result.pdf_type),
page_count: result.page_count,
pages_needing_ocr: result.pages_needing_ocr,
confidence: result.confidence as f64,
})
}
// ---------------------------------------------------------------------------
// Public NAPI API
// ---------------------------------------------------------------------------
@@ -210,15 +233,7 @@ where
#[napi]
pub fn process_pdf(buffer: Buffer, pages: Option<Vec<u32>>) -> Result<PdfResult> {
let bytes: Vec<u8> = buffer.to_vec();
catch_panic("process_pdf", move || {
let mut opts = pdf_inspector::PdfOptions::new();
if let Some(p) = pages {
opts = opts.pages(p);
}
let result = pdf_inspector::process_pdf_mem_with_options(&bytes, opts)
.map_err(|e| to_napi_err(e, "process_pdf"))?;
Ok(to_napi_result(result))
})
catch_panic("process_pdf", move || process_pdf_impl(&bytes, pages))
}
/// Fast detection only — no text extraction or markdown.
@@ -238,16 +253,7 @@ pub fn detect_pdf(buffer: Buffer) -> Result<PdfResult> {
#[napi]
pub fn classify_pdf(buffer: Buffer) -> Result<PdfClassification> {
let bytes: Vec<u8> = buffer.to_vec();
catch_panic("classify_pdf", move || {
let result =
pdf_inspector::classify_pdf_mem(&bytes).map_err(|e| to_napi_err(e, "classify_pdf"))?;
Ok(PdfClassification {
pdf_type: convert_pdf_type(result.pdf_type),
page_count: result.page_count,
pages_needing_ocr: result.pages_needing_ocr,
confidence: result.confidence as f64,
})
})
catch_panic("classify_pdf", move || classify_pdf_impl(&bytes))
}
/// Extract plain text from a PDF Buffer.
@@ -633,25 +639,32 @@ pub fn extract_pages_markdown(
) -> Result<PagesExtractionResult> {
let bytes: Vec<u8> = buffer.to_vec();
catch_panic("extract_pages_markdown", move || {
let result = pdf_inspector::extract_pages_markdown_mem(&bytes, pages.as_deref())
.map_err(|e| to_napi_err(e, "extract_pages_markdown"))?;
Ok(PagesExtractionResult {
pages: result
.pages
.into_iter()
.map(|r| PageMarkdownResult {
page: r.page,
markdown: r.markdown,
needs_ocr: r.needs_ocr,
ocr_reason: r.ocr_reason,
})
.collect(),
pages_with_tables: result.pages_with_tables,
pages_with_columns: result.pages_with_columns,
pages_needing_ocr: result.pages_needing_ocr,
ocr_reasons_by_page: to_napi_page_ocr_reasons(result.ocr_reasons_by_page),
is_complex: result.is_complex,
})
extract_pages_markdown_impl(&bytes, pages.as_deref())
})
}
fn extract_pages_markdown_impl(
bytes: &[u8],
pages: Option<&[u32]>,
) -> Result<PagesExtractionResult> {
let result = pdf_inspector::extract_pages_markdown_mem(bytes, pages)
.map_err(|e| to_napi_err(e, "extract_pages_markdown"))?;
Ok(PagesExtractionResult {
pages: result
.pages
.into_iter()
.map(|r| PageMarkdownResult {
page: r.page,
markdown: r.markdown,
needs_ocr: r.needs_ocr,
ocr_reason: r.ocr_reason,
})
.collect(),
pages_with_tables: result.pages_with_tables,
pages_with_columns: result.pages_with_columns,
pages_needing_ocr: result.pages_needing_ocr,
ocr_reasons_by_page: to_napi_page_ocr_reasons(result.ocr_reasons_by_page),
is_complex: result.is_complex,
})
}
@@ -692,3 +705,131 @@ fn to_page_region_texts(results: Vec<pdf_inspector::PageRegionResult>) -> Vec<Pa
})
.collect()
}
// ---------------------------------------------------------------------------
// Async variants (libuv thread pool via AsyncTask)
//
// The synchronous exports above parse on the calling thread, which in Node is
// the event loop. These `*Async` variants run the same shared implementations
// on the libuv thread pool and hand JavaScript a promise, so servers under
// concurrent load keep answering requests while a document parses. The sync
// exports keep their names, signatures, and behaviour.
//
// Each factory copies the input Buffer to an owned `Vec<u8>` on the calling
// (JS) thread — deliberately. JS execution is single-threaded, so no JS code
// can mutate the buffer while the synchronous part of the call copies it.
// Holding the napi `Buffer` and reading it from the worker instead would be
// zero-copy, but a caller mutating the buffer before the promise settles
// would then race the worker's reads — undefined behavior, not a recoverable
// error (a known napi-rs soundness hazard with cross-thread Buffer access).
// The copy is a one-time memcpy, negligible next to the parse it unblocks.
// ---------------------------------------------------------------------------
pub struct ProcessPdfTask {
bytes: Vec<u8>,
pages: Option<Vec<u32>>,
}
impl Task for ProcessPdfTask {
type Output = PdfResult;
type JsValue = PdfResult;
fn compute(&mut self) -> Result<Self::Output> {
let bytes = std::mem::take(&mut self.bytes);
let pages = self.pages.take();
// AssertUnwindSafe: `bytes`/`pages` are moved into the closure and
// dropped on unwind — no shared state can be observed broken.
catch_panic(
"process_pdf",
panic::AssertUnwindSafe(move || process_pdf_impl(&bytes, pages)),
)
}
fn resolve(&mut self, _env: Env, output: Self::Output) -> Result<Self::JsValue> {
Ok(output)
}
}
/// Async variant of [`processPdf`]: same result, but the parse runs on the
/// libuv thread pool instead of the event loop and the call returns a
/// promise. The buffer is copied before the call returns, so it may be
/// reused or mutated immediately.
// ts_return_type is required: napi-rs emits `Promise<unknown>` for
// `AsyncTask<T>` returns without it.
#[napi(ts_return_type = "Promise<PdfResult>")]
pub fn process_pdf_async(buffer: Buffer, pages: Option<Vec<u32>>) -> AsyncTask<ProcessPdfTask> {
AsyncTask::new(ProcessPdfTask {
bytes: buffer.to_vec(),
pages,
})
}
pub struct ClassifyPdfTask {
bytes: Vec<u8>,
}
impl Task for ClassifyPdfTask {
type Output = PdfClassification;
type JsValue = PdfClassification;
fn compute(&mut self) -> Result<Self::Output> {
let bytes = std::mem::take(&mut self.bytes);
catch_panic(
"classify_pdf",
panic::AssertUnwindSafe(move || classify_pdf_impl(&bytes)),
)
}
fn resolve(&mut self, _env: Env, output: Self::Output) -> Result<Self::JsValue> {
Ok(output)
}
}
/// Async variant of [`classifyPdf`]: same result, but the classification runs
/// on the libuv thread pool instead of the event loop and the call returns a
/// promise. The buffer is copied before the call returns, so it may be
/// reused or mutated immediately.
#[napi(ts_return_type = "Promise<PdfClassification>")]
pub fn classify_pdf_async(buffer: Buffer) -> AsyncTask<ClassifyPdfTask> {
AsyncTask::new(ClassifyPdfTask {
bytes: buffer.to_vec(),
})
}
pub struct ExtractPagesMarkdownTask {
bytes: Vec<u8>,
pages: Option<Vec<u32>>,
}
impl Task for ExtractPagesMarkdownTask {
type Output = PagesExtractionResult;
type JsValue = PagesExtractionResult;
fn compute(&mut self) -> Result<Self::Output> {
let bytes = std::mem::take(&mut self.bytes);
let pages = self.pages.take();
catch_panic(
"extract_pages_markdown",
panic::AssertUnwindSafe(move || extract_pages_markdown_impl(&bytes, pages.as_deref())),
)
}
fn resolve(&mut self, _env: Env, output: Self::Output) -> Result<Self::JsValue> {
Ok(output)
}
}
/// Async variant of [`extractPagesMarkdown`]: same result, but the extraction
/// runs on the libuv thread pool instead of the event loop and the call
/// returns a promise. The buffer is copied before the call returns, so it
/// may be reused or mutated immediately.
#[napi(ts_return_type = "Promise<PagesExtractionResult>")]
pub fn extract_pages_markdown_async(
buffer: Buffer,
pages: Option<Vec<u32>>,
) -> AsyncTask<ExtractPagesMarkdownTask> {
AsyncTask::new(ExtractPagesMarkdownTask {
bytes: buffer.to_vec(),
pages,
})
}
+68
View File
@@ -2,13 +2,16 @@ import { readFileSync } from 'fs';
import { strict as assert } from 'assert';
import {
processPdf,
processPdfAsync,
detectPdf,
classifyPdf,
classifyPdfAsync,
extractText,
extractTextWithPositions,
extractTextInRegions,
detectVectorGridInRegion,
extractPagesMarkdown,
extractPagesMarkdownAsync,
} from './index.js';
const fixture = readFileSync('../tests/fixtures/thermo-freon12.pdf');
@@ -124,10 +127,75 @@ assert.equal(picked.pages[0].page, 2);
assert.equal(picked.pages[1].page, 0);
console.log(' extractPagesMarkdown with pages: OK');
// --- Async variants ---
console.log('Testing async variants...');
// processPdfAsync returns a promise and matches the sync result
const asyncResultPromise = processPdfAsync(fixture);
assert.ok(asyncResultPromise instanceof Promise);
const asyncResult = await asyncResultPromise;
assert.equal(asyncResult.pdfType, result.pdfType);
assert.equal(asyncResult.pageCount, result.pageCount);
assert.equal(asyncResult.markdown, result.markdown);
console.log(' processPdfAsync: OK');
// processPdfAsync with pages
const asyncResult2 = await processPdfAsync(fixture, [1]);
assert.equal(asyncResult2.markdown, result2.markdown);
console.log(' processPdfAsync with pages: OK');
// classifyPdfAsync matches the sync result
const asyncClassified = await classifyPdfAsync(fixture);
assert.equal(asyncClassified.pdfType, classified.pdfType);
assert.equal(asyncClassified.pageCount, classified.pageCount);
assert.equal(asyncClassified.confidence, classified.confidence);
assert.deepEqual(asyncClassified.pagesNeedingOcr, classified.pagesNeedingOcr);
console.log(' classifyPdfAsync: OK');
// extractPagesMarkdownAsync matches the sync result
const asyncAllPages = await extractPagesMarkdownAsync(fixture);
assert.equal(asyncAllPages.pages.length, allPages.pages.length);
assert.deepEqual(
asyncAllPages.pages.map(p => p.markdown),
allPages.pages.map(p => p.markdown),
);
assert.equal(asyncAllPages.isComplex, allPages.isComplex);
console.log(' extractPagesMarkdownAsync: OK');
// selected pages preserve caller order
const asyncPicked = await extractPagesMarkdownAsync(fixture, [2, 0]);
assert.equal(asyncPicked.pages.length, 2);
assert.equal(asyncPicked.pages[0].page, 2);
assert.equal(asyncPicked.pages[1].page, 0);
console.log(' extractPagesMarkdownAsync with pages: OK');
// input buffer is copied at call time: mutating it immediately after the
// call must not affect the in-flight parse
const scratch = Buffer.from(fixture);
const inFlight = processPdfAsync(scratch);
scratch.fill(0);
const fromMutated = await inFlight;
assert.equal(fromMutated.markdown, result.markdown);
console.log(' processPdfAsync input copied at call time: OK');
// concurrent async calls all settle
const [c1, c2, c3] = await Promise.all([
processPdfAsync(fixture),
classifyPdfAsync(fixture),
extractPagesMarkdownAsync(fixture),
]);
assert.equal(c1.pdfType, 'TextBased');
assert.equal(c2.pdfType, 'TextBased');
assert.equal(c3.pages.length, 3);
console.log(' concurrent async calls: OK');
// --- Error handling ---
console.log('Testing error handling...');
assert.throws(() => processPdf(Buffer.from('not a pdf')), /process_pdf/);
assert.throws(() => classifyPdf(Buffer.from('')), /classify_pdf/);
await assert.rejects(processPdfAsync(Buffer.from('not a pdf')), /process_pdf/);
await assert.rejects(classifyPdfAsync(Buffer.from('')), /classify_pdf/);
await assert.rejects(extractPagesMarkdownAsync(Buffer.from('')), /extract_pages_markdown/);
console.log(' error handling: OK');
console.log('\nAll NAPI tests passed!');
+1 -1
View File
@@ -6,7 +6,7 @@ build-backend = "maturin"
name = "pdf-inspector"
# Bump this to publish to PyPI — CI publishes automatically when the version
# changes on main (same flow as napi/package.json for npm).
version = "0.2.6"
version = "0.2.7"
description = "Fast PDF inspection, classification, and text extraction with smart scanned vs text-based detection"
readme = "docs/python.md"
license = { text = "MIT" }
+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>
+327 -17
View File
@@ -41,15 +41,110 @@ pub(crate) fn detect_columns(
}
debug!("page {}: detect_columns: {} items", page, page_items.len());
// Find page bounds
let x_min = page_items.iter().map(|i| i.x).fold(f32::INFINITY, f32::min);
let x_max = page_items
.iter()
.map(|i| i.x + effective_width(i))
.fold(f32::NEG_INFINITY, f32::max);
// The width of one ordinary page, used three ways below: as the largest
// credible width for a single text run, as the size of empty gap that marks
// content as detached, and as the span past which those checks run at all.
// This is a heuristic, not a format rule: PDF 2.0 sets no page-size limit,
// and since PDF 1.6 `UserUnit` scales a page's physical size independently
// of its coordinates. 14_400 units (200in at the default 1/72in unit) is
// the traditional Acrobat architectural limit, which makes it a reasonable
// "wider than any ordinary page" mark in coordinate space.
const MAX_PAGE_EXTENT: f32 = 14_400.0;
// A detached cluster is only dropped if it also holds a small minority of
// the items, so a genuine two-part layout keeps its full bounds even when
// the halves are far apart.
const MAX_TRIM_FRACTION: f32 = 0.10;
// Position and width of each item, skipping only non-finite geometry.
let finite_span = |i: &&TextItem| -> Option<(f32, f32)> {
let (left, width) = (i.x, effective_width(i));
(left.is_finite() && (left + width).is_finite()).then_some((left, width))
};
let (min_left, max_right, total) = page_items.iter().filter_map(finite_span).fold(
(f32::INFINITY, f32::NEG_INFINITY, 0usize),
|(lo, hi, n), (left, width)| (lo.min(left), hi.max(left + width), n + 1),
);
// No item had usable geometry, so there is no layout to report.
if total == 0 {
return vec![];
}
// Every threshold below (gutter margins, spanning-item width, the XY-cut
// margin) is a fraction of the page width, so a far item can set the scale
// for the whole page and shrink the effective detection window to a
// rounding error — real gutters then fall inside the margin band and a
// genuine multi-column page collapses to one region.
//
// Anything inside one page extent is ordinary, so the common case keeps the
// plain bounds and skips the work below entirely.
let (x_min, x_max) = if max_right - min_left <= MAX_PAGE_EXTENT {
(min_left, max_right)
} else {
// Discarding content needs positive evidence that it is not part of the
// layout, because a count-based rule alone cannot tell a stray from a
// sparse far sidebar. The evidence is geometric: positions are grouped
// into clusters separated by more than a whole page of continuous
// emptiness. Real content, however sparse, does not leave a void that
// large; a malformed coordinate sits alone beyond one.
let mut spans: Vec<(f32, f32)> = page_items.iter().filter_map(finite_span).collect();
spans.sort_by(|a, b| a.0.total_cmp(&b.0));
let mut core: Option<std::ops::Range<usize>> = None;
let mut start = 0usize;
for i in 1..=spans.len() {
if i < spans.len() && spans[i].0 - spans[i - 1].0 <= MAX_PAGE_EXTENT {
continue;
}
if core.as_ref().is_none_or(|best| i - start > best.len()) {
core = Some(start..i);
}
start = i;
}
let mut core = core.unwrap_or(0..spans.len());
// Only drop the detached clusters when they are a small minority, so a
// genuine two-part layout keeps its full bounds.
let dropped = spans.len() - core.len();
if dropped as f32 > spans.len() as f32 * MAX_TRIM_FRACTION {
core = 0..spans.len();
}
let core = &spans[core];
// Positions cannot be inflated by a bogus width, so the spread of the
// content is a sound scale for judging one. A run much wider than the
// page's own content is a malformed width — the test is relative, so a
// genuinely large page keeps its genuinely long runs.
let (lo, widest_left) = (core[0].0, core[core.len() - 1].0);
let max_run_width = (widest_left - lo) + MAX_PAGE_EXTENT;
let hi = core
.iter()
.filter(|&&(_, width)| width <= max_run_width)
.map(|&(left, width)| left + width)
.fold(widest_left, f32::max);
if lo != min_left || hi != max_right {
debug!(
"page {page}: bounds {min_left}..{max_right} exceed one page; \
dropped {dropped}/{} detached item(s), using {lo}..{hi}",
spans.len()
);
}
(lo, hi)
};
// Hard ceiling on the histogram size, independent of the trimming above:
// the bounds are attacker-influenced, so an unclamped
// `page_width / BIN_WIDTH` lets a crafted PDF force an arbitrarily large
// `vec![0u32; num_bins]` allocation. 65_536 bins covers ~128k points at
// BIN_WIDTH 2.0 — roughly 9x the largest legal page — so this never binds
// on a real layout. Kept as a bound that does not depend on the outlier
// heuristic staying correct.
const MAX_BINS: usize = 65_536;
let page_width = x_max - x_min;
if page_width < 200.0 {
if !page_width.is_finite() || page_width < 200.0 {
return vec![ColumnRegion { x_min, x_max }];
}
@@ -57,13 +152,20 @@ pub(crate) fn detect_columns(
return vec![ColumnRegion { x_min, x_max }];
}
// Widen the bins rather than dropping the tail of the page. Clamping the
// count alone would leave anything past MAX_BINS * BIN_WIDTH outside the
// histogram, folded into the last bin, which places gutters at the wrong
// coordinates. Scaling keeps full coverage under the same allocation
// ceiling; only the resolution degrades, and only beyond ~131k points.
let bin_width = BIN_WIDTH.max(page_width / MAX_BINS as f32);
// Build occupancy histogram.
// Exclude items wider than 60% of page width — these are spanning items
// (titles, full-width paragraphs) that would fill the gutter and prevent
// detection of partial-page column layouts (e.g. two-column abstracts on
// a page that also has single-column introduction text).
let wide_threshold = page_width * 0.6;
let num_bins = ((page_width / BIN_WIDTH).ceil() as usize).max(1);
let num_bins = ((page_width / bin_width).ceil() as usize).clamp(1, MAX_BINS);
let mut histogram = vec![0u32; num_bins];
for item in &page_items {
@@ -71,8 +173,8 @@ pub(crate) fn detect_columns(
if w > wide_threshold {
continue;
}
let left = ((item.x - x_min) / BIN_WIDTH).floor() as usize;
let right = (((item.x + w) - x_min) / BIN_WIDTH).ceil() as usize;
let left = ((item.x - x_min) / bin_width).floor() as usize;
let right = (((item.x + w) - x_min) / bin_width).ceil() as usize;
let left = left.min(num_bins);
let right = right.min(num_bins);
for count in histogram.iter_mut().take(right).skip(left) {
@@ -109,12 +211,12 @@ pub(crate) fn detect_columns(
let valleys: Vec<(usize, usize)> = valleys
.into_iter()
.filter(|&(start, end)| {
let width_pts = (end - start) as f32 * BIN_WIDTH;
let width_pts = (end - start) as f32 * bin_width;
if width_pts < MIN_GUTTER_WIDTH {
return false;
}
// Valley center must not be within 5% of page edges
let center_pts = ((start + end) as f32 / 2.0) * BIN_WIDTH;
let center_pts = ((start + end) as f32 / 2.0) * bin_width;
center_pts > margin_threshold && center_pts < (page_width - margin_threshold)
})
.collect();
@@ -132,7 +234,7 @@ pub(crate) fn detect_columns(
&histogram,
num_bins,
x_min,
BIN_WIDTH,
bin_width,
page_width,
margin_threshold,
);
@@ -141,7 +243,7 @@ pub(crate) fn detect_columns(
&rel_valleys,
&page_items,
x_min,
BIN_WIDTH,
bin_width,
x_max,
MIN_ITEMS_PER_COLUMN,
MIN_VERTICAL_SPAN_RATIO,
@@ -182,7 +284,7 @@ pub(crate) fn detect_columns(
&valleys,
&page_items,
x_min,
BIN_WIDTH,
bin_width,
x_max,
MIN_ITEMS_PER_COLUMN,
MIN_VERTICAL_SPAN_RATIO,
@@ -196,7 +298,7 @@ pub(crate) fn detect_columns(
&valleys,
&page_items,
x_min,
BIN_WIDTH,
bin_width,
x_max,
MIN_ITEMS_PER_COLUMN,
MIN_VERTICAL_SPAN_RATIO,
@@ -1827,7 +1929,7 @@ fn split_column_stragglers(lines: Vec<TextLine>) -> (Vec<TextLine>, Vec<TextLine
.unwrap();
let (cs, ce) = segments[core_seg];
let mut core = Vec::with_capacity(ce - cs);
let mut core = Vec::with_capacity(ce.saturating_sub(cs));
let mut stragglers = Vec::new();
for (i, line) in lines.into_iter().enumerate() {
if i >= cs && i < ce {
@@ -2533,6 +2635,214 @@ mod tests {
);
}
#[test]
fn extreme_far_coordinate_does_not_allocate_unboundedly() {
// A crafted PDF can place a text run at an arbitrary coordinate via the
// text matrix. The derived page width must not drive an unbounded
// histogram allocation (previously `page_width / BIN_WIDTH` bins with no
// upper bound would try to reserve terabytes and abort the process).
let mut items = Vec::new();
for i in 0..24 {
items.push(make_item(1, i as f32 * 10.0, 700.0 - i as f32 * 5.0, "A"));
}
// Item placed 1e12 points away — 5e11 bins if left unclamped.
items.push(make_item(1, 1e12, 700.0, "Z"));
// Must return without aborting; content is preserved as a single region.
let cols = detect_columns(&items, 1, false);
assert!(!cols.is_empty());
}
#[test]
fn non_finite_coordinates_never_leak_into_region_bounds() {
// An inf/NaN coordinate must not escape as a column boundary: callers
// treat these as page/column edges.
for bad_x in [f32::INFINITY, f32::NEG_INFINITY, f32::NAN] {
let mut items = Vec::new();
for i in 0..24 {
items.push(make_item(1, i as f32 * 10.0, 700.0 - i as f32 * 5.0, "A"));
}
items.push(make_item(1, bad_x, 700.0, "Z"));
for col in detect_columns(&items, 1, false) {
assert!(
col.x_min.is_finite() && col.x_max.is_finite(),
"bad_x {bad_x} leaked bounds {}..{}",
col.x_min,
col.x_max
);
}
}
}
#[test]
fn all_non_finite_coordinates_yield_no_columns() {
let items: Vec<TextItem> = (0..24)
.map(|i| make_item(1, f32::NAN, 700.0 - i as f32 * 5.0, "A"))
.collect();
assert!(detect_columns(&items, 1, false).is_empty());
}
#[test]
fn one_bad_item_does_not_disable_column_detection() {
// A single stray item should not collapse a clean two-column page to
// one region. Every gutter threshold is a fraction of the page width,
// so an untrimmed outlier pushes real gutters inside the rejected
// margin band. A malformed *width* at an ordinary position poisons the
// bounds just as a malformed position does.
for (label, bad_x, bad_width) in [
("nan position", f32::NAN, 0.0),
("inf position", f32::INFINITY, 0.0),
("far position", 50_000.0, 0.0),
("very far position", 1e12, 0.0),
("huge width", 100.0, 1e12),
("inf width", 100.0, f32::INFINITY),
] {
let mut items = Vec::new();
items.extend(fill_zone(1, 30.0, 280.0, 750.0, 50.0));
items.extend(fill_zone(1, 320.0, 570.0, 750.0, 50.0));
let mut bad = make_item(1, bad_x, 400.0, "Z");
bad.width = bad_width;
items.push(bad);
let cols = detect_columns(&items, 1, false);
assert_eq!(
cols.len(),
2,
"{label}: expected 2 columns, got {}",
cols.len()
);
for col in &cols {
assert!(
col.x_max - col.x_min <= MAX_PAGE_EXTENT_FOR_TEST,
"{label}: region {}..{} exceeds one page",
col.x_min,
col.x_max
);
}
}
}
/// Mirrors `MAX_PAGE_EXTENT` in `detect_columns`.
const MAX_PAGE_EXTENT_FOR_TEST: f32 = 14_400.0;
#[test]
fn very_wide_page_keeps_full_histogram_coverage() {
// Beyond MAX_BINS * BIN_WIDTH (~131k points) the bins must widen rather
// than stop covering the page. Three zones: the first gutter is inside
// the old coverage limit, the second is past it. Because the first
// gutter is found, the XY-cut fallback never runs, so a truncated
// histogram silently reports two columns instead of three.
let mut items = Vec::new();
items.extend(fill_zone(1, 0.0, 60_000.0, 750.0, 700.0));
items.extend(fill_zone(1, 70_000.0, 140_000.0, 750.0, 700.0));
items.extend(fill_zone(1, 160_000.0, 200_000.0, 750.0, 700.0));
let cols = detect_columns(&items, 1, false);
assert_eq!(
cols.len(),
3,
"Expected 3 columns across a 200k-wide page, got {}",
cols.len()
);
assert!(
(140_000.0..=160_000.0).contains(&cols[1].x_max),
"second gutter at {}, expected inside the real 140k..160k gap",
cols[1].x_max
);
}
#[test]
fn large_page_with_legitimately_long_runs_is_kept() {
// On a very large page, individual runs can exceed one ordinary page's
// width. They are real content, so they must not be judged malformed:
// the page keeps its columns and its full right edge.
let mut items = Vec::new();
for row in 0..30 {
let y = 750.0 - row as f32 * 14.0;
let mut left = make_item(1, 0.0, y, "Left run");
left.width = 20_000.0;
let mut right = make_item(1, 25_000.0, y, "Right run");
right.width = 20_000.0;
items.extend([left, right]);
}
let cols = detect_columns(&items, 1, false);
assert!(
!cols.is_empty(),
"a page of long-but-valid runs must still report a layout"
);
let right_edge = cols
.iter()
.map(|c| c.x_max)
.fold(f32::NEG_INFINITY, f32::max);
assert!(
right_edge > 44_000.0,
"long runs were treated as malformed: right edge {right_edge}, expected ~45_000"
);
}
#[test]
fn sparse_far_sidebar_on_a_large_page_is_kept() {
// A large-format page with a thin, sparsely-populated sidebar far from
// the main block. The sidebar is a small minority of the items, so an
// item-count rule alone would discard it — but nothing about its
// geometry says it is invalid, so its bounds must survive.
let mut items = Vec::new();
items.extend(fill_zone(1, 0.0, 12_000.0, 750.0, 500.0));
for i in 0..12 {
items.push(make_item(1, 24_000.0, 750.0 - i as f32 * 14.0, "Sidebar"));
}
let cols = detect_columns(&items, 1, false);
let right_edge = cols
.iter()
.map(|c| c.x_max)
.fold(f32::NEG_INFINITY, f32::max);
assert!(
right_edge > 24_000.0,
"sidebar was trimmed away: right edge {right_edge}, expected >24_000"
);
}
#[test]
fn genuinely_wide_layout_keeps_its_true_bounds() {
// A large-format page whose content really is spread beyond one
// ordinary page must not be trimmed to the median cluster: its far
// items are the majority, not strays.
let mut items = Vec::new();
items.extend(fill_zone(1, 100.0, 20_000.0, 750.0, 600.0));
items.extend(fill_zone(1, 22_000.0, 40_000.0, 750.0, 600.0));
let cols = detect_columns(&items, 1, false);
let widest = cols
.iter()
.map(|c| c.x_max)
.fold(f32::NEG_INFINITY, f32::max);
assert!(
widest > 35_000.0,
"wide layout was trimmed: right edge {widest}, expected ~40_000"
);
}
#[test]
fn oversized_but_legal_page_is_not_trimmed() {
// A wide-format page well inside the 14_400pt spec limit must keep its
// real bounds — outlier trimming is only for spans beyond a legal page.
let mut items = Vec::new();
items.extend(fill_zone(1, 100.0, 4_000.0, 750.0, 400.0));
items.extend(fill_zone(1, 4_400.0, 8_000.0, 750.0, 400.0));
let cols = detect_columns(&items, 1, false);
assert_eq!(cols.len(), 2, "Expected 2 columns, got {}", cols.len());
assert!(
cols[1].x_max > 7_000.0,
"right column should keep its true extent, got {}",
cols[1].x_max
);
}
#[test]
fn two_column_regression_guard() {
// Standard 2-column layout with clear gutter at center
+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);
}
}
+40 -3
View File
@@ -4566,9 +4566,13 @@ pub fn glyph_to_char(name: &str) -> Option<char> {
}
}
// Try to parse uniXXXX format
if name.starts_with("uni") && name.len() >= 7 {
if let Ok(code) = u32::from_str_radix(&name[3..7], 16) {
// Try to parse uniXXXX format.
// Use `get` rather than a byte-length check + slice: `name` can contain
// non-ASCII bytes (e.g. U+FFFD from lossy UTF-8 decoding of an attacker
// controlled /Differences name), so byte index 7 may not be a char
// boundary and `&name[3..7]` would panic.
if let Some(hex) = name.strip_prefix("uni").and_then(|rest| rest.get(..4)) {
if let Ok(code) = u32::from_str_radix(hex, 16) {
// Strip PUA F000 offset: uniF0XX → U+00XX (Windows Symbol encoding convention)
let code = if (0xF000..=0xF0FF).contains(&code) {
code - 0xF000
@@ -4588,3 +4592,36 @@ pub fn glyph_to_char(name: &str) -> Option<char> {
None
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn uni_hex_parsing() {
assert_eq!(glyph_to_char("uni0041"), Some('A'));
assert_eq!(glyph_to_char("uni00e9"), Some('\u{00e9}'));
// PUA F0xx symbol-encoding offset is stripped.
assert_eq!(glyph_to_char("uniF041"), Some('A'));
}
#[test]
fn u_hex_parsing() {
assert_eq!(glyph_to_char("u0041"), Some('A'));
assert_eq!(glyph_to_char("u1F600"), Some('\u{1F600}'));
}
#[test]
fn non_ascii_uni_name_does_not_panic() {
// A crafted /Differences name like `/uni#80#80#80#80` decodes via
// from_utf8_lossy into "uni" followed by four U+FFFD replacements.
// Byte index 7 lands mid-character, so a naive `&name[3..7]` slice
// would panic. It must be handled gracefully instead.
let crafted = format!("uni{0}{0}{0}{0}", '\u{FFFD}');
assert_eq!(glyph_to_char(&crafted), None);
// Assorted non-ASCII bytes right after the "uni" prefix.
assert_eq!(glyph_to_char("uni\u{FFFD}bc"), None);
assert_eq!(glyph_to_char("uni\u{00e9}00"), None);
}
}
+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;
+19 -1
View File
@@ -594,7 +594,7 @@ fn hex_to_unicode_string(hex: &str) -> Option<String> {
let bytes: Option<Vec<u8>> = (0..hex.len())
.step_by(2)
.map(|i| u8::from_str_radix(&hex[i..i + 2], 16).ok())
.map(|i| u8::from_str_radix(hex.get(i..i + 2)?, 16).ok())
.collect();
let bytes = bytes?;
@@ -2606,6 +2606,24 @@ endcmap
assert_eq!(cmap.lookup(0x0025), Some("B".to_string()));
}
#[test]
fn test_hex_to_unicode_non_ascii_no_panic() {
// A destination containing a multi-byte char makes the byte length even
// while a byte offset can land inside a char. Slicing must not panic;
// it should be rejected gracefully.
assert_eq!(hex_to_unicode_string("XéY"), None);
assert_eq!(hex_to_unicode_string("\u{fffd}0"), None);
}
#[test]
fn test_parse_bfchar_non_ascii_destination_no_panic() {
// Crafted /ToUnicode CMap: a non-hex, non-ASCII destination previously
// triggered a char-boundary panic in hex_to_unicode_string.
let cmap_content = "beginbfchar <0041> <XéY> endbfchar";
// Must not panic; the malformed entry is simply skipped.
let _ = ToUnicodeCMap::parse(cmap_content.as_bytes());
}
#[test]
fn test_parse_bfchar_1byte() {
// This is the pattern that caused the CJK bug: codespace is <0000><FFFF>
+2 -2
View File
@@ -724,7 +724,7 @@ checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e"
[[package]]
name = "pdf-inspector"
version = "0.1.7"
version = "0.1.8"
dependencies = [
"env_logger",
"include_dir",
@@ -740,7 +740,7 @@ dependencies = [
[[package]]
name = "pdf-inspector-wasm"
version = "0.1.3"
version = "0.1.4"
dependencies = [
"console_error_panic_hook",
"js-sys",
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "pdf-inspector-wasm"
version = "0.1.3"
version = "0.1.4"
edition = "2021"
authors = ["Firecrawl Team"]
description = "Browser WebAssembly bindings for pdf-inspector"