Armour headers support in encryptor and decryptor

[lgoldstein]

Note that I am using Java 8 constructs since they greatly simplify the code - I hope it's not a problem. In any case, I recommend declaring (and enforcing in build setup) usage of 1.8 as the minimum version (as of next release) since it is the most widely used version (the older ones are pretty obsolete).

[justinludwig]

Sorry for taking so long to respond -- thanks for this! One thing I did want to ask you about was, for the last commit, what do you think about adding some additional encrypt() methods to the Encryptor class with per-file armor headers as an additional argument (eg encrypt(InputStream plaintext, OutputStream ciphertext, FileMetadata meta, Map armorHeaders)), instead of using a callback to provide per-file headers? I'm wondering if that might help make using the encryptor API a little more self-explanatory (sometimes with callbacks I find I need to read through a code example or two to wrap my head around how the callback should be used)?

Re java 8, in the past, I had been trying to keep the library compatible with java 7 -- but I think you're right, it's 2020 and time to move on and make java 8 the minimum.

[lgoldstein]

what do you think about adding some additional encrypt() methods to the Encryptor class with per-file armor headers as an additional argument (eg encrypt(InputStream plaintext, OutputStream ciphertext, FileMetadata meta, Map armorHeaders)), instead of using a callback to provide per-file headers?

I thought about it, but then (IMO) it might not be clear whether these headers are instead or in addition to the global ones. Furthermore, how do they interact with the Version header that ArmoredOutputStream automatically adds ? Are they instead or in addition to it ?

sometimes with callbacks I find I need to read through a code example or two to wrap my head around how the callback should be used

I do see your point, but because this is an "extraordinary" API (IMO) it increases the chances of whoever is using it to pay attention to its documentation.

In any case - I defer to your judgement as the maintainer of this project. If you prefer the extra encrypt methods I will replace the callback with them - but let;s make sure of the exact semantics of the armored headers argument (extra or instead, Version replaced or not...)

[lgoldstein]

Here is some more food for thought regarding the replacement of the callback with encrypt methods having extra Map parameter:

• We need quite a few more extra overloads - for ease of use - thus "cluttering" the class
• We can keep the callback for advanced users and add encrypt overloads. If you do opt for this option I recommend we merge this code and add the encrypt overloads in a separate PR.
• (clincher IMO) I don't see an easy way to remove a global header via an entry in the Map. null won't do for many reasons
  • Some maps won't accept them
  • Code cannot easily distinguish between missing entry and one mapped to null

We would need a "special" marker as a null placeholder - making such use also not straightforward

[justinludwig]

Thanks for your thoughts -- all good points. You're right, adding those extra encrypt overloads instead might make things more complicated rather than simpler. I'll go ahead and merge it in as is.