On Mon, 24 May 2021 16:37:05 GMT, Anthony Scarpino <ascarp...@openjdk.org>
wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 189:
>>
>>> 187: ct.position(ct.position());
>>> 188: return processed;
>>> 189: } else if (!ct.isReadOnly()) {
>>
>> Maybe you meant "ct.hasArray()" instead of "!ct.isReadOnly()"?
>
> hasArray() checks both isReadonly() and isDirect(). Since I already did
> isDirect() in the previous if, I just need to check readonly here
I see.
>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
>> line 259:
>>
>>> 257: @Override
>>> 258: protected void engineInit(int opmode, Key key,
>>> 259: AlgorithmParameterSpec params, SecureRandom random)
>>
>> The specified "random" is not saved to internal "random". Perhaps an
>> oversight?
>
> Yeah, I changed this code long ago, i suspect I didn't realize
> engineGetParameters() used it
It seems that you still have not saved the specified 'random' into the instance
'random' field? Did I miss something?
>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
>> line 279:
>>
>>> 277: if (iv.length == 0) {
>>> 278: throw new InvalidAlgorithmParameterException("IV is
>>> empty");
>>> 279: }
>>
>> Why separating the validation of iv and tag length in separate methods, i.e.
>> engineInit() vs init()? Consider consolidating them?
>
> I think I had a reason for this where I didn't know what the exact error was.
> I'd rather keep them separate so it throws the right message for the error.
> It's not a performance critical area.
Ok, it's more for code robustness, not for performance.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4072
