Ensuring 404 PDF is not parsed into texts by jamesbraza · Pull Request #1126 · Future-House/paper-qa

jamesbraza · 2025-10-06T18:10:34Z

Let's ensure PDFs like:

<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->

Have two behaviors:

Don't crash us (the case for PyPDF)
Don't get considered valid by our system (the case for PyMuPDF)

dosubot · 2025-10-06T18:11:02Z

Related Documentation

Checked 1 published document(s). No updates required.

^{How did I do? Any feedback?}

Copilot

Pull Request Overview

This PR adds protection against malformed PDF files (specifically 404 HTML error pages being served as PDFs) by increasing the minimum text length validation for documents. The change also includes a test to verify that such invalid PDFs are properly rejected.

Changes

Increased minimum text length requirement from 10 to 20 characters (after removing newlines) to better filter out malformed PDFs
Added comprehensive test coverage for invalid PDF handling with two different PDF parsers

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/paperqa/docs.py	Modified validation logic to increase minimum text length requirement
tests/test_paperqa.py	Added test case to verify rejection of 404 HTML pages masquerading as PDFs

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

tests/test_paperqa.py

sidnarayanan · 2025-10-06T18:55:40Z

src/paperqa/docs.py

        if metadata.parse_type != "image" and (
            not texts
-            or len(texts[0].text) < 10  # noqa: PLR2004
+            or len(texts[0].text.replace("\n", "")) < 20  # noqa: PLR2004


Can you add a comment describing what motivates 20, and why we care that there are at least that many non-newline characters?

Yeah just did so. I also moved it into the disable_doc_valid_check check, it probably makes more sense there

…nted its rationale

jamesbraza · 2025-10-06T19:26:40Z

@cursor review

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/paper-qa-pypdf/tests/test_paperqa_pypdf.py

packages/paper-qa-pymupdf/tests/test_paperqa_pymupdf.py

cursor

✅ Bugbot reviewed your changes and found no bugs!

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

dosubot · 2025-10-06T19:42:47Z

Documentation Updates

Checked 1 published document(s). No updates required.

^{How did I do? Any feedback?}

jamesbraza requested review from maykcaldas, mskarlin, nadolskit, sidnarayanan and whitead October 6, 2025 18:10

jamesbraza self-assigned this Oct 6, 2025

Copilot AI review requested due to automatic review settings October 6, 2025 18:10

jamesbraza added the bug Something isn't working label Oct 6, 2025

dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 6, 2025

Copilot AI reviewed Oct 6, 2025

View reviewed changes

tests/test_paperqa.py Outdated Show resolved Hide resolved

tests/test_paperqa.py Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

jamesbraza force-pushed the bad-pdf-denial branch from 1112243 to bc6cfb3 Compare October 6, 2025 18:14

dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 6, 2025

jamesbraza force-pushed the bad-pdf-denial branch from bc6cfb3 to 467bdfe Compare October 6, 2025 18:36

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Oct 6, 2025

sidnarayanan approved these changes Oct 6, 2025

View reviewed changes

jamesbraza force-pushed the bad-pdf-denial branch from 467bdfe to 09c8132 Compare October 6, 2025 18:57

jamesbraza added 4 commits October 6, 2025 12:25

Ensuring 404 page is not parsed into text with PyPDF, with a test

26f99c2

Ensuring 404 page is not parsed into text with PyMuPDF, with a test

afa941e

Loosened pylint too-many-boolean-expressions a bit

cb96a5c

Moved 20-char check into disable_doc_valid_check umbrella, and docume…

ef02046

…nted its rationale

jamesbraza force-pushed the bad-pdf-denial branch from 09c8132 to ef02046 Compare October 6, 2025 19:25

jamesbraza requested a review from Copilot October 6, 2025 19:26

Copilot AI reviewed Oct 6, 2025

View reviewed changes

packages/paper-qa-pypdf/tests/test_paperqa_pypdf.py Show resolved Hide resolved

packages/paper-qa-pymupdf/tests/test_paperqa_pymupdf.py Show resolved Hide resolved

cursor bot reviewed Oct 6, 2025

View reviewed changes

jamesbraza merged commit 294c9b0 into main Oct 6, 2025
6 checks passed

jamesbraza deleted the bad-pdf-denial branch October 6, 2025 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensuring 404 PDF is not parsed into texts#1126

Ensuring 404 PDF is not parsed into texts#1126
jamesbraza merged 4 commits intomainfrom
bad-pdf-denial

jamesbraza commented Oct 6, 2025 •

edited

Loading

Uh oh!

dosubot bot commented Oct 6, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

sidnarayanan Oct 6, 2025

Uh oh!

jamesbraza Oct 6, 2025

Uh oh!

jamesbraza commented Oct 6, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

dosubot bot commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jamesbraza commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dosubot bot commented Oct 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Changes

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

sidnarayanan Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

jamesbraza Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

jamesbraza commented Oct 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

✅ Bugbot reviewed your changes and found no bugs!

Uh oh!

Uh oh!

dosubot bot commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamesbraza commented Oct 6, 2025 •

edited

Loading