vbuch / node-signpdf

Simple signing of PDFs in node.
MIT License
676 stars 174 forks source link

Special Words cause issue in signature #209

Closed bretto36 closed 4 months ago

bretto36 commented 7 months ago

Describe the bug and the expected behaviour I have been using this package for a while and have just updated to the latest version HOPING it would fix a bug i found in the older version. When you add a placeholder and the placeholder contains a special word in PDF's it errors out. I've found the when the word trailer exists it causes it to error out with the error Expected xref at NaN but found other content.

pdfBuffer = plainAddPlaceholder({
                    pdfBuffer: pdfBuffer,
                    reason: 'trailer', // <<<<<<<<<
                    contactInfo: 'email',
                    name: 'name',
                    location: 'Location Value',
                });

Is it a bug in signing or in the helpers? Not sure if it's a bug in the helper or the signing - @signpdf/placeholder-plain

To Reproduce This was my code to help reproduce. And the signer.details had email@semitrailerstuff.com i narrowed it down to the word trailer

for (const signer of signers) {
                pdfBuffer = plainAddPlaceholder({
                    pdfBuffer: pdfBuffer,
                    reason: signer.details,
                    contactInfo: signer.email ?? '',
                    name: signer.name ?? '',
                    location: signer.ip,
                });

                const p12Signer = new P12Signer(Buffer.from(certificateString));

                pdfBuffer = await signpdf.sign(pdfBuffer, p12Signer)
            }
vbuch commented 7 months ago

Oh wow! Quite annoying. Thank you for the report. I can pin point the problem in packages/placeholder-plain/src/readPdf.js

bretto36 commented 7 months ago

@vbuch Just as an add on, i think it was only when adding 2 signers. I just updated to latest version (from 3.0 to 3.1) and 1 signer works, but 2 doesn't when the FIRST signer has trailer in the reason

Thanks for the timely reply

vbuch commented 7 months ago

@bretto36 you are right. The error comes because it tries to find any previous trailers with a search for the keyword trailer before the last one. This shouldn't be needed as the last xref already stated where the previous one is and a search for "trailer" is only making things more complicated than they need to be. I will try to come up with a solution to the problem soon. Already have pieces of it in a branch but it's something that is sketchy work in progress.

bretto36 commented 4 months ago

Hey @vbuch just wanted to check in with you on this one. See if you got close to a resolution or not. Thanks

vbuch commented 4 months ago

Hey. Sorry. No update here. To be fair I had fogotten about this because of other additions that came in the lib recently. Will put it on my roadmap to try and resolve it. Hopefully an update later this week. Sorry again.

vbuch commented 4 months ago

@bretto36 this is now in v3.2.3. Sorry for making you wait.

I drink coffee and beer :)

bretto36 commented 4 months ago

@bretto36 this is now in v3.2.3. Sorry for making you wait.

I drink coffee and beer :)

Enjoy your drinks! Thanks again!