On Thu, 5 Sep 2024 17:39:56 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java
>> line 358:
>>
>>> 356: }
>>> 357:
>>> 358: return Arrays.copyOf(kdfOutput, outLen);
>>
>> Here is an alternative solution which does not need `Arrays.copyOf()` call:
>> Suggestion:
>>
>> kdfOutput = new byte[outLen];
>> int i = 0;
>> int offset = 0;
>> try {
>> while (i < rounds) {
>> if (i > 0) {
>> hmacObj.update(kdfOutput, offset - hmacLen, hmacLen); //
>> add T(i-1)
>> }
>> hmacObj.update(info); // Add info
>> hmacObj.update((byte) ++i); // Add round number
>> if (i == rounds && (outLen - offset < hmacLen)) {
>> // special handling for last chunk
>> byte[] tmp = hmacObj.doFinal();
>> System.arraycopy(tmp, 0, kdfOutput, offset,
>> outLen - offset);
>> offset = outLen;
>> } else {
>> hmacObj.doFinal(kdfOutput, offset);
>> offset += hmacLen;
>> }
>> }
>> } catch (ShortBufferException sbe) {
>> // This really shouldn't happen given that we've
>> // sized the buffers to their largest possible size up-front,
>> // but just in case...
>> throw new RuntimeException(sbe);
>> }
>> return kdfOutput;
>
> My personal opinion is that this adds complexity which is unnecessary. The
> `Arrays.copyOf` solution reads more simply and GC will take care of any extra
> allocation once it goes out of scope. I appreciate the alternate fix, but in
> this case the original solution is more easily understood and thus more
> maintainable.
Made a couple small changes and accepted your proposed code. Thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1746247893
