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. Applied identically to both the legacy (com.lowagie) and modern (org.openpdf) copies of PdfNumber, since this branch ships both. 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, to both modules. 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>
e4c66a5 to
94f3a8f
Compare
|
|
Hi @asturio, This PR backports the fix preventing Heads up: the SonarCloud duplication gate is red here, but it's not a code-quality issue — this PR touches the 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. Applied identically to
both the legacy (
com.lowagie) and modern (org.openpdf) copies ofPdfNumberthis branch ships.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),to both modules. Full test suite run locally, no regressions (only 3
pre-existing, unrelated failures from a missing
OS/2table in the localmacOS
Courier.ttcsystem font).🤖 Generated with Claude Code