vbuch / node-signpdf

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

Unable to save a valid version of the signed PDF file #58

Closed radoslavpetranov closed 4 years ago

radoslavpetranov commented 4 years ago

Describe the bug and the expected behaviour Here is a test function that I use to test signing a dummy PDF document with a dummy certificate.

export const testFileSigning = (_req: Request, res: Response, next: NextFunction) => {

  // Hardcode some paths for testing purposes
  const pdfFilePath = "c:/temp/cert/cert.pdf";
  const pdfSignedFilePath = "c:/temp/cert/cert_signed.pdf";
  const certFilePath = "c:/temp/cert/certificate.p12";

  // Create some streams
  const p12Buffer = fs.readFileSync(certFilePath);
  let pdfBuffer = fs.readFileSync(pdfFilePath);

  // Add placeholder
  pdfBuffer = plainAddPlaceholder.default({
    pdfBuffer,
    reason: 'I have reviewed it.',
    signatureLength: 8192,
  });
  try {
    // Sign the doc
    pdfBuffer = signer.sign(pdfBuffer, p12Buffer);
  } catch (err) {
    console.error(`Error signing document`, err);
    next(err);
  }

  // Write to file
  fs.writeFile(pdfSignedFilePath, pdfBuffer, { encoding: 'binary' }, (err) => {
    if (!!err) {
      next(err);
    } else {
      res.locals.data = { signed: true };
      next();
    }
  });
};

So fs.writeFile saves the pdfBuffer that results from the sign method into a file called cert_signed.pdf.

When I open the file, the file is blank, it's broken / tiny so that it's zoomed in to like 4000%

image

and when I try to zoom out I see that error

There was a problem reading this document (14)

image

and the file itself seems to be corrupt / empty.

Is it a bug in signing or in the helpers? I am not even sure if this is a bug or if I'm doing something wrong when trying to save the signed file. If it's a bug I would assume it's a bug with the sign function.

To Reproduce I've described how to reproduce the issue in the previous points. I am testing with the certificate.p12 file that comes in this repo. I have also attached the 2 PDF files: the source cert.pdf file as well as the resulting cert_signed.pdf file.

cert.pdf cert_signed.pdf

Your help would be appreciated. I saw someone else pointing to this error in a closed github issue so I figured I would make a separate issue for anyone facing the same problem as me.

radoslavpetranov commented 4 years ago

Interestingly enough, here's what the cert_signed.pdf file looks like in chrome

2020-02-03_19h38_38

and here what it looks like in Adobe Acrobat Reader DC

image

vbuch commented 4 years ago

In the resulting PDF I see

<<

/Type /Catalog
/Pages 1 0 R

/AcroForm 17 0 R
>>
endobj

6 0 obj
<<
 16 0 R]
/Contents 4 0 R
/Resources 5 0 R

>>
endobj

This looks wrong. Not sure what got broken there without going through code. What version are you using? Is it 1.2.2?

Object 6 was the page. In the original document it was OK. After signing, it's crappy. Save the pdfBuffer after plainAddPlaceholder so we can confirm if the issue is in it.

radoslavpetranov commented 4 years ago

Thanks for looking into this! Yes, I am using version 1.2.2.

Here's the updated code that saves the file after adding the placeholder

// Hardcode some paths for testing purposes
const pdfFilePath = "c:/temp/cert/cert.pdf";
const pdfWithPlaceholderFilePath = "c:/temp/cert/cert_placeholder.pdf";

// Create some streams
let pdfBuffer = fs.readFileSync(pdfFilePath);

// Add placeholder
pdfBuffer = plainAddPlaceholder.default({
  pdfBuffer,
  reason: 'I have reviewed it.',
  signatureLength: 8192,
});
fs.writeFile(pdfWithPlaceholderFilePath, pdfBuffer, { encoding: 'binary' }, (err) => {
  if (!!err) {
    next(err);
  } else {
    res.locals.data = { signed: true };
    next();
  }
});

And the resulting file is attached.

cert_placeholder.pdf

vbuch commented 4 years ago

OK. Confirmed. It's an issue with plainAddPlaceholder. The page object gets broken through it. Would love it if you can make a PR with a fix. Otherwise you'll have to wait as I don't have the time to debug it.

radoslavpetranov commented 4 years ago

I'm no PDF expert but I'll see if I can figure out what's going on.

I just used a hex editor and I confirm that if I change the broken obj part from

6 0 obj
<<
 16 0 R]
/Contents 4 0 R
/Resources 5 0 R

>>
endobj

to

6 0 obj
<<
/Type /Page
/Parent 1 0 R
/MediaBox [0 0 595.28 841.89]
/Contents 4 0 R
/Resources 5 0 R
>>
endobj

the file gets properly rendered

image

vbuch commented 4 years ago

And this resulting one you will be able to sign. The problem is somewhere in plainaddplaceholder. Somewhere where this /type /page object is written back in the pdf. Probably around here: https://github.com/vbuch/node-signpdf/blob/develop/src/helpers/plainAddPlaceholder/createBufferPageWithAnnotation.js

vbuch commented 4 years ago

@radoslavpetranov (now that I wrote your name I am pretty sure you are also a Bulgarian :-)) Can try and check the code in #59 I will get this to 1.2.3 later today but if you could confirm meanwhile, would be really nice. Thanks.

radoslavpetranov commented 4 years ago

Guilty as charged :)

I just tested your changes and everything works great now! Thanks a lot for figuring this out!!

image

vbuch commented 4 years ago

Have not released yet. Just merged into develop.

vbuch commented 4 years ago

1.2.3 is out in npm. Hope it fixes this issue.