vbuch / node-signpdf

Simple signing of PDFs in node.
MIT License
719 stars 178 forks source link

Duplicate startxref in some PDFs #76

Closed mgyugcha closed 4 years ago

mgyugcha commented 4 years ago

In some PDFs the /Root is the last key of the trailer so the search should be with / and also with >.

image

Explanation

If this is ignored the rootSlice has unwanted values:

Expected: image

Obtained: image

This causes the PDF to have an additional startxref:

image

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 297


Totals Coverage Status
Change from base Build 275: 0.0%
Covered Lines: 285
Relevant Lines: 285

💛 - Coveralls
therpobinski commented 4 years ago

Thanks, with this 'extraction request' this isuue is solved. We have been working hard with @mgyugcha to fix it, and everything has been satisfactory. Thanks @mgyugcha and congratulations. It is a great contribution.

vbuch commented 4 years ago

Great! Not sure if this is the best solution. Best would be to extract the dictionary contents and then either convert to a js Map or search only within the contents. Either way.

By reading the code I don't understand what this commit fixes. If I read through the issue, I do. If you write a test that covers that, it will be obvious when reading the code and code coverage will not be lowered, hence @coveralls won't complain. Please take the time to write a test so this can be merged.

mgyugcha commented 4 years ago

Hi @vbuch, please check the test I did.

Without this update the new trailer contains two startxref when generating the placeholder, as shown in the picture:

image

vbuch commented 4 years ago

Released in 1.3.0