Skip to content

improvement(file-parsers): bound PDF text extraction - #6425

Merged
waleedlatif1 merged 2 commits into
stagingfrom
worktree-pdf-text-bomb
Aug 8, 2026
Merged

improvement(file-parsers): bound PDF text extraction#6425
waleedlatif1 merged 2 commits into
stagingfrom
worktree-pdf-text-bomb

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Extract PDF page text through pdf.js's streaming API with page, character, and wall-clock budgets, instead of buffering the whole document at once — extraction memory is now bounded regardless of input
  • Release the document proxy after parsing so pdf.js's page/font/image caches don't linger
  • Route output through sanitizeTextForUTF8, matching every other parser in the folder
  • Add truncated to FileParseMetadata (csv and xlsx already emitted it untyped)

Type of Change

  • Improvement

Testing

Added pdf-parser.test.ts (the parser had none). Verified output is byte-identical to the previous implementation on real multi-page PDFs, and that the new tests fail against the old code.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Extract page text through pdf.js's streaming API with page, character, and
wall-clock budgets instead of buffering the whole document, so extraction
memory stays bounded regardless of input. Release the document proxy when
done, and route output through sanitizeTextForUTF8 like the other parsers.
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 8, 2026 7:15pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes PDF ingestion on a shared server path with new truncation semantics callers may need to handle, but the goal is reducing DoS/crash risk from unbounded extraction.

Overview
Replaces whole-document PDF text buffering with per-page streaming via pdf.js streamTextContent, so malicious or pathological PDFs (e.g. compression bombs) cannot expand to heap exhaustion and crash the process.

Extraction is capped by page count (10k), total characters (~10M), and a 60s wall-clock deadline. When a limit bites, metadata sets truncated and a warning, matching how CSV/XLSX already signaled partial reads (now typed on FileParseMetadata).

Output goes through sanitizeTextForUTF8 instead of only stripping null bytes, and the document proxy is destroy()ed after parsing to drop pdf.js caches. New Vitest coverage includes a hand-built FlateDecode bomb fixture and a small-PDF happy path.

Reviewed by Cursor Bugbot for commit 3420641. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bounds PDF text extraction by page count, character count, and elapsed time while releasing pdf.js resources after parsing.

  • Streams page text instead of buffering complete documents.
  • Reports truncation through typed parser metadata.
  • Sanitizes extracted PDF text consistently with the other file parsers.
  • Adds compression-bomb and normal-extraction coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/file-parsers/pdf-parser.ts Replaces whole-document extraction with budgeted streaming extraction, sanitizes output, reports truncation, and destroys the document proxy.
apps/sim/lib/file-parsers/pdf-parser.test.ts Adds coverage for bounded extraction of a highly compressed text PDF and complete extraction of a small PDF.
apps/sim/lib/file-parsers/types.ts Adds the optional typed truncation indicator already used by parser metadata.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Input[PDF buffer] --> Proxy[Create pdf.js document proxy]
  Proxy --> Pages[Visit pages within page limit]
  Pages --> Stream[Stream page text]
  Stream --> Budget{Character or time budget reached?}
  Budget -->|No| Pages
  Budget -->|Yes| Truncated[Mark output truncated]
  Pages -->|Document exhausted| Complete[Mark output complete]
  Truncated --> Sanitize[Sanitize UTF-8 text]
  Complete --> Sanitize
  Sanitize --> Result[Return content and metadata]
  Result --> Destroy[Destroy document proxy]
Loading

Reviews (2): Last reviewed commit: "fix(file-parsers): only flag truncation ..." | Re-trigger Greptile

Comment thread apps/sim/lib/file-parsers/pdf-parser.ts Outdated
Comment thread apps/sim/lib/file-parsers/pdf-parser.ts
Comment thread apps/sim/lib/file-parsers/pdf-parser.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3420641. Configure here.

@waleedlatif1
waleedlatif1 merged commit 441004a into staging Aug 8, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the worktree-pdf-text-bomb branch August 8, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant