Add option for leaving private key stream opened

[tomasweigenast]

I often run into a problem when I execute the following line of code:

using (PGP pgp = new PGP()) {
       await pgp.DecryptStreamAsync(inputStream, outputStream, mPrivateKeyStream, mPassword);
}

Because the private key gets disposed and I cannot seek it to the beginning to decrypt the next message

[mattosaurus]

Hi, I'm actually currently looking at adding functionality to load the keys once at initialization so they can be reused later if required which should fix this issue. I'll hoppefully push out a new update this week or next.

[mattosaurus]

Hi, you can create an instance of the PGP object that holds the keys in memory now like so.

Hopefully this will fix your issue as you now only need to read the key once.

[toddlucas]

Hi, I'm having this same issue. The EncryptStream methods don't dispose of the public key, but DecryptStream methods do. We create new instances of the PGP class so we can't use the singleton approach.

[mattosaurus]

I defaulted the ReadSecretKey methods in EncryptionKeys to leave the stream open which fixed this issue but then caused some other ones so I reverted it back. I think this will need to be an option as specified in the original issue so it doesn't break any existing code but that'll take a bit more time to implement. Hopefully I'll get it done this week or next.

[mattosaurus]

Thinking about it some more I believe that closing the key streams is the correct behaviour and that they shouldn't be left open and adding an additional option for this would just confuse things. The PGP object can be initialised once with key streams as in the example linked above (I still need to update the documentation for this) and then reused which removes the need for rewinding the streams and reading the keys in again.

If this doesn't work for whatever reason then let me know and I may reconsider.

[toddlucas]

After reading your post, your reasoning made sense. I decided to look for analogous patterns for comparison. One pattern I often come across looks something like:

using var resourceA = new ResourceA();
using var resourceB = new ResourceB(resourceA);

In these cases, I would expect resourceB not to own resourceA, or for resourceA to be OK with a double dispose. A common example of this pattern is file reading:

using var fs = new FileStream(filename, FileMode.Open, FileAccess.Read);
using var sr = new StreamReader(fs, Encoding.UTF8);

string text = sr.ReadToEnd();

I looked at the source for StreamReader and found that it actually takes ownership of the file stream, which isn't what I would expect.

Looking for some explanation of best practices, I found this post on Stack Overflow, which says:

A general rule is that if you created (or acquired ownership of) the object then it is your responsibility to dispose it.

Apparently StreamReader is one of the few classes in the BCL that goes against this general rule and performs what they call dispose ownership transfer with the constructor parameter.

Anyway, I thought this might be useful in your considerations. I'm OK with either approach. It would be ideal if encrypt and decrypt both do the same thing. As long as it's documented, there won't be any confusion.