raise_on_encrypted: true

[leviwilson]

This pull request adds a raise_on_encrypted: true option to CombinePDF.load. In this manner, a consumer can choose to have CombinePDF throw an exception in the event that the PDF they are trying to open is encrypted.

In addition to this, sometimes during the CombinePDF::PDFParser#parse, it can come across a scenario where it raises a Zlib::DataError but was still able to obtain some information about the file in the root_object such as whether or not there is root_object[:Encrypt] information in it. For those conditions, rather than raising a Zlib::DataError it will raise the CombinePDF::EncryptionError instead but only if the raise_on_encrypted: true has been set. Otherwise, it'll allow the Zlib::DataError to bubble up.

[leviwilson]

@boazsegev any feedback on these changes?

[boazsegev]

Sorry, busy days... I'll look it over soon... I hope.

[takahashim]

This PR is great! 👏
The :raise_on_encrypted option and CombinePDF::EncryptionError are exactly what I was looking for. I'm very happy to see these two features merged.

On the other hand, there are some things I've been wondering about in the PR.

• test/combine_pdf/load_test.rb is spec style, but test/combine_pdf/renderer_test.rb is Minitest::Test style. I don't think it's very desirable to have two specs with different styles in there.
• Fixing the Rakefile for test runs is great, but it solves a different problem than encryption and EncryptionError. I think it would be better to separate it into another Pull Request.
  • Ditto for adding the .travis.yml.

[leviwilson]

• test/combine_pdf/load_test.rb is spec style, but test/combine_pdf/renderer_test.rb is Minitest::Test style. I don't think it's very desirable to have two specs with different styles in there.

I've never used Minitest before but the load_test.rb had more verifications which left me seeking additional Minitest functionality, but I still stuck within Minitest

• Fixing the Rakefile for test runs is great, but it solves a different problem than encryption and EncryptionError. I think it would be better to separate it into another Pull Request.

It's still related to the PR since when I went to run specs, there was no way to run all of them. Thought it would be best to make it a default rake task like other projects.

• Ditto for adding the .travis.yml.
  I'm indifferent. Seems innocuous enough to not warrant another PR.

Thanks for the feedback / looking at it!

[leviwilson]

@boazsegev just checking in on this. It look good?

[reiz]

@boazsegev @leviwilson that would be a great feature.

[kimyu92]

@boazsegev Do you mind to take a look on this feature to at least allow user to stop proceeding until a better decryption is implemented?

There is no way user can stop proceeding encrypted pdf without able to ingest flag

[boazsegev]

Really nice work. I apologize for the (extremely) belated response.

I would rather avoid the catch-and-release for the Zlib::DataError exception unless it is always true that the exception is caused by an encryption error (and I think it could be caused by a PDF object compression error as well)...

Is it possible to remove this before a merge?

Thanks!

[boazsegev]

Thank you every one for the work you put in on this :)

I'll test and push the patch as soon as I have a moment.

[boazsegev]

released @ 1.0.23