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

[Anthony Scarpino]

On Fri, 13 Nov 2020 22:25:13 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Code review comment update
>>   Major change to test to detect corruption with incremental buffers test
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 777:
> 
>> 775:         if ((src.remaining() + ((ibuffer != null) ? ibuffer.size() : 0) -
>> 776:             tagLenBytes) > dst.remaining()) {
>> 777:             throw new RuntimeException("output buffer too small");
> 
> Shouldn't this be ShortBufferException instead of RuntimeException?

I thought so too, but that isn't what GCTR returns.  All the GCTR checks are RuntimeExceptions.  This check was original inside of GCTR, but I had to bring it out because of the ibuffer lengths.  I don't mind changing, it's just a strange inconsistency.

> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 822:
> 
>> 820:         if (len > dst.remaining()) {
>> 821:             throw new ShortBufferException("Output buffer too small");
>> 822:         }
> 
> How is this different from the check at line 775-778 at the beginning of this method?

Ah.. good.. I'll remove the one above from GCTR as I liked this check better and it protects the GCTR update ()

> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 818:
> 
>> 816:         // do this check here can also catch the potential integer overflow
>> 817:         // scenario for the subsequent output buffer capacity check.
>> 818:         checkDataLength(0, len);
> 
> Perhaps this can be moved up to the beginning of this method just like the dst size check?

You don't know what 'len', which includes ibuffer data, until this point in the code.

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

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