RFR: 8253821: Improve ByteBuffer performance with GCM [v3]

[Anthony Scarpino]

On Mon, 2 Nov 2020 19:55:26 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>>>     * I do not understand where the corruption comes from.  The user provides a buffer that output is suppose to be placed into, what could it be corrupting?  
>> Existing tests may not catch/check all error cases. Especially, in the past, all calls w/ ByteBuffer inputs are converted to calls w/ byte[] inputs. Thus, testing is focused on byte[] scenarios. In addition, since for decryption, internal buffering is done, there may be no test checking if padding bytes are written to the output buffer. Please see my comment follows. The corruption that I refer to is about the padding bytes which are now written into the output buffer with this change.
>
>>     * It was my expectation that checkOutputCapacity() is making sure all the inOfs ==<> outOfs are going to work. Does that not catch all cases?
> checkOutputCapacity() as the name has, only changes that the output buffer is large enough.
> 
>>     * outWithPadding" is excessive because it doubles the allocation for every operation followed by a copy to the original buffer, even if the original buffer was adequate.  I'm ok with doing the extra alloc & copy in certain situations, but not everytime.  Can you be more specific what things may fail that we don't already check for in the regression tests?
> 
> For example,
> 1) various input/output offset combinations, e.g. inOfs <,=,> outOfs, 
> 2) Given a output buffer of 200-byte and outOfs == 0, assuming the returned decrypted data length is 160 bytes, the bytes from index 160 and onward should not be overwritten. GCM has no padding, so you may not notice any difference if this is what you are testing. This is generic CipherCore code and changes impact all ciphers.

checkOutputCapacity:  Yes.. The method includes the offsets for the output buffer, which I believe would verify that the output area in the buffer with offsets is large enough.

outWithPadding:  I understand the situation and I am assuming there are tests that cover this case.  Given it's a generic situation.

-------------

PR: https://git.openjdk.java.net/jdk/pull/411