On Thu, 6 Jun 2024 22:21:57 GMT, Martin Balao <mba...@openjdk.org> wrote:
>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java
>> line 1169:
>>
>>> 1167: }
>>> 1168: if (ciphertextArr != null) {
>>> 1169: ciphertextBuf = ByteBuffer.wrap(ciphertextArr);
>>
>> Can we add a comment here to caution that position may be incorrect (since
>> the offset is not passed to this call) and thus need to always supply an
>> index for reading/writing values to the `ciphertextBuf`?
>
> It's not so much about position being incorrect —`convertCTSVariant` makes no
> assumptions about it, but could have reset position to the beginning of the
> last 2 blocks— but that `ciphertextBuf` bytes should not be modified except
> for the last 2 blocks. I'm okay with adding a comment but I don't see any
> extension of the `convertCTSVariant` function that could be at risk of having
> to access anything other than the last 2 blocks.
True, the proposed change does not use the position of the `ciphertextBuf`.
I'd just like some comment covering this since I find it strange that you call
`ByteBuffer.wrap(cipherTextArr)` without the offset when the javadoc states
that the position is set to 0. Maybe others may also wonder why. That's all.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18898#discussion_r1630528647
