From 03315fc7e47a92894e987d7afc4ed8fca85ce54c Mon Sep 17 00:00:00 2001 From: mwish Date: Thu, 11 Jul 2024 21:31:57 +0800 Subject: [PATCH] Refactor parquet::encryption::AesEncryptor --- cpp/src/parquet/encryption/encryption_internal.cc | 15 ++++++--------- cpp/src/parquet/encryption/encryption_internal.h | 9 ++++----- .../encryption/encryption_internal_nossl.cc | 9 ++++----- .../parquet/encryption/internal_file_encryptor.cc | 10 +++++----- .../parquet/encryption/internal_file_encryptor.h | 2 +- cpp/src/parquet/metadata.cc | 7 +++---- 6 files changed, 23 insertions(+), 29 deletions(-) diff --git a/cpp/src/parquet/encryption/encryption_internal.cc b/cpp/src/parquet/encryption/encryption_internal.cc index c5d2d1728ba1e..d8568e1dc8009 100644 --- a/cpp/src/parquet/encryption/encryption_internal.cc +++ b/cpp/src/parquet/encryption/encryption_internal.cc @@ -420,23 +420,20 @@ AesDecryptor::AesDecryptorImpl::AesDecryptorImpl(ParquetCipher::type alg_id, int } } -AesEncryptor* AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, bool metadata, - std::vector* all_encryptors) { - return Make(alg_id, key_len, metadata, true /*write_length*/, all_encryptors); +std::unique_ptr AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, + bool metadata) { + return Make(alg_id, key_len, metadata, true /*write_length*/); } -AesEncryptor* AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, bool metadata, - bool write_length, - std::vector* all_encryptors) { +std::unique_ptr AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, + bool metadata, bool write_length) { if (ParquetCipher::AES_GCM_V1 != alg_id && ParquetCipher::AES_GCM_CTR_V1 != alg_id) { std::stringstream ss; ss << "Crypto algorithm " << alg_id << " is not supported"; throw ParquetException(ss.str()); } - AesEncryptor* encryptor = new AesEncryptor(alg_id, key_len, metadata, write_length); - if (all_encryptors != nullptr) all_encryptors->push_back(encryptor); - return encryptor; + return std::make_unique(alg_id, key_len, metadata, write_length); } AesDecryptor::AesDecryptor(ParquetCipher::type alg_id, int key_len, bool metadata, diff --git a/cpp/src/parquet/encryption/encryption_internal.h b/cpp/src/parquet/encryption/encryption_internal.h index 2d5450553c16d..4aae63ffa5f1a 100644 --- a/cpp/src/parquet/encryption/encryption_internal.h +++ b/cpp/src/parquet/encryption/encryption_internal.h @@ -52,12 +52,11 @@ class PARQUET_EXPORT AesEncryptor { explicit AesEncryptor(ParquetCipher::type alg_id, int key_len, bool metadata, bool write_length = true); - static AesEncryptor* Make(ParquetCipher::type alg_id, int key_len, bool metadata, - std::vector* all_encryptors); + static std::unique_ptr Make(ParquetCipher::type alg_id, int key_len, + bool metadata); - static AesEncryptor* Make(ParquetCipher::type alg_id, int key_len, bool metadata, - bool write_length, - std::vector* all_encryptors); + static std::unique_ptr Make(ParquetCipher::type alg_id, int key_len, + bool metadata, bool write_length); ~AesEncryptor(); diff --git a/cpp/src/parquet/encryption/encryption_internal_nossl.cc b/cpp/src/parquet/encryption/encryption_internal_nossl.cc index ed323c4aa6167..3fd8010ba9fc5 100644 --- a/cpp/src/parquet/encryption/encryption_internal_nossl.cc +++ b/cpp/src/parquet/encryption/encryption_internal_nossl.cc @@ -70,14 +70,13 @@ void AesDecryptor::WipeOut() { ThrowOpenSSLRequiredException(); } AesDecryptor::~AesDecryptor() {} -AesEncryptor* AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, bool metadata, - std::vector* all_encryptors) { +std::unique_ptr AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, + bool metadata) { return NULLPTR; } -AesEncryptor* AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, bool metadata, - bool write_length, - std::vector* all_encryptors) { +std::unique_ptr AesEncryptor::Make(ParquetCipher::type alg_id, int key_len, + bool metadata, bool write_length) { return NULLPTR; } diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.cc b/cpp/src/parquet/encryption/internal_file_encryptor.cc index 15bf52b84dd1b..1a10a0004efa4 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.cc +++ b/cpp/src/parquet/encryption/internal_file_encryptor.cc @@ -135,7 +135,7 @@ InternalFileEncryptor::InternalFileEncryptor::GetColumnEncryptor( return encryptor; } -int InternalFileEncryptor::MapKeyLenToEncryptorArrayIndex(int key_len) { +int InternalFileEncryptor::MapKeyLenToEncryptorArrayIndex(int key_len) const { if (key_len == 16) return 0; else if (key_len == 24) @@ -150,8 +150,8 @@ encryption::AesEncryptor* InternalFileEncryptor::GetMetaAesEncryptor( int key_len = static_cast(key_size); int index = MapKeyLenToEncryptorArrayIndex(key_len); if (meta_encryptor_[index] == nullptr) { - meta_encryptor_[index].reset( - encryption::AesEncryptor::Make(algorithm, key_len, true, &all_encryptors_)); + meta_encryptor_[index] = encryption::AesEncryptor::Make(algorithm, key_len, true); + all_encryptors_.push_back(meta_encryptor_[index].get()); } return meta_encryptor_[index].get(); } @@ -161,8 +161,8 @@ encryption::AesEncryptor* InternalFileEncryptor::GetDataAesEncryptor( int key_len = static_cast(key_size); int index = MapKeyLenToEncryptorArrayIndex(key_len); if (data_encryptor_[index] == nullptr) { - data_encryptor_[index].reset( - encryption::AesEncryptor::Make(algorithm, key_len, false, &all_encryptors_)); + data_encryptor_[index] = encryption::AesEncryptor::Make(algorithm, key_len, false); + all_encryptors_.push_back(data_encryptor_[index].get()); } return data_encryptor_[index].get(); } diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.h b/cpp/src/parquet/encryption/internal_file_encryptor.h index 3cbe53500c2c5..1af474cc2c66c 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.h +++ b/cpp/src/parquet/encryption/internal_file_encryptor.h @@ -103,7 +103,7 @@ class InternalFileEncryptor { encryption::AesEncryptor* GetDataAesEncryptor(ParquetCipher::type algorithm, size_t key_len); - int MapKeyLenToEncryptorArrayIndex(int key_len); + int MapKeyLenToEncryptorArrayIndex(int key_len) const; }; } // namespace parquet diff --git a/cpp/src/parquet/metadata.cc b/cpp/src/parquet/metadata.cc index d7be50a6116bd..6f275950ddb41 100644 --- a/cpp/src/parquet/metadata.cc +++ b/cpp/src/parquet/metadata.cc @@ -649,9 +649,9 @@ class FileMetaData::FileMetaDataImpl { std::string key = file_decryptor_->GetFooterKey(); std::string aad = encryption::CreateFooterAad(file_decryptor_->file_aad()); - auto aes_encryptor = encryption::AesEncryptor::Make( - file_decryptor_->algorithm(), static_cast(key.size()), true, - false /*write_length*/, nullptr); + auto aes_encryptor = encryption::AesEncryptor::Make(file_decryptor_->algorithm(), + static_cast(key.size()), + true, false /*write_length*/); std::shared_ptr encrypted_buffer = std::static_pointer_cast( AllocateBuffer(file_decryptor_->pool(), @@ -662,7 +662,6 @@ class FileMetaData::FileMetaDataImpl { encrypted_buffer->mutable_data()); // Delete AES encryptor object. It was created only to verify the footer signature. aes_encryptor->WipeOut(); - delete aes_encryptor; return 0 == memcmp(encrypted_buffer->data() + encrypted_len - encryption::kGcmTagLength, tag, encryption::kGcmTagLength);