justinludwig / jpgpj

Java Pretty Good Privacy Jig
MIT License
74 stars 20 forks source link

Added mutable max buffer size for Encryptor and Decryptor #15

Closed amorillas closed 6 years ago

amorillas commented 6 years ago

These are the changes that we talked about in Issue #14 . I haven't included any change in versions, as I thought you would want to do it when you are happy for releasing a new one.

I'm not sure if the buffer size must be a power of 2, but I've made this assumption.

Any comment or correction is welcome!

justinludwig commented 6 years ago

Thanks for the pull request! I have a few suggestions about the property name, and about the sizing of the buffers:

  1. For the property name (and corresponding getters/setters), how about calling it maxFileBufferSize instead of just maxBufferSize, to better distinguish it from other internal buffers?

  2. For the input file buffer size, it doesn't actually need to be a power of 2, so using the file's length exactly should be just fine.

  3. For the output file buffer size, it's hard to predict the size needed, because of compression. But if compression has been turned off, or if the file is already well compressed, then the encrypted file will be a little bit bigger than the decrypted file (because of the extra PGP headers and packeting). And if ASCII-armoring is used, then the encrypted file will be about an additional 1/3 bigger than the decrypted file (because of Base64 encoding).

    I think it's better to over-size the buffer somewhat, rather than under-size it, so for decryption, I think using the same size for the output file buffer as the input is the best way to go. But for encryption, I'd suggest using a formula like this for determining the best output buffer size:

    output buffer size = input file size
    output buffer size += (1 + number of encryption keys + number of signing keys) * 512
    if (ascii armored) {
    output buffer size *= 4/3 * (64 + line separator length)/64
    output buffer size += 80
    }

My reasoning is that each key with which the message is encrypted is going to add about 500 bytes, and each signature is going to add about another 500 bytes, and the rest of the various headers are going to add about another 500 bytes -- so I'd add 512 bytes to the buffer size for each of those things.

Then if ASCII-armoring is used, it's going to increase the total file size by 4/3 (because Base64 encoding will generate 4 characters for every 3 bytes) and then again by either 65/64 or 66/64 (as lines will be wrapped after 64 characters -- with a ratio of 65/64 on systems where the java line.separator property is just \n, and 66/64 where it is \r\n). Plus ASCII-armoring will add another 80 characters or so for the armor header/tail lines (-----BEGIN PGP MESSAGE-----) and version header (Version: BCPG v1.59) and checksum.

So those things (together with the input file size) should make for a decent rough guess for the encrypted output file size.

amorillas commented 6 years ago

OK, no problem with the improvements you proposed although I'm not an expert in encryption algorithms, but they seem very reasonable. I've pushed them for you to review.

justinludwig commented 6 years ago

Thanks -- looks good! I will merge and release it this weekend.

justinludwig commented 6 years ago

I merged and released as version 0.3.0. I did change the long to int conversions from using Math.toIntExact() to just an unsafe cast as (int), for java 7 compatibility (but as long as the code short-circuts to return the maxFileBufferSize whenever the actual file size is bigger, it shouldn't be an issue). I also added some unit tests to the EncryptorSpec for the Encryptor.estimateOutFileSize() method. Thanks again!