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>

SECURITY.md

@@ -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

napi/README.md

@@ -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

napi/src/lib.rs

@@ -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,
+    })
+}

napi/test.mjs

@@ -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!');

src/extractor/links.rs

@@ -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);
+    }
 }

src/glyph_names.rs

@@ -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);
+    }
+}

src/tounicode.rs

@@ -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>