kou commented on code in PR #45695:
URL: https://github.com/apache/arrow/pull/45695#discussion_r1988161604


##########
cpp/src/gandiva/encrypt_utils.cc:
##########
@@ -16,22 +16,24 @@
 // under the License.
 
 #include "gandiva/encrypt_utils.h"
+#include <string.h>

Review Comment:
   Do we really need this?



##########
cpp/src/gandiva/encrypt_utils.cc:
##########
@@ -87,4 +90,17 @@ int32_t aes_decrypt(const char* ciphertext, int32_t 
ciphertext_len, const char*
   EVP_CIPHER_CTX_free(de_ctx);
   return plaintext_len;
 }
+
+const EVP_CIPHER* get_cipher_algo(int32_t key_length) {
+  switch (key_length) {
+    case 16:
+      return EVP_aes_128_ecb();
+    case 24:
+      return EVP_aes_192_ecb();
+    case 32:
+      return EVP_aes_256_ecb();
+    default:
+      throw std::runtime_error("unsupported key length");

Review Comment:
   How about showing the given key length too for easy to debug?



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -318,21 +316,32 @@ const char* gdv_fn_aes_encrypt(int64_t context, const 
char* data, int32_t data_l
     return "";
   }
 
+  int64_t kAesBlockSize = 0;
+  if (key_data_len == 16 || key_data_len == 24 || key_data_len == 32) {
+    kAesBlockSize = static_cast<int64_t>(key_data_len);
+  } else {
+    gdv_fn_context_set_error_msg(context, "invalid key length");

Review Comment:
   How about showing the given key length for easy to debug?



##########
cpp/src/gandiva/encrypt_utils_test.cc:
##########
@@ -20,36 +20,40 @@
 #include <gtest/gtest.h>
 
 TEST(TestShaEncryptUtils, TestAesEncryptDecrypt) {
-  // 8 bytes key
-  auto* key = "1234abcd";

Review Comment:
   We drop support for 8 bytes key, right?
   (Or 8 bytes key didn't work?)



##########
cpp/src/gandiva/encrypt_utils.h:
##########
@@ -28,13 +28,15 @@ namespace gandiva {
  **/
 GANDIVA_EXPORT
 int32_t aes_encrypt(const char* plaintext, int32_t plaintext_len, const char* 
key,
-                    unsigned char* cipher);
+                    int32_t key_len, unsigned char* cipher);
 
 /**
  * Decrypt data using aes algorithm
  **/
 GANDIVA_EXPORT
 int32_t aes_decrypt(const char* ciphertext, int32_t ciphertext_len, const 
char* key,
-                    unsigned char* plaintext);
+                    int32_t key_len, unsigned char* plaintext);
+
+const EVP_CIPHER* get_cipher_algo(int32_t key_length);

Review Comment:
   Can we define this in `encrypt_utils.cc` with anonymous namespace?



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -349,24 +358,35 @@ const char* gdv_fn_aes_decrypt(int64_t context, const 
char* data, int32_t data_l
     return "";
   }
 
+  int64_t kAesBlockSize = 0;
+  if (key_data_len == 16 || key_data_len == 24 || key_data_len == 32) {
+    kAesBlockSize = static_cast<int64_t>(key_data_len);
+  } else {
+    gdv_fn_context_set_error_msg(context, "invalid key length");

Review Comment:
   ditto.



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to