ggershinsky commented on code in PR #3231:
URL: https://github.com/apache/iceberg/pull/3231#discussion_r1278917331


##########
core/src/main/java/org/apache/iceberg/encryption/Ciphers.java:
##########
@@ -67,7 +83,7 @@ public byte[] encrypt(byte[] plainText, byte[] aad) {
         if (null != aad) {
           cipher.updateAAD(aad);
         }
-        cipher.doFinal(plainText, 0, plainText.length, cipherText, 
NONCE_LENGTH);
+        cipher.doFinal(plaintext, plaintextOffset, plaintextLength, 
cipherText, NONCE_LENGTH);

Review Comment:
   > Does the call to getInstance in the constructor guarantee that the Cipher 
instance is not shared? 
   
   The `AesGcmEncryptor` and `AesGcmDecryptor` are designed for a single thread 
use.
   
   > Should this check that the number of bytes returned by doFinal is correct?
   
   Sure, we can check this.
   
   > Can you confirm that doFinal also writes the GCM tag to the output buffer?
   
   From the `Cipher.doFinal` javadoc: 
    _If an AEAD mode such as GCM/CCM is being used, the authentication tag is 
appended in the case of encryption, or verified in the case of decryption._



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to