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