Conversation
PdfNumber#intValue() narrowed the underlying double with a plain (int) cast. Some PDF producers write the /Encrypt dictionary's /P (permissions) entry using its unsigned 32-bit decimal representation instead of the equivalent signed value (e.g. 4294965956 instead of -1340) - both encode the identical 4-byte value, and this is spec-legal. Java's narrowing double-to-int conversion saturates rather than wraps for an out-of-range value, so such a /P value was misread as Integer.MAX_VALUE instead of -1340. That corrupted the encryption key the standard security handler derives, so its own recomputed /U check failed for every password - including a correct, empty one - and PdfReader reported BadPasswordException for a document that was never actually password-protected. Route the cast through long first, which reproduces the expected low-order-32-bit truncation instead of clamping. Add PdfNumberTest covering the truncation directly, and DecryptUnsignedPermissionsTest, a PdfReader-level regression test using a crafted PDF whose /P is written this way with an empty user password. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
DecryptUnsignedPermissionsTest reached PdfReader's private ownerPasswordUsed field via reflection (setAccessible), which Codacy flagged as a high-severity visibility-alteration issue, plus a medium issue for wrapping the reflection exceptions in a raw RuntimeException. PdfReader already exposes a public isOwnerPasswordUsed() getter that returns the same field, so the reflection is unnecessary. Calling it directly returns the identical value and removes both findings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
daa1f5e to
fa72846
Compare
|
|
Hi @asturio, This PR backports the fix preventing Whenever you get a chance, I'd appreciate your review, and if it looks good, approval/merge into I'd also kindly ask, once merged, if an official release including this fix could be cut — particularly for the Java 8-compatible line (#1627), since I'd rather rely on an official release than patch it in locally. Thanks for maintaining OpenPDF! |
1 participant
Description of the new Feature/Bugfix
PdfNumber#intValue()narrowed the underlyingdoublewith a plain(int)cast. Some PDF producers write the/Encryptdictionary's/P(permissions) entry using its unsigned 32-bit decimal representation
instead of the equivalent signed value (e.g.
4294965956instead of-1340) - both encode the identical 4-byte value, and this is spec-legal.Java's narrowing double-to-int conversion saturates rather than wraps for
an out-of-range value, so such a
/Pvalue was misread asInteger.MAX_VALUEinstead of-1340. That corrupted the encryption keythe standard security handler derives, so its own recomputed
/Ucheckfailed for every password - including a correct, empty one - and
PdfReaderreportedBadPasswordExceptionfor a document that was neveractually password-protected.
Routed the cast through
longfirst, which reproduces the expectedlow-order-32-bit truncation instead of clamping.
Related Issue: none (found while investigating a user-reported PDF, not
tied to a filed issue)
Unit-Tests for the new Feature/Bugfix
Compatibilities Issues
No method signatures changed. Any caller previously relying on the
(incorrect) saturated value of an out-of-range
PdfNumber#intValue()wouldnow see the correctly truncated value instead - this is a correctness fix,
not expected to be a compatibility concern in practice.
Your real name
Diego Garcia
Testing details
Added
PdfNumberTest(direct truncation coverage) andDecryptUnsignedPermissionsTest(aPdfReader-level regression test usinga crafted PDF whose
/Pis written this way with an empty user password).Full module test suite run locally per branch, no regressions (only 3
pre-existing, unrelated failures on some branches, from a missing
OS/2table in the local macOS
Courier.ttcsystem font).🤖 Generated with Claude Code