On Thu, 22 Jul 2021 18:30:50 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java >> line 762: >> >>> 760: >>> 761: dst.put(out, 0, rlen); >>> 762: processed += srcLen; >> >> It seems that callers of this implGCMCrypt() method such as >> GCMEngine.doLastBlock() adds the returned value to the "processed" field >> which looks like double counting? However, some caller such as >> GCMEncrypt.doUpdate() does not. Seems inconsistent and may lead to wrong >> value for the "processed" field? > > All the callers that use GCMOperations, ie op.update(...), have the processed > value updated. implGCMCrypt() calls op.update() and updates the value. It > cannot double count 'processed' is not updated after implGCMCrypt(). I can > see your point, but the other methods do not have access to 'processed' and > would mean I copy that line 3 times elsewhere. I'd rather keep it as is As long the "processed" value is correct, it's fine. Not sure if I may be missing some subtle things, GCMEngine.implGCMCrypt(GCMOperation op, ByteBuffer src, ByteBuffer dst) impl would increment "processed" with "srcLen". But then in GCMEngine.doLastBlock(GCMOperation op, ByteBuffer buffer, ByteBuffer src, ByteBuffer dst) impl, it calls the GCMEngine.implGCMCrypt(GCMOperation op, ByteBuffer src, ByteBuffer dst) method and stores the return value into "resultLen" and then again increment "processed" with "resultLen" after op.doFinal(...) call. Since "resultLen" contains the number of bytes processed by GCMEngine.implGCMCrypt(GCMOperation op, ByteBuffer src, ByteBuffer dst) method already, adding it to "prcessed" looks like double counting. Not sure what did I miss. ------------- PR: https://git.openjdk.java.net/jdk/pull/4019
