Repository navigation
Conversation
validate_pdf_structure refused every PDF with is_encrypted set, which is also true for a PDF with an empty user password and only an owner password: it opens in every reader and only restricts printing or copying, which is how publishers commonly ship article PDFs. Try the empty password and refuse only when pypdf reports NOT_DECRYPTED, with the message unchanged. No other password is tried. The page limit and the page-tree walk still run after decryption. pypdf checks the empty password without AES but needs an optional crypto backend to decrypt AES content, and the API has none that pypdf detects. Decrypt the first page's content once, and refuse a file this server cannot decrypt as encrypted (pypdf's DependencyError) instead of storing it without text. That also replaces "not a structurally valid PDF" for AES-256 files. Closes #115 Signed-off-by: L4XB <lukas.buck@e-mail.de>
Member
Author
|
I have read and agree to the SixSentences CLA v1.0. |
3 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #115
Summary
validate_pdf_structurerefused every PDF whoseis_encryptedflag is set. That flag is also true for a PDF with an empty user password and only an owner password, which opens in every reader and merely restricts printing or copying. This is how publishers commonly ship article PDFs. Such a file was refused with "encrypted PDFs are not supported".The check now calls
reader.decrypt("")and refuses only when pypdf returnsPasswordType.NOT_DECRYPTED, with the message unchanged. No other password is tried, and none is accepted, stored or logged. The page-count limit and the page-tree walk run after decryption exactly as before.One case the issue did not mention: pypdf checks the empty password without AES, but it can only decrypt AES-encrypted content through an optional crypto backend (
cryptographyorpycryptodome). The API shipspycryptodomex, whoseCryptodomenamespace pypdf does not look for, so pypdf runs on its RC4-only fallback here. Without a guard, an AES-128 owner-only PDF would pass validation and then be stored with no text. For an encrypted file, the check therefore also decrypts the first page's content stream once. ADependencyErrorfrom pypdf now maps to "encrypted PDFs are not supported". Before, an AES-256 file was refused as "not a structurally valid PDF".Behavior and compatibility
Only
services/api/src/sixsentences_server/acquisition/upload.py(validate_pdf_structure) changes in source. Measured on synthetic files built with pypdf 6.19.0 (the locked version), using the API's locked environment:mainparsedWith
cryptographyadded to the same environment, the same code accepts the AES-128 and AES-256 owner-only files with textparsedand still refuses every user-password file. Making AES work therefore needs only a crypto backend for pypdf. CONTRIBUTING asks for an issue before a new dependency, so that is not part of this change. The proposal with these measurements is #209.No API, event, schema or migration change. Rollback is a revert.
Validation
ruff format --checkontests/test_documents.pyreports the same five hunks as onmain, all in older tests. The lines added here are formatted.New tests in
services/api/tests/test_documents.pybuild every PDF in the test from_mini_pdfand encrypt it with RC4-128, which pypdf writes and reads without a crypto backend:text_status == "parsed", and its title comes from the extracted text;DependencyError, as AES does without a backend, is refused as encrypted.Five mutants of the change each fail at least one of these tests: refusing every encrypted PDF (as on
main), accepting every encrypted PDF, dropping the content check, dropping theDependencyErrormapping, and skipping the page checks once a file is decrypted.Engine, worker, migrations, web app, browser extension, Companion and self-hosting are unaffected.
Review boundaries
The decrypt attempt runs before bytes are persisted or handed to another parser, like the rest of this function. No password input is added, and the only value ever passed to
decryptis the empty string. Bytes reaching the extractor are the uploaded bytes, unchanged. The extractors (PdfTextExtractorand the page helpers inacquisition/pdf.py) already open such files through pypdf's automatic empty-password attempt, so they need no change. No dependency, network call or processor is added.Source-release hygiene
Every test file is generated in the test. No publisher PDF, upload or other data is committed.
CHANGELOG.mdhas an entry under Unreleased. The commit is signed and carries my DCO sign-off, and the CLA acceptance sentence follows as a standalone comment.Visual evidence
None. No UI change.