Skip to content

Write the embedded file CheckSum as a hex string of the raw MD5 bytes - #1821

Open
HawkForge wants to merge 1 commit into
foliojs:masterfrom
HawkForge:fix/attachment-checksum-bytes
Open

HawkForge wants to merge 1 commit into
foliojs:masterfrom
HawkForge:fix/attachment-checksum-bytes

Conversation

@HawkForge

Copy link
Copy Markdown

What kind of change does this PR introduce?

Bug fix. Fixes the /CheckSum part of #1531. Thanks to @petervwyatt for the report.

The embedded file stream's /Params /CheckSum was written as new String(md5Hex(data)), a literal string holding the 32 hex characters. The PDF specification defines the checksum as a 16-byte string holding the MD5 digest, so veraPDF and pdfcpu reject the 32-byte literal.

The checksum is now kept as the digest bytes from md5Hash, which PDFObject.convert serialises as a hex string (/CheckSum <...>). The duplicate-attachment check in isEqual compares the bytes instead of calling toString().

The existing expectations in tests/unit/attachments.spec.js now expect /CheckSum <md5hex>; three of them fail without the fix. The full suite passes (550 tests), and eslint and prettier are clean on the changed files.

One thing to be aware of: PDFObject.convert writes Uint8Array values without passing them through encryptFn (that is what keeps the encryption dictionary's /O and /U entries intact), so in an encrypted document the checksum is now written unencrypted. PDF/A, where validators check this entry, doesn't allow encryption, so the reported case is not affected. If you'd prefer encrypted documents to get an encrypted hex string here, I'm happy to add that, but it needs a way to tell those byte strings apart from the encryption dictionary ones, so I left it out of this change.

The "default to PDF 1.7" part of the issue is left for the maintainers to decide.

Checklist:

  • Unit Tests
  • Documentation
  • Update CHANGELOG.md
  • Ready to be merged

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant