From 52a2b8074c884113a08630861d3ac8646a7dd09f Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Thu, 12 Feb 2026 11:12:42 -0600 Subject: [PATCH 1/7] First attempt at refactoring classes --- cpp/src/parquet/column_reader.cc | 8 +- cpp/src/parquet/column_writer.cc | 14 ++-- cpp/src/parquet/encryption/aes_encryption.cc | 6 +- cpp/src/parquet/encryption/aes_encryption.h | 12 ++- .../encryption/aes_encryption_nossl.cc | 6 +- .../parquet/encryption/decryptor_interface.h | 22 +++--- .../parquet/encryption/encryptor_interface.h | 22 +++--- .../encryption/external_dbpa_encryption.cc | 76 +++++++++---------- .../encryption/external_dbpa_encryption.h | 28 ++++--- .../external_dbpa_encryption_test.cc | 18 ++--- .../encryption/internal_file_decryptor.cc | 19 +++-- .../encryption/internal_file_decryptor.h | 11 +-- .../encryption/internal_file_encryptor.cc | 17 ++--- .../encryption/internal_file_encryptor.h | 11 +-- cpp/src/parquet/file_deserialize_test.cc | 20 +++-- 15 files changed, 142 insertions(+), 148 deletions(-) diff --git a/cpp/src/parquet/column_reader.cc b/cpp/src/parquet/column_reader.cc index 78ddb1ed0860..89236ee9a7db 100644 --- a/cpp/src/parquet/column_reader.cc +++ b/cpp/src/parquet/column_reader.cc @@ -574,19 +574,19 @@ std::shared_ptr SerializedPageReader::NextPage() { std::unique_ptr encoding_properties = GetEncodingProperties(current_page_header_); - data_decryptor_->UpdateEncodingProperties(std::move(encoding_properties)); - std::shared_ptr decryption_buffer; if (data_decryptor_->CanCalculateLengths()) { decryption_buffer = AllocateBuffer( properties_.memory_pool(), data_decryptor_->PlaintextLength(compressed_len)); compressed_len = data_decryptor_->Decrypt(page_buffer->span_as(), - decryption_buffer->mutable_span_as()); + decryption_buffer->mutable_span_as(), + std::move(encoding_properties)); } else { decryption_buffer = AllocateBuffer(properties_.memory_pool(), 0); compressed_len = data_decryptor_->DecryptWithManagedBuffer( - page_buffer->span_as(), decryption_buffer.get()); + page_buffer->span_as(), decryption_buffer.get(), + std::move(encoding_properties)); } page_buffer = decryption_buffer; diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc index cbeb2534dc67..010f4ee7a06b 100644 --- a/cpp/src/parquet/column_writer.cc +++ b/cpp/src/parquet/column_writer.cc @@ -326,17 +326,18 @@ class SerializedPageWriter : public PageWriter { EncodingProperties::MakeFromMetadata(metadata_->descr(), metadata_->properties(), static_cast(page)); - data_encryptor_->UpdateEncodingProperties(std::move(encoding_properties)); if (data_encryptor_->CanCalculateCiphertextLength()) { PARQUET_THROW_NOT_OK(encryption_buffer_->Resize( data_encryptor_->CiphertextLength(output_data_len), false)); output_data_len = data_encryptor_->Encrypt(compressed_data->span_as(), - encryption_buffer_->mutable_span_as()); + encryption_buffer_->mutable_span_as(), + std::move(encoding_properties)); } else { output_data_len = data_encryptor_->EncryptWithManagedBuffer( - compressed_data->span_as(), encryption_buffer_.get()); + compressed_data->span_as(), encryption_buffer_.get(), + std::move(encoding_properties)); } output_data_buffer = encryption_buffer_->data(); @@ -451,17 +452,18 @@ class SerializedPageWriter : public PageWriter { EncodingProperties::MakeFromMetadata(metadata_->descr(), metadata_->properties(), static_cast(page)); - data_encryptor_->UpdateEncodingProperties(std::move(encoding_properties)); if (data_encryptor_->CanCalculateCiphertextLength()) { PARQUET_THROW_NOT_OK(encryption_buffer_->Resize( data_encryptor_->CiphertextLength(output_data_len), false)); output_data_len = data_encryptor_->Encrypt(compressed_data->span_as(), - encryption_buffer_->mutable_span_as()); + encryption_buffer_->mutable_span_as(), + std::move(encoding_properties)); } else { output_data_len = data_encryptor_->EncryptWithManagedBuffer( - compressed_data->span_as(), encryption_buffer_.get()); + compressed_data->span_as(), encryption_buffer_.get(), + std::move(encoding_properties)); } output_data_buffer = encryption_buffer_->data(); diff --git a/cpp/src/parquet/encryption/aes_encryption.cc b/cpp/src/parquet/encryption/aes_encryption.cc index 72ca1fb45bf9..d85e91d3a764 100644 --- a/cpp/src/parquet/encryption/aes_encryption.cc +++ b/cpp/src/parquet/encryption/aes_encryption.cc @@ -159,7 +159,8 @@ int32_t AesEncryptor::SignedFooterEncrypt(::arrow::util::span foo int32_t AesEncryptor::Encrypt(::arrow::util::span plaintext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span ciphertext) { + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties) { if (static_cast(key_length_) != key.size()) { std::stringstream ss; ss << "Wrong key length " << key.size() << ". Should be " << key_length_; @@ -413,7 +414,8 @@ int32_t AesDecryptor::CiphertextLength(int32_t plaintext_len) const { int32_t AesDecryptor::Decrypt(::arrow::util::span ciphertext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span plaintext) { + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties) { if (static_cast(key_length_) != key.size()) { std::stringstream ss; ss << "Wrong key length " << key.size() << ". Should be " << key_length_; diff --git a/cpp/src/parquet/encryption/aes_encryption.h b/cpp/src/parquet/encryption/aes_encryption.h index 86486099646d..92390a5489fd 100644 --- a/cpp/src/parquet/encryption/aes_encryption.h +++ b/cpp/src/parquet/encryption/aes_encryption.h @@ -83,12 +83,14 @@ class PARQUET_EXPORT AesEncryptor : public AesCryptoContext, public EncryptorInt int32_t Encrypt(::arrow::util::span plaintext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span ciphertext) override; + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr) override; /// Encrypt the plaintext and leave the results in the ciphertext buffer. This method is /// not supported as we can calculate the ciphertext length before encryption. int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext) override { + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "EncryptWithManagedBuffer is not supported in AesEncryptor, use Encrypt instead"); } @@ -171,13 +173,15 @@ class PARQUET_EXPORT AesDecryptor : public AesCryptoContext, public DecryptorInt int32_t Decrypt(::arrow::util::span ciphertext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span plaintext) override; + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr) override; /// Decrypt the ciphertext and leave the results in the plaintext buffer. This /// method is not supported as we can calculate the plaintext length before /// decryption. int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext) override { + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "DecryptWithManagedBuffer is not supported in AesDecryptor, use Decrypt instead"); } diff --git a/cpp/src/parquet/encryption/aes_encryption_nossl.cc b/cpp/src/parquet/encryption/aes_encryption_nossl.cc index 304cbb1fcba0..2a05bf93e70e 100644 --- a/cpp/src/parquet/encryption/aes_encryption_nossl.cc +++ b/cpp/src/parquet/encryption/aes_encryption_nossl.cc @@ -51,7 +51,8 @@ int32_t AesEncryptor::CiphertextLength(int64_t plaintext_len) const { int32_t AesEncryptor::Encrypt(::arrow::util::span plaintext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span ciphertext) { + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties) { ThrowOpenSSLRequiredException(); return -1; } @@ -65,7 +66,8 @@ AesEncryptor::AesEncryptor(ParquetCipher::type alg_id, int32_t key_len, bool met int32_t AesDecryptor::Decrypt(::arrow::util::span ciphertext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span plaintext) { + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties) { ThrowOpenSSLRequiredException(); return -1; } diff --git a/cpp/src/parquet/encryption/decryptor_interface.h b/cpp/src/parquet/encryption/decryptor_interface.h index 19d10e2be1e1..8ff5f469913a 100644 --- a/cpp/src/parquet/encryption/decryptor_interface.h +++ b/cpp/src/parquet/encryption/decryptor_interface.h @@ -41,22 +41,20 @@ class PARQUET_EXPORT DecryptorInterface { /// Decrypt the ciphertext and leave the results in the plaintext buffer. /// Most implementations will require the key and aad to be provided, but it is up to /// each decryptor whether to use them or not. - virtual int32_t Decrypt(::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span plaintext) = 0; + virtual int32_t Decrypt( + ::arrow::util::span ciphertext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr) = 0; /// Decrypt the ciphertext and leave the results in the plaintext buffer. /// The buffer will be resized to the correct size during decryption. This method /// is used when the decryptor cannot calculate the plaintext length before decryption. - virtual int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext) = 0; - - // Some Encryptors may need to understand the page encoding before the encryption - /// process. This method will be called from ColumnWriter before invoking the - /// Encrypt method. - virtual void UpdateEncodingProperties( - std::unique_ptr encoding_properties) {} + virtual int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr) = 0; }; } // namespace parquet::encryption diff --git a/cpp/src/parquet/encryption/encryptor_interface.h b/cpp/src/parquet/encryption/encryptor_interface.h index 948b923e55be..94b02fff4b05 100644 --- a/cpp/src/parquet/encryption/encryptor_interface.h +++ b/cpp/src/parquet/encryption/encryptor_interface.h @@ -38,23 +38,21 @@ class PARQUET_EXPORT EncryptorInterface { /// Encrypt the plaintext and leave the results in the ciphertext buffer. /// Most implementations will require the key and aad to be provided, but it is /// up to each encryptor whether to use them or not. - virtual int32_t Encrypt(::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span ciphertext) = 0; + virtual int32_t Encrypt( + ::arrow::util::span plaintext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr) = 0; /// Encrypt the plaintext and leave the results in the ciphertext buffer. /// The buffer will be resized to the appropriate size by the encryptor during /// encryption. This method is used when the encryptor cannot calculate the /// ciphertext length before encryption. - virtual int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext) = 0; - - /// Some Encryptors may need to understand the page encoding before the encryption - /// process. This method will be called from ColumnWriter before invoking the - /// Encrypt method. - virtual void UpdateEncodingProperties( - std::unique_ptr encoding_properties) {} + virtual int32_t EncryptWithManagedBuffer( + ::arrow::util::span plaintext, + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr) = 0; /// After the column_writer writes a dictionary or a data page, this method will be /// called so that each encryptor can provide any encryptor-specific column diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index 3494019913a9..b31290b1a853 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -183,6 +183,22 @@ void UpdateEncryptorMetadata( } } // UpdateEncryptorMetadata() +// Some Encryptors and Decryptors may need to understand the page encoding before the +// encryption process. This method will be called from the Encrypt and Decrypt +// WithManagedBuffer methods. +std::unique_ptr UpdateEncodingProperties( + std::string column_name, Type::type data_type, + std::optional datatype_length, Compression::type compression_type, + std::unique_ptr encoding_properties) { + + encoding_properties->set_column_path(column_name); + encoding_properties->set_physical_type(data_type, datatype_length); + encoding_properties->set_compression_codec(compression_type); + + encoding_properties->validate(); + return encoding_properties; +} // UpdateEncodingProperties() + std::optional> ExternalDBPAUtils::KeyValueMetadataToStringMap( const std::shared_ptr& key_value_metadata) { @@ -289,20 +305,6 @@ int32_t ExternalDBPAEncryptorAdapter::CiphertextLength(int64_t plaintext_len) co "ExternalDBPAEncryptorAdapter::CiphertextLength is not supported"); } -void ExternalDBPAEncryptorAdapter::UpdateEncodingProperties( - std::unique_ptr encoding_properties) { - ARROW_LOG(DEBUG) << "ExternalDBPAEncryptorAdapter::UpdateEncodingProperties"; - - // Fill-in values from the decryptor constructor. - encoding_properties->set_column_path(column_name_); - encoding_properties->set_physical_type(data_type_, datatype_length_); - encoding_properties->set_compression_codec(compression_type_); - - encoding_properties->validate(); - encoding_properties_ = std::move(encoding_properties); - encoding_properties_updated_ = true; -} - std::shared_ptr ExternalDBPAEncryptorAdapter::GetKeyValueMetadata( int8_t module_type) { auto it = column_encryption_metadata_.find(module_type); @@ -317,14 +319,16 @@ std::shared_ptr ExternalDBPAEncryptorAdapter::GetKeyValueMetad } // GetKeyValueMetadata() int32_t ExternalDBPAEncryptorAdapter::EncryptWithManagedBuffer( - ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext) { - if (!encoding_properties_updated_) { - ARROW_LOG(ERROR) << "ExternalDBPAEncryptorAdapter:: EncryptionParams not updated"; - throw ParquetException("ExternalDBPAEncryptorAdapter:: EncryptionParams not updated"); + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties) { + if (encoding_properties == nullptr) { + ARROW_LOG(ERROR) << "ExternalDBPAEncryptorAdapter:: encoding_properties is nullptr"; + throw ParquetException( + "ExternalDBPAEncryptorAdapter:: encoding_properties not provided, params not updated"); } - - encoding_properties_updated_ = false; - + encoding_properties_ = UpdateEncodingProperties( + column_name_, data_type_, datatype_length_, compression_type_, + std::move(encoding_properties)); return InvokeExternalEncrypt(plaintext, ciphertext, encoding_properties_->ToPropertiesMap()); } @@ -551,29 +555,17 @@ int32_t ExternalDBPADecryptorAdapter::CiphertextLength(int32_t plaintext_len) co "ExternalDBPADecryptorAdapter::CiphertextLength is not supported"); } -void ExternalDBPADecryptorAdapter::UpdateEncodingProperties( - std::unique_ptr encoding_properties) { - ARROW_LOG(DEBUG) << "ExternalDBPADecryptorAdapter::UpdateEncodingProperties"; - - // Fill-in values from the decryptor constructor. - encoding_properties->set_column_path(column_name_); - encoding_properties->set_physical_type(data_type_, datatype_length_); - encoding_properties->set_compression_codec(compression_type_); - - encoding_properties->validate(); - encoding_properties_ = std::move(encoding_properties); - encoding_properties_updated_ = true; -} - int32_t ExternalDBPADecryptorAdapter::DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext) { - if (!encoding_properties_updated_) { - ARROW_LOG(ERROR) << "ExternalDBPADecryptorAdapter:: DecryptionParams not updated"; - throw ParquetException("ExternalDBPADecryptorAdapter:: DecryptionParams not updated"); + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties) { + if (encoding_properties == nullptr) { + ARROW_LOG(ERROR) << "ExternalDBPADecryptorAdapter:: encoding_properties is nullptr"; + throw ParquetException( + "ExternalDBPADecryptorAdapter:: encoding_properties not provided, params not updated"); } - - encoding_properties_updated_ = false; - + encoding_properties_ = UpdateEncodingProperties( + column_name_, data_type_, datatype_length_, compression_type_, + std::move(encoding_properties)); return InvokeExternalDecrypt(ciphertext, plaintext, encoding_properties_->ToPropertiesMap()); } diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index d8da194c1883..c876ec22aa77 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -57,7 +57,8 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { int32_t Encrypt(::arrow::util::span plaintext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span ciphertext) override { + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; ss << "Encrypt is not supported in ExternalDBPAEncryptorAdapter, "; ss << "use EncryptWithManagedBuffer instead"; @@ -67,7 +68,8 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { /// Encrypt the plaintext and leave the results in the ciphertext buffer. /// The buffer will be resized to the appropriate size by the agent during encryption. int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext) override; + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr) override; /// Encrypts plaintext footer, in order to compute footer signature (tag). int32_t SignedFooterEncrypt(::arrow::util::span footer, @@ -76,9 +78,6 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { ::arrow::util::span nonce, ::arrow::util::span encrypted_footer) override; - void UpdateEncodingProperties( - std::unique_ptr encoding_properties) override; - std::shared_ptr GetKeyValueMetadata(int8_t module_type) override; private: @@ -108,7 +107,6 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { std::unique_ptr agent_instance_; std::unique_ptr encoding_properties_; - bool encoding_properties_updated_ = false; // Accumulated column encryption metadata per module type (e.g., data page, // dictionary page) to be used later by GetKeyValueMetadata. @@ -133,6 +131,14 @@ void UpdateEncryptorMetadata( const EncodingProperties& encoding_properties, const dbps::external::EncryptionResult& result); +// Update the encoding properties based on the column name, data type, compression type, +// and datatype length. +PARQUET_EXPORT +std::unique_ptr UpdateEncodingProperties( + std::string column_name, Type::type data_type, + std::optional datatype_length, Compression::type compression_type, + std::unique_ptr encoding_properties); + /// Factory for ExternalDBPAEncryptorAdapter instances. The cache exists while the write /// operation is open, and is used to guarantee the lifetime of the encryptor. class PARQUET_EXPORT ExternalDBPAEncryptorAdapterFactory { @@ -188,7 +194,8 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { int32_t Decrypt(::arrow::util::span ciphertext, ::arrow::util::span key, ::arrow::util::span aad, - ::arrow::util::span plaintext) override { + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; ss << "Decrypt is not supported in ExternalDBPADecryptorAdapter, "; ss << "use DecryptWithManagedBuffer instead"; @@ -200,10 +207,8 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { /// is used when the decryptor cannot calculate the plaintext length before /// decryption. int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext) override; - - void UpdateEncodingProperties( - std::unique_ptr encoding_properties) override; + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr) override; private: // agent_instance is assumed to be initialized at the time of construction. @@ -232,7 +237,6 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { std::unique_ptr agent_instance_; std::unique_ptr encoding_properties_; - bool encoding_properties_updated_ = false; // Store the key value metadata from the column chunk metadata. std::shared_ptr key_value_metadata_; diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc index a32bc1b738a9..c70decdede5e 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc @@ -90,15 +90,14 @@ class ExternalDBPAEncryptorAdapterTest : public ::testing::Test { .DataPageNumValues(100) .Build(); - encryptor->UpdateEncodingProperties(builder.Build()); - ASSERT_LE(plaintext.size(), static_cast(std::numeric_limits::max())); int32_t expected_ciphertext_length = static_cast(plaintext.size()); std::shared_ptr ciphertext_buffer = AllocateBuffer(::arrow::default_memory_pool(), expected_ciphertext_length); int32_t encryption_length = - encryptor->EncryptWithManagedBuffer(str2span(plaintext), ciphertext_buffer.get()); + encryptor->EncryptWithManagedBuffer(str2span(plaintext), ciphertext_buffer.get(), + builder.Build()); ASSERT_EQ(expected_ciphertext_length, encryption_length); std::string ciphertext_str(ciphertext_buffer->data(), @@ -116,15 +115,13 @@ class ExternalDBPAEncryptorAdapterTest : public ::testing::Test { algorithm, column_name, key_id, data_type, compression_type, app_context_, connection_config_, std::nullopt, key_value_metadata_); - decryptor->UpdateEncodingProperties(builder.Build()); - ASSERT_LE(ciphertext_str.size(), static_cast(std::numeric_limits::max())); int32_t expected_plaintext_length = static_cast(ciphertext_str.size()); std::shared_ptr plaintext_buffer = AllocateBuffer(::arrow::default_memory_pool(), expected_plaintext_length); int32_t decryption_length = decryptor->DecryptWithManagedBuffer( - str2span(ciphertext_str), plaintext_buffer.get()); + str2span(ciphertext_str), plaintext_buffer.get(), builder.Build()); ASSERT_EQ(expected_plaintext_length, decryption_length); std::string plaintext_str(plaintext_buffer->data(), @@ -498,15 +495,14 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithWrongKeyIdFails) { .DataPageNumValues(100) .Build(); - encryptor->UpdateEncodingProperties(builder.Build()); - std::string plaintext = "Sensitive Data"; std::shared_ptr ciphertext_buffer = AllocateBuffer(::arrow::default_memory_pool(), 0); std::string empty; int32_t enc_len = - encryptor->EncryptWithManagedBuffer(str2span(plaintext), ciphertext_buffer.get()); + encryptor->EncryptWithManagedBuffer( + str2span(plaintext), ciphertext_buffer.get(), builder.Build()); std::string ciphertext_str(ciphertext_buffer->data(), ciphertext_buffer->data() + enc_len); @@ -515,8 +511,6 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithWrongKeyIdFails) { algorithm, column_name, wrong_key_id, data_type, compression_type, app_context, config, std::nullopt, key_value_metadata_); - decryptor->UpdateEncodingProperties(builder.Build()); - std::shared_ptr plaintext_buffer = AllocateBuffer(::arrow::default_memory_pool(), 0); @@ -524,7 +518,7 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithWrongKeyIdFails) { int32_t dec_len = 0; try { dec_len = decryptor->DecryptWithManagedBuffer(str2span(ciphertext_str), - plaintext_buffer.get()); + plaintext_buffer.get(), builder.Build()); } catch (const ParquetException&) { threw = true; } diff --git a/cpp/src/parquet/encryption/internal_file_decryptor.cc b/cpp/src/parquet/encryption/internal_file_decryptor.cc index 646e48d66385..34642ab32540 100644 --- a/cpp/src/parquet/encryption/internal_file_decryptor.cc +++ b/cpp/src/parquet/encryption/internal_file_decryptor.cc @@ -52,20 +52,19 @@ int32_t Decryptor::CiphertextLength(int32_t plaintext_len) const { return decryptor_instance_->CiphertextLength(plaintext_len); } -void Decryptor::UpdateEncodingProperties( - std::unique_ptr encoding_properties) { - decryptor_instance_->UpdateEncodingProperties(std::move(encoding_properties)); -} - int32_t Decryptor::Decrypt(::arrow::util::span ciphertext, - ::arrow::util::span plaintext) { + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties) { return decryptor_instance_->Decrypt(ciphertext, key_.as_span(), str2span(aad_), - plaintext); + plaintext, std::move(encoding_properties)); } -int32_t Decryptor::DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext) { - return decryptor_instance_->DecryptWithManagedBuffer(ciphertext, plaintext); +int32_t Decryptor::DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties) { + return decryptor_instance_->DecryptWithManagedBuffer( + ciphertext, plaintext, std::move(encoding_properties)); } // InternalFileDecryptor diff --git a/cpp/src/parquet/encryption/internal_file_decryptor.h b/cpp/src/parquet/encryption/internal_file_decryptor.h index 11994ec99a1f..91f7d0bc41fb 100644 --- a/cpp/src/parquet/encryption/internal_file_decryptor.h +++ b/cpp/src/parquet/encryption/internal_file_decryptor.h @@ -52,11 +52,12 @@ class PARQUET_EXPORT Decryptor { [[nodiscard]] int32_t PlaintextLength(int32_t ciphertext_len) const; [[nodiscard]] int32_t CiphertextLength(int32_t plaintext_len) const; int32_t Decrypt(::arrow::util::span ciphertext, - ::arrow::util::span plaintext); - int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext); - - void UpdateEncodingProperties(std::unique_ptr encoding_properties); + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr); + int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr); private: std::unique_ptr decryptor_instance_; diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.cc b/cpp/src/parquet/encryption/internal_file_encryptor.cc index d3c59cfb5950..120a0bbfab6c 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.cc +++ b/cpp/src/parquet/encryption/internal_file_encryptor.cc @@ -44,19 +44,18 @@ bool Encryptor::CanCalculateCiphertextLength() const { } int32_t Encryptor::Encrypt(::arrow::util::span plaintext, - ::arrow::util::span ciphertext) { + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties) { return encryptor_instance_->Encrypt(plaintext, key_.as_span(), str2span(aad_), - ciphertext); + ciphertext, std::move(encoding_properties)); } -int32_t Encryptor::EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext) { - return encryptor_instance_->EncryptWithManagedBuffer(plaintext, ciphertext); -} - -void Encryptor::UpdateEncodingProperties( +int32_t Encryptor::EncryptWithManagedBuffer( + ::arrow::util::span plaintext, + ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties) { - encryptor_instance_->UpdateEncodingProperties(std::move(encoding_properties)); + return encryptor_instance_->EncryptWithManagedBuffer( + plaintext, ciphertext, std::move(encoding_properties)); } std::shared_ptr Encryptor::GetKeyValueMetadata(int8_t module_type) { diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.h b/cpp/src/parquet/encryption/internal_file_encryptor.h index 4276ee45d782..adadb035f28f 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.h +++ b/cpp/src/parquet/encryption/internal_file_encryptor.h @@ -48,12 +48,13 @@ class PARQUET_EXPORT Encryptor { [[nodiscard]] int32_t CiphertextLength(int64_t plaintext_len) const; int32_t Encrypt(::arrow::util::span plaintext, - ::arrow::util::span ciphertext); + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr); - int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext); - - void UpdateEncodingProperties(std::unique_ptr encoding_properties); + int32_t EncryptWithManagedBuffer( + ::arrow::util::span plaintext, + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr); /// After the column_writer writes a dictionary or a data page, this method will /// be called so that each encryptor can provide any encryptor-specific column diff --git a/cpp/src/parquet/file_deserialize_test.cc b/cpp/src/parquet/file_deserialize_test.cc index ef54d8d40366..037658fa520c 100644 --- a/cpp/src/parquet/file_deserialize_test.cc +++ b/cpp/src/parquet/file_deserialize_test.cc @@ -1026,18 +1026,9 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { int32_t Decrypt(::arrow::util::span ciphertext, ::arrow::util::span /*key*/, ::arrow::util::span /*aad*/, - ::arrow::util::span plaintext) override { + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties) override { std::copy(ciphertext.begin(), ciphertext.end(), plaintext.begin()); - return static_cast(ciphertext.size()); - } - - int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext) override { - throw ParquetException("DecryptWithManagedBuffer not supported"); - } - - void UpdateEncodingProperties(std::unique_ptr - encoding_properties) override { // Fill column-level properties so validate() succeeds encoding_properties->set_column_path(column_path_); encoding_properties->set_physical_type(physical_type_); @@ -1045,6 +1036,13 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { encoding_properties->validate(); sink_->entries.emplace_back(encoding_properties->ToPropertiesMap()); + return static_cast(ciphertext.size()); + } + + int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties) override { + throw ParquetException("DecryptWithManagedBuffer not supported"); } private: From 70d955d40790ae75ecb7a94fb81d1ca868f0cfd2 Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Thu, 12 Feb 2026 16:27:48 -0600 Subject: [PATCH 2/7] Fix linter errors --- cpp/src/parquet/encryption/aes_encryption.h | 36 +++++++++------- .../parquet/encryption/decryptor_interface.h | 6 +-- .../parquet/encryption/encryptor_interface.h | 6 +-- .../encryption/external_dbpa_encryption.cc | 25 +++++------ .../encryption/external_dbpa_encryption.h | 42 ++++++++++--------- .../external_dbpa_encryption_test.cc | 14 +++---- .../encryption/internal_file_decryptor.cc | 7 ++-- .../encryption/internal_file_decryptor.h | 3 +- .../encryption/internal_file_encryptor.cc | 7 ++-- .../encryption/internal_file_encryptor.h | 3 +- cpp/src/parquet/file_deserialize_test.cc | 26 +++++++----- 11 files changed, 89 insertions(+), 86 deletions(-) diff --git a/cpp/src/parquet/encryption/aes_encryption.h b/cpp/src/parquet/encryption/aes_encryption.h index 92390a5489fd..e4346ce4d02b 100644 --- a/cpp/src/parquet/encryption/aes_encryption.h +++ b/cpp/src/parquet/encryption/aes_encryption.h @@ -80,17 +80,19 @@ class PARQUET_EXPORT AesEncryptor : public AesCryptoContext, public EncryptorInt /// Encrypts plaintext with the key and aad. Key length is passed only for validation. /// If different from value in constructor, exception will be thrown. - int32_t Encrypt(::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span ciphertext, - std::unique_ptr encoding_properties = nullptr) override; + int32_t Encrypt( + ::arrow::util::span plaintext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr) override; /// Encrypt the plaintext and leave the results in the ciphertext buffer. This method is /// not supported as we can calculate the ciphertext length before encryption. - int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, - std::unique_ptr encoding_properties = nullptr) override { + int32_t EncryptWithManagedBuffer( + ::arrow::util::span plaintext, + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "EncryptWithManagedBuffer is not supported in AesEncryptor, use Encrypt instead"); } @@ -170,18 +172,20 @@ class PARQUET_EXPORT AesDecryptor : public AesCryptoContext, public DecryptorInt /// validation. If different from value in constructor, exception will be thrown. /// The caller is responsible for ensuring that the plaintext buffer is at least as /// large as PlaintextLength(ciphertext_len). - int32_t Decrypt(::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span plaintext, - std::unique_ptr encoding_properties = nullptr) override; + int32_t Decrypt( + ::arrow::util::span ciphertext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr) override; /// Decrypt the ciphertext and leave the results in the plaintext buffer. This /// method is not supported as we can calculate the plaintext length before /// decryption. - int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, - std::unique_ptr encoding_properties = nullptr) override { + int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "DecryptWithManagedBuffer is not supported in AesDecryptor, use Decrypt instead"); } diff --git a/cpp/src/parquet/encryption/decryptor_interface.h b/cpp/src/parquet/encryption/decryptor_interface.h index 8ff5f469913a..6230f066fe94 100644 --- a/cpp/src/parquet/encryption/decryptor_interface.h +++ b/cpp/src/parquet/encryption/decryptor_interface.h @@ -43,8 +43,7 @@ class PARQUET_EXPORT DecryptorInterface { /// each decryptor whether to use them or not. virtual int32_t Decrypt( ::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span plaintext, std::unique_ptr encoding_properties = nullptr) = 0; @@ -52,8 +51,7 @@ class PARQUET_EXPORT DecryptorInterface { /// The buffer will be resized to the correct size during decryption. This method /// is used when the decryptor cannot calculate the plaintext length before decryption. virtual int32_t DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, std::unique_ptr encoding_properties = nullptr) = 0; }; diff --git a/cpp/src/parquet/encryption/encryptor_interface.h b/cpp/src/parquet/encryption/encryptor_interface.h index 94b02fff4b05..9ce4a201c0f8 100644 --- a/cpp/src/parquet/encryption/encryptor_interface.h +++ b/cpp/src/parquet/encryption/encryptor_interface.h @@ -40,8 +40,7 @@ class PARQUET_EXPORT EncryptorInterface { /// up to each encryptor whether to use them or not. virtual int32_t Encrypt( ::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span ciphertext, std::unique_ptr encoding_properties = nullptr) = 0; @@ -50,8 +49,7 @@ class PARQUET_EXPORT EncryptorInterface { /// encryption. This method is used when the encryptor cannot calculate the /// ciphertext length before encryption. virtual int32_t EncryptWithManagedBuffer( - ::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties = nullptr) = 0; /// After the column_writer writes a dictionary or a data page, this method will be diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index b31290b1a853..89de7fc1bca1 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -187,15 +187,16 @@ void UpdateEncryptorMetadata( // encryption process. This method will be called from the Encrypt and Decrypt // WithManagedBuffer methods. std::unique_ptr UpdateEncodingProperties( - std::string column_name, Type::type data_type, - std::optional datatype_length, Compression::type compression_type, - std::unique_ptr encoding_properties) { - - encoding_properties->set_column_path(column_name); - encoding_properties->set_physical_type(data_type, datatype_length); - encoding_properties->set_compression_codec(compression_type); + std::string column_name, Type::type data_type, std::optional datatype_length, + Compression::type compression_type, + std::unique_ptr encoding_properties) { + if (encoding_properties != nullptr) { + encoding_properties->set_column_path(column_name); + encoding_properties->set_physical_type(data_type, datatype_length); + encoding_properties->set_compression_codec(compression_type); - encoding_properties->validate(); + encoding_properties->validate(); + } return encoding_properties; } // UpdateEncodingProperties() @@ -324,11 +325,11 @@ int32_t ExternalDBPAEncryptorAdapter::EncryptWithManagedBuffer( if (encoding_properties == nullptr) { ARROW_LOG(ERROR) << "ExternalDBPAEncryptorAdapter:: encoding_properties is nullptr"; throw ParquetException( - "ExternalDBPAEncryptorAdapter:: encoding_properties not provided, params not updated"); + "ExternalDBPAEncryptorAdapter:: encoding_properties is null, params not updated"); } - encoding_properties_ = UpdateEncodingProperties( - column_name_, data_type_, datatype_length_, compression_type_, - std::move(encoding_properties)); + encoding_properties_ = + UpdateEncodingProperties(column_name_, data_type_, datatype_length_, + compression_type_, std::move(encoding_properties)); return InvokeExternalEncrypt(plaintext, ciphertext, encoding_properties_->ToPropertiesMap()); } diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index c876ec22aa77..da3ee144a6be 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -54,11 +54,12 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { [[nodiscard]] int32_t CiphertextLength(int64_t plaintext_len) const override; /// Encryption not supported as we cannot calculate the ciphertext before encryption. - int32_t Encrypt(::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span ciphertext, - std::unique_ptr encoding_properties = nullptr) override { + int32_t Encrypt( + ::arrow::util::span plaintext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span ciphertext, + std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; ss << "Encrypt is not supported in ExternalDBPAEncryptorAdapter, "; ss << "use EncryptWithManagedBuffer instead"; @@ -67,9 +68,10 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { /// Encrypt the plaintext and leave the results in the ciphertext buffer. /// The buffer will be resized to the appropriate size by the agent during encryption. - int32_t EncryptWithManagedBuffer(::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, - std::unique_ptr encoding_properties = nullptr) override; + int32_t EncryptWithManagedBuffer( + ::arrow::util::span plaintext, + ::arrow::ResizableBuffer* ciphertext, + std::unique_ptr encoding_properties = nullptr) override; /// Encrypts plaintext footer, in order to compute footer signature (tag). int32_t SignedFooterEncrypt(::arrow::util::span footer, @@ -135,9 +137,9 @@ void UpdateEncryptorMetadata( // and datatype length. PARQUET_EXPORT std::unique_ptr UpdateEncodingProperties( - std::string column_name, Type::type data_type, - std::optional datatype_length, Compression::type compression_type, - std::unique_ptr encoding_properties); + std::string column_name, Type::type data_type, std::optional datatype_length, + Compression::type compression_type, + std::unique_ptr encoding_properties); /// Factory for ExternalDBPAEncryptorAdapter instances. The cache exists while the write /// operation is open, and is used to guarantee the lifetime of the encryptor. @@ -191,11 +193,12 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { /// Decrypt is not supported as we cannot calculate the plaintext length before /// decryption. - int32_t Decrypt(::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, - ::arrow::util::span plaintext, - std::unique_ptr encoding_properties = nullptr) override { + int32_t Decrypt( + ::arrow::util::span ciphertext, + ::arrow::util::span key, + ::arrow::util::span aad, + ::arrow::util::span plaintext, + std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; ss << "Decrypt is not supported in ExternalDBPADecryptorAdapter, "; ss << "use DecryptWithManagedBuffer instead"; @@ -206,9 +209,10 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { /// The buffer will be resized to the correct size during decryption. This method /// is used when the decryptor cannot calculate the plaintext length before /// decryption. - int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, - std::unique_ptr encoding_properties = nullptr) override; + int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, + ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties = nullptr) override; private: // agent_instance is assumed to be initialized at the time of construction. diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc index c70decdede5e..28bb56bb53ae 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc @@ -95,9 +95,8 @@ class ExternalDBPAEncryptorAdapterTest : public ::testing::Test { std::shared_ptr ciphertext_buffer = AllocateBuffer(::arrow::default_memory_pool(), expected_ciphertext_length); - int32_t encryption_length = - encryptor->EncryptWithManagedBuffer(str2span(plaintext), ciphertext_buffer.get(), - builder.Build()); + int32_t encryption_length = encryptor->EncryptWithManagedBuffer( + str2span(plaintext), ciphertext_buffer.get(), builder.Build()); ASSERT_EQ(expected_ciphertext_length, encryption_length); std::string ciphertext_str(ciphertext_buffer->data(), @@ -500,9 +499,8 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithWrongKeyIdFails) { AllocateBuffer(::arrow::default_memory_pool(), 0); std::string empty; - int32_t enc_len = - encryptor->EncryptWithManagedBuffer( - str2span(plaintext), ciphertext_buffer.get(), builder.Build()); + int32_t enc_len = encryptor->EncryptWithManagedBuffer( + str2span(plaintext), ciphertext_buffer.get(), builder.Build()); std::string ciphertext_str(ciphertext_buffer->data(), ciphertext_buffer->data() + enc_len); @@ -517,8 +515,8 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithWrongKeyIdFails) { bool threw = false; int32_t dec_len = 0; try { - dec_len = decryptor->DecryptWithManagedBuffer(str2span(ciphertext_str), - plaintext_buffer.get(), builder.Build()); + dec_len = decryptor->DecryptWithManagedBuffer( + str2span(ciphertext_str), plaintext_buffer.get(), builder.Build()); } catch (const ParquetException&) { threw = true; } diff --git a/cpp/src/parquet/encryption/internal_file_decryptor.cc b/cpp/src/parquet/encryption/internal_file_decryptor.cc index 34642ab32540..2520b712d541 100644 --- a/cpp/src/parquet/encryption/internal_file_decryptor.cc +++ b/cpp/src/parquet/encryption/internal_file_decryptor.cc @@ -60,11 +60,10 @@ int32_t Decryptor::Decrypt(::arrow::util::span ciphertext, } int32_t Decryptor::DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, std::unique_ptr encoding_properties) { - return decryptor_instance_->DecryptWithManagedBuffer( - ciphertext, plaintext, std::move(encoding_properties)); + return decryptor_instance_->DecryptWithManagedBuffer(ciphertext, plaintext, + std::move(encoding_properties)); } // InternalFileDecryptor diff --git a/cpp/src/parquet/encryption/internal_file_decryptor.h b/cpp/src/parquet/encryption/internal_file_decryptor.h index 91f7d0bc41fb..2039e42a5ec1 100644 --- a/cpp/src/parquet/encryption/internal_file_decryptor.h +++ b/cpp/src/parquet/encryption/internal_file_decryptor.h @@ -55,8 +55,7 @@ class PARQUET_EXPORT Decryptor { ::arrow::util::span plaintext, std::unique_ptr encoding_properties = nullptr); int32_t DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, std::unique_ptr encoding_properties = nullptr); private: diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.cc b/cpp/src/parquet/encryption/internal_file_encryptor.cc index 120a0bbfab6c..06618d917fcb 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.cc +++ b/cpp/src/parquet/encryption/internal_file_encryptor.cc @@ -51,11 +51,10 @@ int32_t Encryptor::Encrypt(::arrow::util::span plaintext, } int32_t Encryptor::EncryptWithManagedBuffer( - ::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties) { - return encryptor_instance_->EncryptWithManagedBuffer( - plaintext, ciphertext, std::move(encoding_properties)); + return encryptor_instance_->EncryptWithManagedBuffer(plaintext, ciphertext, + std::move(encoding_properties)); } std::shared_ptr Encryptor::GetKeyValueMetadata(int8_t module_type) { diff --git a/cpp/src/parquet/encryption/internal_file_encryptor.h b/cpp/src/parquet/encryption/internal_file_encryptor.h index adadb035f28f..148a11b70a71 100644 --- a/cpp/src/parquet/encryption/internal_file_encryptor.h +++ b/cpp/src/parquet/encryption/internal_file_encryptor.h @@ -52,8 +52,7 @@ class PARQUET_EXPORT Encryptor { std::unique_ptr encoding_properties = nullptr); int32_t EncryptWithManagedBuffer( - ::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties = nullptr); /// After the column_writer writes a dictionary or a data page, this method will diff --git a/cpp/src/parquet/file_deserialize_test.cc b/cpp/src/parquet/file_deserialize_test.cc index 037658fa520c..096f6c23e2ae 100644 --- a/cpp/src/parquet/file_deserialize_test.cc +++ b/cpp/src/parquet/file_deserialize_test.cc @@ -1027,21 +1027,25 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { ::arrow::util::span /*key*/, ::arrow::util::span /*aad*/, ::arrow::util::span plaintext, - std::unique_ptr encoding_properties) override { + std::unique_ptr + encoding_properties) override { std::copy(ciphertext.begin(), ciphertext.end(), plaintext.begin()); - // Fill column-level properties so validate() succeeds - encoding_properties->set_column_path(column_path_); - encoding_properties->set_physical_type(physical_type_); - encoding_properties->set_compression_codec(compression_codec_); - - encoding_properties->validate(); - sink_->entries.emplace_back(encoding_properties->ToPropertiesMap()); + if (encoding_properties != nullptr) { + // Fill column-level properties so validate() succeeds + encoding_properties->set_column_path(column_path_); + encoding_properties->set_physical_type(physical_type_); + encoding_properties->set_compression_codec(compression_codec_); + + encoding_properties->validate(); + sink_->entries.emplace_back(encoding_properties->ToPropertiesMap()); + } return static_cast(ciphertext.size()); } - int32_t DecryptWithManagedBuffer(::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, - std::unique_ptr encoding_properties) override { + int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties) + override { throw ParquetException("DecryptWithManagedBuffer not supported"); } From c169f27cf234e8f563dbbfe8e6f13ce86741b8ba Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Thu, 12 Feb 2026 17:03:26 -0600 Subject: [PATCH 3/7] More Linter errors --- cpp/src/parquet/encryption/aes_encryption.h | 12 ++++-------- .../encryption/external_dbpa_encryption.cc | 16 ++++++++-------- .../encryption/external_dbpa_encryption.h | 12 ++++-------- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/cpp/src/parquet/encryption/aes_encryption.h b/cpp/src/parquet/encryption/aes_encryption.h index e4346ce4d02b..788c602b7b32 100644 --- a/cpp/src/parquet/encryption/aes_encryption.h +++ b/cpp/src/parquet/encryption/aes_encryption.h @@ -82,16 +82,14 @@ class PARQUET_EXPORT AesEncryptor : public AesCryptoContext, public EncryptorInt /// If different from value in constructor, exception will be thrown. int32_t Encrypt( ::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span ciphertext, std::unique_ptr encoding_properties = nullptr) override; /// Encrypt the plaintext and leave the results in the ciphertext buffer. This method is /// not supported as we can calculate the ciphertext length before encryption. int32_t EncryptWithManagedBuffer( - ::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "EncryptWithManagedBuffer is not supported in AesEncryptor, use Encrypt instead"); @@ -174,8 +172,7 @@ class PARQUET_EXPORT AesDecryptor : public AesCryptoContext, public DecryptorInt /// large as PlaintextLength(ciphertext_len). int32_t Decrypt( ::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span plaintext, std::unique_ptr encoding_properties = nullptr) override; @@ -183,8 +180,7 @@ class PARQUET_EXPORT AesDecryptor : public AesCryptoContext, public DecryptorInt /// method is not supported as we can calculate the plaintext length before /// decryption. int32_t DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, std::unique_ptr encoding_properties = nullptr) override { throw ParquetException( "DecryptWithManagedBuffer is not supported in AesDecryptor, use Decrypt instead"); diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index 89de7fc1bca1..82ed8b577ee7 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -191,11 +191,11 @@ std::unique_ptr UpdateEncodingProperties( Compression::type compression_type, std::unique_ptr encoding_properties) { if (encoding_properties != nullptr) { - encoding_properties->set_column_path(column_name); - encoding_properties->set_physical_type(data_type, datatype_length); - encoding_properties->set_compression_codec(compression_type); + encoding_properties->set_column_path(column_name); + encoding_properties->set_physical_type(data_type, datatype_length); + encoding_properties->set_compression_codec(compression_type); - encoding_properties->validate(); + encoding_properties->validate(); } return encoding_properties; } // UpdateEncodingProperties() @@ -562,11 +562,11 @@ int32_t ExternalDBPADecryptorAdapter::DecryptWithManagedBuffer( if (encoding_properties == nullptr) { ARROW_LOG(ERROR) << "ExternalDBPADecryptorAdapter:: encoding_properties is nullptr"; throw ParquetException( - "ExternalDBPADecryptorAdapter:: encoding_properties not provided, params not updated"); + "ExternalDBPADecryptorAdapter:: encoding_properties is null, params not updated"); } - encoding_properties_ = UpdateEncodingProperties( - column_name_, data_type_, datatype_length_, compression_type_, - std::move(encoding_properties)); + encoding_properties_ = + UpdateEncodingProperties(column_name_, data_type_, datatype_length_, + compression_type_, std::move(encoding_properties)); return InvokeExternalDecrypt(ciphertext, plaintext, encoding_properties_->ToPropertiesMap()); } diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index da3ee144a6be..618a57b0123e 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -56,8 +56,7 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { /// Encryption not supported as we cannot calculate the ciphertext before encryption. int32_t Encrypt( ::arrow::util::span plaintext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span ciphertext, std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; @@ -69,8 +68,7 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { /// Encrypt the plaintext and leave the results in the ciphertext buffer. /// The buffer will be resized to the appropriate size by the agent during encryption. int32_t EncryptWithManagedBuffer( - ::arrow::util::span plaintext, - ::arrow::ResizableBuffer* ciphertext, + ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, std::unique_ptr encoding_properties = nullptr) override; /// Encrypts plaintext footer, in order to compute footer signature (tag). @@ -195,8 +193,7 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { /// decryption. int32_t Decrypt( ::arrow::util::span ciphertext, - ::arrow::util::span key, - ::arrow::util::span aad, + ::arrow::util::span key, ::arrow::util::span aad, ::arrow::util::span plaintext, std::unique_ptr encoding_properties = nullptr) override { std::stringstream ss; @@ -210,8 +207,7 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { /// is used when the decryptor cannot calculate the plaintext length before /// decryption. int32_t DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, - ::arrow::ResizableBuffer* plaintext, + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, std::unique_ptr encoding_properties = nullptr) override; private: From 0269fbeeff9866d6b204a5318707efefde77ea7b Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Wed, 25 Feb 2026 11:00:51 -0600 Subject: [PATCH 4/7] Iterate over interfaces change --- cpp/src/parquet/CMakeLists.txt | 4 +- .../parquet/arrow/arrow_reader_writer_test.cc | 67 ------------------- cpp/src/parquet/arrow/reader.cc | 22 ------ cpp/src/parquet/column_reader.cc | 8 +-- cpp/src/parquet/column_writer.cc | 37 +++++----- .../encryption/external_dbpa_encryption.cc | 12 ++-- .../encryption/external_dbpa_encryption.h | 2 +- .../external_dbpa_encryption_test.cc | 4 +- cpp/src/parquet/file_deserialize_test.cc | 2 + 9 files changed, 31 insertions(+), 127 deletions(-) diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt index 86d8a95a9c17..b0b75b84677c 100644 --- a/cpp/src/parquet/CMakeLists.txt +++ b/cpp/src/parquet/CMakeLists.txt @@ -506,9 +506,7 @@ add_parquet_test(arrow-reader-writer-test SOURCES arrow/arrow_reader_writer_test.cc arrow/arrow_statistics_test.cc - arrow/variant_test.cc - $<$:encryption/external/test_utils.cc> -) + arrow/variant_test.cc) add_parquet_test(arrow-internals-test SOURCES arrow/path_internal_test.cc arrow/reconstruct_internal_test.cc) diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc index 7c7b62b1fc55..8f89ef6d4d1f 100644 --- a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc +++ b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc @@ -76,10 +76,6 @@ #include "parquet/properties.h" #include "parquet/test_util.h" -#ifdef PARQUET_REQUIRE_ENCRYPTION -# include "parquet/encryption/external/test_utils.h" -#endif - using arrow::Array; using arrow::ArrayData; using arrow::ArrayFromJSON; @@ -1965,69 +1961,6 @@ TEST(TestArrowReadWrite, UseDeprecatedInt96) { ASSERT_NO_FATAL_FAILURE(::arrow::AssertTablesEqual(*ex_result, *result)); } -#ifdef PARQUET_REQUIRE_ENCRYPTION -TEST(ExternalDbpaConcurrencyTest, FailsWhenUseThreadsTrue) { - std::shared_ptr<::arrow::Array> arr; - ::arrow::Int32Builder b; - ASSERT_OK(b.AppendValues({1, 2, 3})); - ASSERT_OK(b.Finish(&arr)); - auto schema = ::arrow::schema({::arrow::field("f0", ::arrow::int32())}); - auto table = ::arrow::Table::Make(schema, {arr}); - - ::arrow::util::SecureString column_key(std::string("key1234567890123")); - ::arrow::util::SecureString footer_key(std::string("footer_key123456")); - - std::map> enc_cols; - parquet::ColumnEncryptionProperties::Builder col_builder("f0"); - col_builder.parquet_cipher(parquet::ParquetCipher::EXTERNAL_DBPA_V1); - col_builder.key_id("key1234567890123"); - col_builder.key(column_key); - enc_cols["f0"] = col_builder.build(); - - const std::string lib_path = - parquet::encryption::external::test::TestUtils::GetTestLibraryPath(); - - parquet::ExternalFileEncryptionProperties::Builder fep_builder(footer_key); - fep_builder.footer_key_metadata("kf") - ->encrypted_columns(enc_cols) - ->algorithm(parquet::ParquetCipher::AES_GCM_V1) - ->configuration_properties( - {{parquet::ParquetCipher::EXTERNAL_DBPA_V1, - {{"agent_library_path", lib_path}, {"file_path", "/tmp/test"}}}}); - - auto writer_props = parquet::WriterProperties::Builder() - .encryption(fep_builder.build_external()) - ->build(); - - ASSERT_OK_AND_ASSIGN( - auto sink, ::arrow::io::BufferOutputStream::Create(1 << 16, default_memory_pool())); - ASSERT_OK(parquet::arrow::WriteTable(*table, default_memory_pool(), sink, - /*chunk_size=*/1024, writer_props)); - ASSERT_OK_AND_ASSIGN(auto buffer, sink->Finish()); - - auto kr = std::make_shared(); - kr->PutKey("kf", footer_key); - kr->PutKey("key1234567890123", column_key); - - parquet::ExternalFileDecryptionProperties::Builder dep_builder; - dep_builder.key_retriever(kr)->app_context("{}")->configuration_properties( - {{parquet::ParquetCipher::EXTERNAL_DBPA_V1, - {{"agent_library_path", lib_path}, {"file_path", "/tmp/test"}}}}); - parquet::ReaderProperties rp = parquet::default_reader_properties(); - rp.file_decryption_properties(dep_builder.build_external()); - - ArrowReaderProperties arp; - arp.set_use_threads(true); - - parquet::arrow::FileReaderBuilder frb; - ASSERT_OK(frb.Open(std::make_shared(buffer), rp)); - frb.properties(arp); - - std::unique_ptr fr; - ASSERT_RAISES(Invalid, frb.Build(&fr)); -} -#endif - TEST(TestArrowReadWrite, DownsampleDeprecatedInt96) { using ::arrow::ArrayFromJSON; using ::arrow::field; diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc index f6d92fb81810..138ed44f750f 100644 --- a/cpp/src/parquet/arrow/reader.cc +++ b/cpp/src/parquet/arrow/reader.cc @@ -131,24 +131,6 @@ std::shared_ptr> VectorToSharedSet( return result; } -bool IsExternalDBPAEncryptionUsedInColumns(std::shared_ptr metadata) { - for (int row_group = 0; row_group < metadata->num_row_groups(); row_group++) { - auto row_group_metadata = metadata->RowGroup(row_group); - for (int column = 0; column < row_group_metadata->num_columns(); column++) { - auto column_metadata = row_group_metadata->ColumnChunk(column); - if (column_metadata->crypto_metadata()) { - auto crypto_metadata = column_metadata->crypto_metadata(); - if (crypto_metadata->is_encryption_algorithm_set() && - crypto_metadata->encryption_algorithm().algorithm == - ParquetCipher::EXTERNAL_DBPA_V1) { - return true; - } - } - } - } - return false; -} - // Forward declaration Status GetReader(const SchemaField& field, const std::shared_ptr& context, std::unique_ptr* out); @@ -169,10 +151,6 @@ class FileReaderImpl : public FileReader { // is not safe to use multiple threads to read the file. if (reader_properties_.use_threads()) { auto metadata = reader_->metadata(); - if (IsExternalDBPAEncryptionUsedInColumns(metadata)) { - return Status::Invalid( - "EXTERNAL_DBPA_V1 encryption does not support multiple threads"); - } } return SchemaManifest::Make(reader_->metadata()->schema(), reader_->metadata()->key_value_metadata(), diff --git a/cpp/src/parquet/column_reader.cc b/cpp/src/parquet/column_reader.cc index 89236ee9a7db..8f81c7951aba 100644 --- a/cpp/src/parquet/column_reader.cc +++ b/cpp/src/parquet/column_reader.cc @@ -571,18 +571,16 @@ std::shared_ptr SerializedPageReader::NextPage() { // Decrypt it if we need to if (data_decryptor_ != nullptr) { - std::unique_ptr encoding_properties = - GetEncodingProperties(current_page_header_); - std::shared_ptr decryption_buffer; if (data_decryptor_->CanCalculateLengths()) { decryption_buffer = AllocateBuffer( properties_.memory_pool(), data_decryptor_->PlaintextLength(compressed_len)); compressed_len = data_decryptor_->Decrypt(page_buffer->span_as(), - decryption_buffer->mutable_span_as(), - std::move(encoding_properties)); + decryption_buffer->mutable_span_as()); } else { + std::unique_ptr encoding_properties = + GetEncodingProperties(current_page_header_); decryption_buffer = AllocateBuffer(properties_.memory_pool(), 0); compressed_len = data_decryptor_->DecryptWithManagedBuffer( page_buffer->span_as(), decryption_buffer.get(), diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc index 010f4ee7a06b..9e4a1cec9069 100644 --- a/cpp/src/parquet/column_writer.cc +++ b/cpp/src/parquet/column_writer.cc @@ -307,10 +307,6 @@ class SerializedPageWriter : public PageWriter { if (data_encryptor_.get()) { UpdateEncryption(encryption::kDictionaryPage); - // Creating an EncodingProperties object from the metadata. - // We're retrieving the column descriptor and writer properties - // from the metadata_ object to simplify the code. - // // WriterProperties is created using WriterProperties::Builder::build() (in // parquet/properties.h) via ParquetFileFormat::MakeWriter (in // arrow/dataset/file_parquet.cc), and passed down to @@ -322,19 +318,20 @@ class SerializedPageWriter : public PageWriter { // where a ColumnDescriptor is extracted from the SchemaDescriptor, and // passed into ColumnChunkMetaDataBuilder::Make() - std::unique_ptr encoding_properties = - EncodingProperties::MakeFromMetadata(metadata_->descr(), - metadata_->properties(), - static_cast(page)); - if (data_encryptor_->CanCalculateCiphertextLength()) { PARQUET_THROW_NOT_OK(encryption_buffer_->Resize( data_encryptor_->CiphertextLength(output_data_len), false)); output_data_len = data_encryptor_->Encrypt(compressed_data->span_as(), - encryption_buffer_->mutable_span_as(), - std::move(encoding_properties)); + encryption_buffer_->mutable_span_as()); } else { + // Creating an EncodingProperties object from the metadata. + // We're retrieving the column descriptor and writer properties + // from the metadata_ object to simplify the code. + std::unique_ptr encoding_properties = + EncodingProperties::MakeFromMetadata(metadata_->descr(), + metadata_->properties(), + static_cast(page)); output_data_len = data_encryptor_->EncryptWithManagedBuffer( compressed_data->span_as(), encryption_buffer_.get(), std::move(encoding_properties)); @@ -434,10 +431,6 @@ class SerializedPageWriter : public PageWriter { if (data_encryptor_.get()) { UpdateEncryption(encryption::kDataPage); - // Creating an EncodingProperties object from the metadata. - // We're retrieving the column descriptor and writer properties - // from the metadata_ object to simplify the code. - // // WriterProperties is created using WriterProperties::Builder::build() // (in parquet/properties.h ) via ParquetFileFormat::MakeWriter // (in arrow/dataset/file_parquet.cc), and passed down to @@ -448,19 +441,21 @@ class SerializedPageWriter : public PageWriter { // RowGroupMetadataBuilder NextColumnChunk() (parquet/metadata.cc) where a // ColumnDescriptor is extracted from the SchemaDescriptor, and passed into // ColumnChunkMetaDataBuilder::Make() - std::unique_ptr encoding_properties = - EncodingProperties::MakeFromMetadata(metadata_->descr(), - metadata_->properties(), - static_cast(page)); if (data_encryptor_->CanCalculateCiphertextLength()) { PARQUET_THROW_NOT_OK(encryption_buffer_->Resize( data_encryptor_->CiphertextLength(output_data_len), false)); output_data_len = data_encryptor_->Encrypt(compressed_data->span_as(), - encryption_buffer_->mutable_span_as(), - std::move(encoding_properties)); + encryption_buffer_->mutable_span_as()); } else { + // Creating an EncodingProperties object from the metadata. + // We're retrieving the column descriptor and writer properties + // from the metadata_ object to simplify the code. + std::unique_ptr encoding_properties = + EncodingProperties::MakeFromMetadata(metadata_->descr(), + metadata_->properties(), + static_cast(page)); output_data_len = data_encryptor_->EncryptWithManagedBuffer( compressed_data->span_as(), encryption_buffer_.get(), std::move(encoding_properties)); diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index 82ed8b577ee7..59ebdf26057f 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -186,7 +186,7 @@ void UpdateEncryptorMetadata( // Some Encryptors and Decryptors may need to understand the page encoding before the // encryption process. This method will be called from the Encrypt and Decrypt // WithManagedBuffer methods. -std::unique_ptr UpdateEncodingProperties( +std::unique_ptr BuildEncodingProperties( std::string column_name, Type::type data_type, std::optional datatype_length, Compression::type compression_type, std::unique_ptr encoding_properties) { @@ -198,7 +198,7 @@ std::unique_ptr UpdateEncodingProperties( encoding_properties->validate(); } return encoding_properties; -} // UpdateEncodingProperties() +} // BuildEncodingProperties() std::optional> ExternalDBPAUtils::KeyValueMetadataToStringMap( @@ -328,8 +328,8 @@ int32_t ExternalDBPAEncryptorAdapter::EncryptWithManagedBuffer( "ExternalDBPAEncryptorAdapter:: encoding_properties is null, params not updated"); } encoding_properties_ = - UpdateEncodingProperties(column_name_, data_type_, datatype_length_, - compression_type_, std::move(encoding_properties)); + BuildEncodingProperties(column_name_, data_type_, datatype_length_, + compression_type_, std::move(encoding_properties)); return InvokeExternalEncrypt(plaintext, ciphertext, encoding_properties_->ToPropertiesMap()); } @@ -565,8 +565,8 @@ int32_t ExternalDBPADecryptorAdapter::DecryptWithManagedBuffer( "ExternalDBPADecryptorAdapter:: encoding_properties is null, params not updated"); } encoding_properties_ = - UpdateEncodingProperties(column_name_, data_type_, datatype_length_, - compression_type_, std::move(encoding_properties)); + BuildEncodingProperties(column_name_, data_type_, datatype_length_, + compression_type_, std::move(encoding_properties)); return InvokeExternalDecrypt(ciphertext, plaintext, encoding_properties_->ToPropertiesMap()); } diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index 618a57b0123e..403d37fda461 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -134,7 +134,7 @@ void UpdateEncryptorMetadata( // Update the encoding properties based on the column name, data type, compression type, // and datatype length. PARQUET_EXPORT -std::unique_ptr UpdateEncodingProperties( +std::unique_ptr BuildEncodingProperties( std::string column_name, Type::type data_type, std::optional datatype_length, Compression::type compression_type, std::unique_ptr encoding_properties); diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc index 28bb56bb53ae..3a7225ea781d 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption_test.cc @@ -196,7 +196,7 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, SignedFooterEncryptionThrowsException) ParquetException); } -TEST_F(ExternalDBPAEncryptorAdapterTest, EncryptWithoutUpdateEncodingPropertiesThrows) { +TEST_F(ExternalDBPAEncryptorAdapterTest, EncryptWithoutBuildEncodingPropertiesThrows) { ParquetCipher::type algorithm = ParquetCipher::EXTERNAL_DBPA_V1; std::string column_name = "employee_name"; std::string key_id = "employee_name_key"; @@ -215,7 +215,7 @@ TEST_F(ExternalDBPAEncryptorAdapterTest, EncryptWithoutUpdateEncodingPropertiesT ParquetException); } -TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithoutUpdateEncodingPropertiesThrows) { +TEST_F(ExternalDBPAEncryptorAdapterTest, DecryptWithoutBuildEncodingPropertiesThrows) { ParquetCipher::type algorithm = ParquetCipher::EXTERNAL_DBPA_V1; std::string column_name = "employee_name"; std::string key_id = "employee_name_key"; diff --git a/cpp/src/parquet/file_deserialize_test.cc b/cpp/src/parquet/file_deserialize_test.cc index 096f6c23e2ae..6f9582bdbb31 100644 --- a/cpp/src/parquet/file_deserialize_test.cc +++ b/cpp/src/parquet/file_deserialize_test.cc @@ -1038,6 +1038,8 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { encoding_properties->validate(); sink_->entries.emplace_back(encoding_properties->ToPropertiesMap()); + } else { + throw ParquetException("Encoding properties are null"); } return static_cast(ciphertext.size()); } From 0ef00032bc746dc5ee5d3d9c2340d44273449b23 Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Wed, 25 Feb 2026 14:50:55 -0600 Subject: [PATCH 5/7] Fixing test and linter --- cpp/src/parquet/column_writer.cc | 6 +++--- cpp/src/parquet/file_deserialize_test.cc | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc index 9e4a1cec9069..040bddf66325 100644 --- a/cpp/src/parquet/column_writer.cc +++ b/cpp/src/parquet/column_writer.cc @@ -329,9 +329,9 @@ class SerializedPageWriter : public PageWriter { // We're retrieving the column descriptor and writer properties // from the metadata_ object to simplify the code. std::unique_ptr encoding_properties = - EncodingProperties::MakeFromMetadata(metadata_->descr(), - metadata_->properties(), - static_cast(page)); + EncodingProperties::MakeFromMetadata( + metadata_->descr(), metadata_->properties(), + static_cast(page)); output_data_len = data_encryptor_->EncryptWithManagedBuffer( compressed_data->span_as(), encryption_buffer_.get(), std::move(encoding_properties)); diff --git a/cpp/src/parquet/file_deserialize_test.cc b/cpp/src/parquet/file_deserialize_test.cc index 6f9582bdbb31..c4e55bfae7d8 100644 --- a/cpp/src/parquet/file_deserialize_test.cc +++ b/cpp/src/parquet/file_deserialize_test.cc @@ -1013,7 +1013,7 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { physical_type_(physical_type), compression_codec_(compression_codec) {} - [[nodiscard]] bool CanCalculateLengths() const override { return true; } + [[nodiscard]] bool CanCalculateLengths() const override { return false; } [[nodiscard]] int32_t PlaintextLength(int32_t ciphertext_len) const override { return ciphertext_len; @@ -1029,7 +1029,15 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { ::arrow::util::span plaintext, std::unique_ptr encoding_properties) override { - std::copy(ciphertext.begin(), ciphertext.end(), plaintext.begin()); + throw ParquetException("Decrypt not supported"); + } + + int32_t DecryptWithManagedBuffer( + ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, + std::unique_ptr encoding_properties) + override { + PARQUET_THROW_NOT_OK(plaintext->Resize(ciphertext.size())); + std::memcpy(plaintext->mutable_data(), ciphertext.data(), ciphertext.size()); if (encoding_properties != nullptr) { // Fill column-level properties so validate() succeeds encoding_properties->set_column_path(column_path_); @@ -1044,13 +1052,6 @@ class CapturingTestDecryptor : public parquet::encryption::DecryptorInterface { return static_cast(ciphertext.size()); } - int32_t DecryptWithManagedBuffer( - ::arrow::util::span ciphertext, ::arrow::ResizableBuffer* plaintext, - std::unique_ptr encoding_properties) - override { - throw ParquetException("DecryptWithManagedBuffer not supported"); - } - private: std::shared_ptr sink_; std::string column_path_; From 7b858822f530ebc3d5193eb3505c3c04e9684787 Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Wed, 25 Feb 2026 18:21:23 -0600 Subject: [PATCH 6/7] Removed global encoding_properties variables --- .../encryption/external_dbpa_encryption.cc | 43 +++++++++---------- .../encryption/external_dbpa_encryption.h | 16 +++---- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index 59ebdf26057f..41201a2e7854 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -186,19 +186,17 @@ void UpdateEncryptorMetadata( // Some Encryptors and Decryptors may need to understand the page encoding before the // encryption process. This method will be called from the Encrypt and Decrypt // WithManagedBuffer methods. -std::unique_ptr BuildEncodingProperties( - std::string column_name, Type::type data_type, std::optional datatype_length, - Compression::type compression_type, - std::unique_ptr encoding_properties) { - if (encoding_properties != nullptr) { - encoding_properties->set_column_path(column_name); - encoding_properties->set_physical_type(data_type, datatype_length); - encoding_properties->set_compression_codec(compression_type); - - encoding_properties->validate(); - } - return encoding_properties; -} // BuildEncodingProperties() +void PopulateEncodingProperties( + EncodingProperties* encoding_properties, std::string column_name, + Type::type data_type, std::optional datatype_length, + Compression::type compression_type) { + if (!encoding_properties) return; + encoding_properties->set_column_path(column_name); + encoding_properties->set_physical_type(data_type, datatype_length); + encoding_properties->set_compression_codec(compression_type); + + encoding_properties->validate(); +} // PopulateEncodingProperties() std::optional> ExternalDBPAUtils::KeyValueMetadataToStringMap( @@ -327,11 +325,10 @@ int32_t ExternalDBPAEncryptorAdapter::EncryptWithManagedBuffer( throw ParquetException( "ExternalDBPAEncryptorAdapter:: encoding_properties is null, params not updated"); } - encoding_properties_ = - BuildEncodingProperties(column_name_, data_type_, datatype_length_, - compression_type_, std::move(encoding_properties)); + PopulateEncodingProperties(encoding_properties.get(), column_name_, data_type_, + datatype_length_, compression_type_); return InvokeExternalEncrypt(plaintext, ciphertext, - encoding_properties_->ToPropertiesMap()); + std::move(encoding_properties)); } int32_t ExternalDBPAEncryptorAdapter::SignedFooterEncrypt( @@ -344,7 +341,7 @@ int32_t ExternalDBPAEncryptorAdapter::SignedFooterEncrypt( int32_t ExternalDBPAEncryptorAdapter::InvokeExternalEncrypt( ::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, - std::map encoding_attrs) { + std::unique_ptr encoding_properties) { if (::arrow::util::ArrowLog::IsLevelEnabled( ::arrow::util::ArrowLogLevel::ARROW_DEBUG)) { ARROW_LOG(DEBUG) << "*-*-*- START: ExternalDBPAEncryptor::Encrypt *-*-*-"; @@ -361,6 +358,7 @@ int32_t ExternalDBPAEncryptorAdapter::InvokeExternalEncrypt( } ARROW_LOG(DEBUG) << "Calling agent_instance_->Encrypt..."; + auto encoding_attrs = encoding_properties->ToPropertiesMap(); std::unique_ptr result = agent_instance_->Encrypt(plaintext, std::move(encoding_attrs)); @@ -394,7 +392,7 @@ int32_t ExternalDBPAEncryptorAdapter::InvokeExternalEncrypt( // Accumulate any column_encryption_metadata returned by the result per module type UpdateEncryptorMetadata( /*metadata_by_module*/ column_encryption_metadata_, - /*encoding_properties*/ *encoding_properties_, + /*encoding_properties*/ *encoding_properties, /*result*/ *result); return static_cast(result->size()); @@ -564,11 +562,10 @@ int32_t ExternalDBPADecryptorAdapter::DecryptWithManagedBuffer( throw ParquetException( "ExternalDBPADecryptorAdapter:: encoding_properties is null, params not updated"); } - encoding_properties_ = - BuildEncodingProperties(column_name_, data_type_, datatype_length_, - compression_type_, std::move(encoding_properties)); + PopulateEncodingProperties(encoding_properties.get(), column_name_, data_type_, + datatype_length_, compression_type_); return InvokeExternalDecrypt(ciphertext, plaintext, - encoding_properties_->ToPropertiesMap()); + encoding_properties->ToPropertiesMap()); } int32_t ExternalDBPADecryptorAdapter::InvokeExternalDecrypt( diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index 403d37fda461..6a3d2f28e165 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -93,7 +93,7 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { int32_t InvokeExternalEncrypt(::arrow::util::span plaintext, ::arrow::ResizableBuffer* ciphertext, - std::map encoding_attrs); + std::unique_ptr encoding_properties); ParquetCipher::type algorithm_; std::string column_name_; @@ -106,8 +106,6 @@ class PARQUET_EXPORT ExternalDBPAEncryptorAdapter : public EncryptorInterface { std::unique_ptr agent_instance_; - std::unique_ptr encoding_properties_; - // Accumulated column encryption metadata per module type (e.g., data page, // dictionary page) to be used later by GetKeyValueMetadata. std::map> column_encryption_metadata_; @@ -131,13 +129,13 @@ void UpdateEncryptorMetadata( const EncodingProperties& encoding_properties, const dbps::external::EncryptionResult& result); -// Update the encoding properties based on the column name, data type, compression type, +// Populate the encoding properties based on the column name, data type, compression type, // and datatype length. PARQUET_EXPORT -std::unique_ptr BuildEncodingProperties( - std::string column_name, Type::type data_type, std::optional datatype_length, - Compression::type compression_type, - std::unique_ptr encoding_properties); +void PopulateEncodingProperties( + EncodingProperties* encoding_properties, std::string column_name, + Type::type data_type, std::optional datatype_length, + Compression::type compression_type); /// Factory for ExternalDBPAEncryptorAdapter instances. The cache exists while the write /// operation is open, and is used to guarantee the lifetime of the encryptor. @@ -236,8 +234,6 @@ class PARQUET_EXPORT ExternalDBPADecryptorAdapter : public DecryptorInterface { std::unique_ptr agent_instance_; - std::unique_ptr encoding_properties_; - // Store the key value metadata from the column chunk metadata. std::shared_ptr key_value_metadata_; }; From 58e17ba01b07a94df44644bdaff12bd822c0fa4e Mon Sep 17 00:00:00 2001 From: Sofia Brenes Date: Wed, 25 Feb 2026 18:44:38 -0600 Subject: [PATCH 7/7] Linter fixes --- .../parquet/encryption/external_dbpa_encryption.cc | 11 +++++------ cpp/src/parquet/encryption/external_dbpa_encryption.h | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.cc b/cpp/src/parquet/encryption/external_dbpa_encryption.cc index 41201a2e7854..6683430bff21 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.cc +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.cc @@ -186,10 +186,10 @@ void UpdateEncryptorMetadata( // Some Encryptors and Decryptors may need to understand the page encoding before the // encryption process. This method will be called from the Encrypt and Decrypt // WithManagedBuffer methods. -void PopulateEncodingProperties( - EncodingProperties* encoding_properties, std::string column_name, - Type::type data_type, std::optional datatype_length, - Compression::type compression_type) { +void PopulateEncodingProperties(EncodingProperties* encoding_properties, + std::string column_name, Type::type data_type, + std::optional datatype_length, + Compression::type compression_type) { if (!encoding_properties) return; encoding_properties->set_column_path(column_name); encoding_properties->set_physical_type(data_type, datatype_length); @@ -327,8 +327,7 @@ int32_t ExternalDBPAEncryptorAdapter::EncryptWithManagedBuffer( } PopulateEncodingProperties(encoding_properties.get(), column_name_, data_type_, datatype_length_, compression_type_); - return InvokeExternalEncrypt(plaintext, ciphertext, - std::move(encoding_properties)); + return InvokeExternalEncrypt(plaintext, ciphertext, std::move(encoding_properties)); } int32_t ExternalDBPAEncryptorAdapter::SignedFooterEncrypt( diff --git a/cpp/src/parquet/encryption/external_dbpa_encryption.h b/cpp/src/parquet/encryption/external_dbpa_encryption.h index 6a3d2f28e165..00d33464e1a1 100644 --- a/cpp/src/parquet/encryption/external_dbpa_encryption.h +++ b/cpp/src/parquet/encryption/external_dbpa_encryption.h @@ -132,10 +132,10 @@ void UpdateEncryptorMetadata( // Populate the encoding properties based on the column name, data type, compression type, // and datatype length. PARQUET_EXPORT -void PopulateEncodingProperties( - EncodingProperties* encoding_properties, std::string column_name, - Type::type data_type, std::optional datatype_length, - Compression::type compression_type); +void PopulateEncodingProperties(EncodingProperties* encoding_properties, + std::string column_name, Type::type data_type, + std::optional datatype_length, + Compression::type compression_type); /// Factory for ExternalDBPAEncryptorAdapter instances. The cache exists while the write /// operation is open, and is used to guarantee the lifetime of the encryptor.