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>
Not up to standards ⛔
|
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>
ac945df to
5d27a07
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