This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 0bd4e73cdb GH-37891: [C++][Parquet] Refine several classes in Parquet
encryption (#46202)
0bd4e73cdb is described below
commit 0bd4e73cdbb31e48cc5a678062d82bd3957308bc
Author: Eddie Chang <[email protected]>
AuthorDate: Thu Jun 19 20:50:51 2025 +0800
GH-37891: [C++][Parquet] Refine several classes in Parquet encryption
(#46202)
### What changes are included in this PR?
* Pass movable objects by value to functions that keep a copy of them.
* Use emplace() instead of insert() when the arguments can be moved safely.
* Remove unnecessary const qualifier on member variables that makes the
class non-copyable and non-movable.
* Remove unnecessary constructors and destructors that can be generated
automatically by compilers.
### Are these changes tested?
Yes, by existing tests.
### Are there any user-facing changes?
No.
* GitHub Issue: #37891
Authored-by: Eddie Chang <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
.../low_level_api/encryption_reader_writer.cc | 4 +-
.../encryption_reader_writer_all_crypto_options.cc | 18 ++--
cpp/src/parquet/encryption/crypto_factory.cc | 6 +-
cpp/src/parquet/encryption/encryption.cc | 110 ++++++++++-----------
cpp/src/parquet/encryption/encryption.h | 90 +++++++----------
.../encryption/file_system_key_material_store.cc | 17 ++--
.../encryption/file_system_key_material_store.h | 10 +-
cpp/src/parquet/encryption/key_toolkit.cc | 5 +-
cpp/src/parquet/encryption/properties_test.cc | 14 +--
.../parquet/encryption/read_configurations_test.cc | 8 +-
.../encryption/write_configurations_test.cc | 10 +-
11 files changed, 136 insertions(+), 156 deletions(-)
diff --git a/cpp/examples/parquet/low_level_api/encryption_reader_writer.cc
b/cpp/examples/parquet/low_level_api/encryption_reader_writer.cc
index aa0f07ff5c..e0a1f80b6b 100644
--- a/cpp/examples/parquet/low_level_api/encryption_reader_writer.cc
+++ b/cpp/examples/parquet/low_level_api/encryption_reader_writer.cc
@@ -81,7 +81,7 @@ int main(int argc, char** argv) {
parquet::WriterProperties::Builder builder;
// Add the current encryption configuration to WriterProperties.
builder.encryption(file_encryption_builder.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols)
+ ->encrypted_columns(std::move(encryption_cols))
->build());
// Add other writer properties
@@ -216,7 +216,7 @@ int main(int argc, char** argv) {
// Add the current decryption configuration to ReaderProperties.
reader_properties.file_decryption_properties(
- file_decryption_builder.key_retriever(kr1)->build());
+ file_decryption_builder.key_retriever(std::move(kr1))->build());
// Create a ParquetReader instance
std::unique_ptr<parquet::ParquetFileReader> parquet_reader =
diff --git
a/cpp/examples/parquet/low_level_api/encryption_reader_writer_all_crypto_options.cc
b/cpp/examples/parquet/low_level_api/encryption_reader_writer_all_crypto_options.cc
index b564cddcb5..6b6e9b6b1b 100644
---
a/cpp/examples/parquet/low_level_api/encryption_reader_writer_all_crypto_options.cc
+++
b/cpp/examples/parquet/low_level_api/encryption_reader_writer_all_crypto_options.cc
@@ -185,7 +185,7 @@ void InteropTestWriteEncryptedParquetFiles(std::string
root_path) {
vector_of_encryption_configurations.push_back(
file_encryption_builder_2.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols2)
+ ->encrypted_columns(std::move(encryption_cols2))
->build());
// Encryption configuration 3: Encrypt two columns, with different keys.
@@ -205,7 +205,7 @@ void InteropTestWriteEncryptedParquetFiles(std::string
root_path) {
vector_of_encryption_configurations.push_back(
file_encryption_builder_3.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols3)
+ ->encrypted_columns(std::move(encryption_cols3))
->set_plaintext_footer()
->build());
@@ -225,7 +225,7 @@ void InteropTestWriteEncryptedParquetFiles(std::string
root_path) {
vector_of_encryption_configurations.push_back(
file_encryption_builder_4.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols4)
+ ->encrypted_columns(std::move(encryption_cols4))
->aad_prefix(fileName)
->build());
@@ -244,7 +244,7 @@ void InteropTestWriteEncryptedParquetFiles(std::string
root_path) {
kFooterEncryptionKey);
vector_of_encryption_configurations.push_back(
- file_encryption_builder_5.encrypted_columns(encryption_cols5)
+ file_encryption_builder_5.encrypted_columns(std::move(encryption_cols5))
->footer_key_metadata("kf")
->aad_prefix(fileName)
->disable_aad_prefix_storage()
@@ -266,7 +266,7 @@ void InteropTestWriteEncryptedParquetFiles(std::string
root_path) {
vector_of_encryption_configurations.push_back(
file_encryption_builder_6.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols6)
+ ->encrypted_columns(std::move(encryption_cols6))
->algorithm(parquet::ParquetCipher::AES_GCM_CTR_V1)
->build());
@@ -373,7 +373,7 @@ void InteropTestReadEncryptedParquetFiles(std::string
root_path) {
parquet::FileDecryptionProperties::Builder file_decryption_builder_1;
vector_of_decryption_configurations.push_back(
- file_decryption_builder_1.key_retriever(kr1)->build());
+ file_decryption_builder_1.key_retriever(std::move(kr1))->build());
// Decryption configuration 2: Decrypt using key retriever callback that
holds the keys
// of two encrypted columns and the footer key. Supply aad_prefix.
@@ -387,7 +387,9 @@ void InteropTestReadEncryptedParquetFiles(std::string
root_path) {
parquet::FileDecryptionProperties::Builder file_decryption_builder_2;
vector_of_decryption_configurations.push_back(
-
file_decryption_builder_2.key_retriever(kr2)->aad_prefix(fileName)->build());
+ file_decryption_builder_2.key_retriever(std::move(kr2))
+ ->aad_prefix(fileName)
+ ->build());
// Decryption configuration 3: Decrypt using explicit column and footer keys.
std::string path_double = "double_field";
@@ -405,7 +407,7 @@ void InteropTestReadEncryptedParquetFiles(std::string
root_path) {
parquet::FileDecryptionProperties::Builder file_decryption_builder_3;
vector_of_decryption_configurations.push_back(
file_decryption_builder_3.footer_key(kFooterEncryptionKey)
- ->column_keys(decryption_cols)
+ ->column_keys(std::move(decryption_cols))
->build());
/**********************************************************************************
diff --git a/cpp/src/parquet/encryption/crypto_factory.cc
b/cpp/src/parquet/encryption/crypto_factory.cc
index 56069d5597..f420a7307e 100644
--- a/cpp/src/parquet/encryption/crypto_factory.cc
+++ b/cpp/src/parquet/encryption/crypto_factory.cc
@@ -79,13 +79,13 @@ std::shared_ptr<FileEncryptionProperties>
CryptoFactory::GetFileEncryptionProper
FileEncryptionProperties::Builder properties_builder =
FileEncryptionProperties::Builder(footer_key);
- properties_builder.footer_key_metadata(footer_key_metadata);
+ properties_builder.footer_key_metadata(std::move(footer_key_metadata));
properties_builder.algorithm(encryption_config.encryption_algorithm);
if (!encryption_config.uniform_encryption) {
ColumnPathToEncryptionPropertiesMap encrypted_columns =
GetColumnEncryptionProperties(dek_length, column_key_str,
&key_wrapper);
- properties_builder.encrypted_columns(encrypted_columns);
+ properties_builder.encrypted_columns(std::move(encrypted_columns));
if (encryption_config.plaintext_footer) {
properties_builder.set_plaintext_footer();
@@ -175,7 +175,7 @@ std::shared_ptr<FileDecryptionProperties>
CryptoFactory::GetFileDecryptionProper
file_path, file_system);
return FileDecryptionProperties::Builder()
- .key_retriever(key_retriever)
+ .key_retriever(std::move(key_retriever))
->plaintext_files_allowed()
->build();
}
diff --git a/cpp/src/parquet/encryption/encryption.cc
b/cpp/src/parquet/encryption/encryption.cc
index 95a717c5e3..9085076390 100644
--- a/cpp/src/parquet/encryption/encryption.cc
+++ b/cpp/src/parquet/encryption/encryption.cc
@@ -54,20 +54,19 @@ ColumnEncryptionProperties::Builder*
ColumnEncryptionProperties::Builder::key(
if (column_key.empty()) return this;
DCHECK(key_.empty());
- key_ = column_key;
+ key_ = std::move(column_key);
return this;
}
ColumnEncryptionProperties::Builder*
ColumnEncryptionProperties::Builder::key_metadata(
- const std::string& key_metadata) {
+ std::string key_metadata) {
DCHECK(!key_metadata.empty());
- DCHECK(key_metadata_.empty());
- key_metadata_ = key_metadata;
+ key_metadata_ = std::move(key_metadata);
return this;
}
ColumnEncryptionProperties::Builder*
ColumnEncryptionProperties::Builder::key_id(
- const std::string& key_id) {
+ std::string key_id) {
// key_id is expected to be in UTF8 encoding
::arrow::util::InitializeUTF8();
const uint8_t* data = reinterpret_cast<const uint8_t*>(key_id.c_str());
@@ -76,47 +75,47 @@ ColumnEncryptionProperties::Builder*
ColumnEncryptionProperties::Builder::key_id
}
DCHECK(!key_id.empty());
- this->key_metadata(key_id);
+ this->key_metadata(std::move(key_id));
return this;
}
FileDecryptionProperties::Builder*
FileDecryptionProperties::Builder::column_keys(
- const ColumnPathToDecryptionPropertiesMap& column_decryption_properties) {
+ ColumnPathToDecryptionPropertiesMap column_decryption_properties) {
if (column_decryption_properties.size() == 0) return this;
if (column_decryption_properties_.size() != 0)
throw ParquetException("Column properties already set");
- column_decryption_properties_ = column_decryption_properties;
+ column_decryption_properties_ = std::move(column_decryption_properties);
return this;
}
FileDecryptionProperties::Builder*
FileDecryptionProperties::Builder::footer_key(
- const std::string footer_key) {
+ std::string footer_key) {
if (footer_key.empty()) {
return this;
}
DCHECK(footer_key_.empty());
- footer_key_ = footer_key;
+ footer_key_ = std::move(footer_key);
return this;
}
FileDecryptionProperties::Builder*
FileDecryptionProperties::Builder::key_retriever(
- const std::shared_ptr<DecryptionKeyRetriever>& key_retriever) {
+ std::shared_ptr<DecryptionKeyRetriever> key_retriever) {
if (key_retriever == nullptr) return this;
DCHECK(key_retriever_ == nullptr);
- key_retriever_ = key_retriever;
+ key_retriever_ = std::move(key_retriever);
return this;
}
FileDecryptionProperties::Builder*
FileDecryptionProperties::Builder::aad_prefix(
- const std::string& aad_prefix) {
+ std::string aad_prefix) {
if (aad_prefix.empty()) {
return this;
}
DCHECK(aad_prefix_.empty());
- aad_prefix_ = aad_prefix;
+ aad_prefix_ = std::move(aad_prefix);
return this;
}
@@ -130,11 +129,11 @@ FileDecryptionProperties::Builder*
FileDecryptionProperties::Builder::aad_prefix
}
ColumnDecryptionProperties::Builder* ColumnDecryptionProperties::Builder::key(
- const std::string& key) {
+ std::string key) {
if (key.empty()) return this;
DCHECK(!key.empty());
- key_ = key;
+ key_ = std::move(key);
return this;
}
@@ -144,31 +143,31 @@ std::shared_ptr<ColumnDecryptionProperties>
ColumnDecryptionProperties::Builder:
}
FileEncryptionProperties::Builder*
FileEncryptionProperties::Builder::footer_key_metadata(
- const std::string& footer_key_metadata) {
+ std::string footer_key_metadata) {
if (footer_key_metadata.empty()) return this;
DCHECK(footer_key_metadata_.empty());
- footer_key_metadata_ = footer_key_metadata;
+ footer_key_metadata_ = std::move(footer_key_metadata);
return this;
}
FileEncryptionProperties::Builder*
FileEncryptionProperties::Builder::encrypted_columns(
- const ColumnPathToEncryptionPropertiesMap& encrypted_columns) {
+ ColumnPathToEncryptionPropertiesMap encrypted_columns) {
if (encrypted_columns.size() == 0) return this;
if (encrypted_columns_.size() != 0)
throw ParquetException("Column properties already set");
- encrypted_columns_ = encrypted_columns;
+ encrypted_columns_ = std::move(encrypted_columns);
return this;
}
FileEncryptionProperties::Builder*
FileEncryptionProperties::Builder::aad_prefix(
- const std::string& aad_prefix) {
+ std::string aad_prefix) {
if (aad_prefix.empty()) return this;
DCHECK(aad_prefix_.empty());
- aad_prefix_ = aad_prefix;
+ aad_prefix_ = std::move(aad_prefix);
store_aad_prefix_in_file_ = true;
return this;
}
@@ -182,11 +181,11 @@
FileEncryptionProperties::Builder::disable_aad_prefix_storage() {
}
ColumnEncryptionProperties::ColumnEncryptionProperties(bool encrypted,
- const std::string&
column_path,
- const std::string& key,
- const std::string&
key_metadata)
- : column_path_(column_path) {
+ std::string column_path,
+ std::string key,
+ std::string
key_metadata) {
DCHECK(!column_path.empty());
+ column_path_ = std::move(column_path);
if (!encrypted) {
DCHECK(key.empty() && key_metadata.empty());
}
@@ -201,20 +200,20 @@
ColumnEncryptionProperties::ColumnEncryptionProperties(bool encrypted,
}
encrypted_ = encrypted;
- key_metadata_ = key_metadata;
- key_ = key;
+ key_metadata_ = std::move(key_metadata);
+ key_ = std::move(key);
}
-ColumnDecryptionProperties::ColumnDecryptionProperties(const std::string&
column_path,
- const std::string& key)
- : column_path_(column_path) {
+ColumnDecryptionProperties::ColumnDecryptionProperties(std::string column_path,
+ std::string key) {
DCHECK(!column_path.empty());
+ column_path_ = std::move(column_path);
if (!key.empty()) {
DCHECK(key.length() == 16 || key.length() == 24 || key.length() == 32);
}
- key_ = key;
+ key_ = std::move(key);
}
std::string FileDecryptionProperties::column_key(const std::string&
column_path) const {
@@ -225,14 +224,14 @@ std::string FileDecryptionProperties::column_key(const
std::string& column_path)
return column_prop->key();
}
}
- return empty_string_;
+ return {};
}
FileDecryptionProperties::FileDecryptionProperties(
- const std::string& footer_key, std::shared_ptr<DecryptionKeyRetriever>
key_retriever,
- bool check_plaintext_footer_integrity, const std::string& aad_prefix,
+ std::string footer_key, std::shared_ptr<DecryptionKeyRetriever>
key_retriever,
+ bool check_plaintext_footer_integrity, std::string aad_prefix,
std::shared_ptr<AADPrefixVerifier> aad_prefix_verifier,
- const ColumnPathToDecryptionPropertiesMap& column_decryption_properties,
+ ColumnPathToDecryptionPropertiesMap column_decryption_properties,
bool plaintext_files_allowed) {
DCHECK(!footer_key.empty() || nullptr != key_retriever ||
0 != column_decryption_properties.size());
@@ -245,16 +244,16 @@ FileDecryptionProperties::FileDecryptionProperties(
DCHECK(nullptr != key_retriever);
}
aad_prefix_verifier_ = std::move(aad_prefix_verifier);
- footer_key_ = footer_key;
+ footer_key_ = std::move(footer_key);
check_plaintext_footer_integrity_ = check_plaintext_footer_integrity;
key_retriever_ = std::move(key_retriever);
- aad_prefix_ = aad_prefix;
- column_decryption_properties_ = column_decryption_properties;
+ aad_prefix_ = std::move(aad_prefix);
+ column_decryption_properties_ = std::move(column_decryption_properties);
plaintext_files_allowed_ = plaintext_files_allowed;
}
FileEncryptionProperties::Builder*
FileEncryptionProperties::Builder::footer_key_id(
- const std::string& key_id) {
+ std::string key_id) {
// key_id is expected to be in UTF8 encoding
::arrow::util::InitializeUTF8();
const uint8_t* data = reinterpret_cast<const uint8_t*>(key_id.c_str());
@@ -266,7 +265,7 @@ FileEncryptionProperties::Builder*
FileEncryptionProperties::Builder::footer_key
return this;
}
- return footer_key_metadata(key_id);
+ return footer_key_metadata(std::move(key_id));
}
std::shared_ptr<ColumnEncryptionProperties>
@@ -283,20 +282,19 @@
FileEncryptionProperties::column_encryption_properties(const std::string& column
}
FileEncryptionProperties::FileEncryptionProperties(
- ParquetCipher::type cipher, const std::string& footer_key,
- const std::string& footer_key_metadata, bool encrypted_footer,
- const std::string& aad_prefix, bool store_aad_prefix_in_file,
- const ColumnPathToEncryptionPropertiesMap& encrypted_columns)
- : footer_key_(footer_key),
- footer_key_metadata_(footer_key_metadata),
+ ParquetCipher::type cipher, std::string footer_key, std::string
footer_key_metadata,
+ bool encrypted_footer, std::string aad_prefix, bool
store_aad_prefix_in_file,
+ ColumnPathToEncryptionPropertiesMap encrypted_columns)
+ : footer_key_(std::move(footer_key)),
+ footer_key_metadata_(std::move(footer_key_metadata)),
encrypted_footer_(encrypted_footer),
- aad_prefix_(aad_prefix),
+ aad_prefix_(std::move(aad_prefix)),
store_aad_prefix_in_file_(store_aad_prefix_in_file),
- encrypted_columns_(encrypted_columns) {
- DCHECK(!footer_key.empty());
+ encrypted_columns_(std::move(encrypted_columns)) {
+ DCHECK(!footer_key_.empty());
// footer_key must be either 16, 24 or 32 bytes.
- DCHECK(footer_key.length() == 16 || footer_key.length() == 24 ||
- footer_key.length() == 32);
+ DCHECK(footer_key_.length() == 16 || footer_key_.length() == 24 ||
+ footer_key_.length() == 32);
uint8_t aad_file_unique[kAadFileUniqueLength];
encryption::RandBytes(aad_file_unique, kAadFileUniqueLength);
@@ -304,17 +302,17 @@ FileEncryptionProperties::FileEncryptionProperties(
kAadFileUniqueLength);
bool supply_aad_prefix = false;
- if (aad_prefix.empty()) {
+ if (aad_prefix_.empty()) {
file_aad_ = aad_file_unique_str;
} else {
- file_aad_ = aad_prefix + aad_file_unique_str;
+ file_aad_ = aad_prefix_ + aad_file_unique_str;
if (!store_aad_prefix_in_file_) supply_aad_prefix = true;
}
algorithm_.algorithm = cipher;
algorithm_.aad.aad_file_unique = aad_file_unique_str;
algorithm_.aad.supply_aad_prefix = supply_aad_prefix;
- if (!aad_prefix.empty() && store_aad_prefix_in_file_) {
- algorithm_.aad.aad_prefix = aad_prefix;
+ if (!aad_prefix_.empty() && store_aad_prefix_in_file_) {
+ algorithm_.aad.aad_prefix = aad_prefix_;
}
}
diff --git a/cpp/src/parquet/encryption/encryption.h
b/cpp/src/parquet/encryption/encryption.h
index 6604e32991..207687adc8 100644
--- a/cpp/src/parquet/encryption/encryption.h
+++ b/cpp/src/parquet/encryption/encryption.h
@@ -102,11 +102,11 @@ class PARQUET_EXPORT ColumnEncryptionProperties {
class PARQUET_EXPORT Builder {
public:
/// Convenience builder for encrypted columns.
- explicit Builder(const std::string& name) : Builder(name, true) {}
+ explicit Builder(std::string name) : Builder(std::move(name), true) {}
/// Convenience builder for encrypted columns.
- explicit Builder(const std::shared_ptr<schema::ColumnPath>& path)
- : Builder(path->ToDotString(), true) {}
+ explicit Builder(const schema::ColumnPath& path)
+ : Builder(path.ToDotString(), true) {}
/// Set a column-specific key.
/// If key is not set on an encrypted column, the column will
@@ -117,13 +117,13 @@ class PARQUET_EXPORT ColumnEncryptionProperties {
/// Set a key retrieval metadata.
/// use either key_metadata() or key_id(), not both
- Builder* key_metadata(const std::string& key_metadata);
+ Builder* key_metadata(std::string key_metadata);
/// A convenience function to set key metadata using a string id.
/// Set a key retrieval metadata (converted from String).
/// use either key_metadata() or key_id(), not both
/// key_id will be converted to metadata (UTF-8 array).
- Builder* key_id(const std::string& key_id);
+ Builder* key_id(std::string key_id);
std::shared_ptr<ColumnEncryptionProperties> build() {
return std::shared_ptr<ColumnEncryptionProperties>(
@@ -131,13 +131,13 @@ class PARQUET_EXPORT ColumnEncryptionProperties {
}
private:
- const std::string column_path_;
+ std::string column_path_;
bool encrypted_;
std::string key_;
std::string key_metadata_;
- Builder(const std::string path, bool encrypted)
- : column_path_(path), encrypted_(encrypted) {}
+ Builder(std::string path, bool encrypted)
+ : column_path_(std::move(path)), encrypted_(encrypted) {}
};
std::string column_path() const { return column_path_; }
@@ -146,63 +146,48 @@ class PARQUET_EXPORT ColumnEncryptionProperties {
std::string key() const { return key_; }
std::string key_metadata() const { return key_metadata_; }
- ColumnEncryptionProperties() = default;
- ColumnEncryptionProperties(const ColumnEncryptionProperties& other) =
default;
- ColumnEncryptionProperties(ColumnEncryptionProperties&& other) = default;
-
- ~ColumnEncryptionProperties() { key_.clear(); }
-
private:
- const std::string column_path_;
+ std::string column_path_;
bool encrypted_;
bool encrypted_with_footer_key_;
std::string key_;
std::string key_metadata_;
- explicit ColumnEncryptionProperties(bool encrypted, const std::string&
column_path,
- const std::string& key,
- const std::string& key_metadata);
+ explicit ColumnEncryptionProperties(bool encrypted, std::string column_path,
+ std::string key, std::string
key_metadata);
};
class PARQUET_EXPORT ColumnDecryptionProperties {
public:
class PARQUET_EXPORT Builder {
public:
- explicit Builder(const std::string& name) : column_path_(name) {}
+ explicit Builder(std::string name) : column_path_(std::move(name)) {}
- explicit Builder(const std::shared_ptr<schema::ColumnPath>& path)
- : Builder(path->ToDotString()) {}
+ explicit Builder(const schema::ColumnPath& path) :
Builder(path.ToDotString()) {}
/// Set an explicit column key. If applied on a file that contains
/// key metadata for this column the metadata will be ignored,
/// the column will be decrypted with this key.
/// key length must be either 16, 24 or 32 bytes.
- Builder* key(const std::string& key);
+ Builder* key(std::string key);
std::shared_ptr<ColumnDecryptionProperties> build();
private:
- const std::string column_path_;
+ std::string column_path_;
std::string key_;
};
- ColumnDecryptionProperties() = default;
- ColumnDecryptionProperties(const ColumnDecryptionProperties& other) =
default;
- ColumnDecryptionProperties(ColumnDecryptionProperties&& other) = default;
-
- ~ColumnDecryptionProperties() { key_.clear(); }
-
std::string column_path() const { return column_path_; }
std::string key() const { return key_; }
private:
- const std::string column_path_;
+ std::string column_path_;
std::string key_;
/// This class is only required for setting explicit column decryption keys -
/// to override key retriever (or to provide keys when key metadata and/or
/// key retriever are not available)
- explicit ColumnDecryptionProperties(const std::string& column_path,
- const std::string& key);
+ explicit ColumnDecryptionProperties(std::string column_path, std::string
key);
};
class PARQUET_EXPORT AADPrefixVerifier {
@@ -237,7 +222,7 @@ class PARQUET_EXPORT FileDecryptionProperties {
/// will be wiped out (array values set to 0).
/// Caller is responsible for wiping out the input key array.
/// param footerKey Key length must be either 16, 24 or 32 bytes.
- Builder* footer_key(const std::string footer_key);
+ Builder* footer_key(std::string footer_key);
/// Set explicit column keys (decryption properties).
/// Its also possible to set a key retriever on this property object.
@@ -246,7 +231,7 @@ class PARQUET_EXPORT FileDecryptionProperties {
/// If an explicit key is available for a footer or a column,
/// its key metadata will be ignored.
Builder* column_keys(
- const ColumnPathToDecryptionPropertiesMap&
column_decryption_properties);
+ ColumnPathToDecryptionPropertiesMap column_decryption_properties);
/// Set a key retriever callback. Its also possible to
/// set explicit footer or column keys on this file property object.
@@ -254,7 +239,7 @@ class PARQUET_EXPORT FileDecryptionProperties {
/// invocation of the retriever callback.
/// If an explicit key is available for a footer or a column,
/// its key metadata will be ignored.
- Builder* key_retriever(const std::shared_ptr<DecryptionKeyRetriever>&
key_retriever);
+ Builder* key_retriever(std::shared_ptr<DecryptionKeyRetriever>
key_retriever);
/// Skip integrity verification of plaintext footers.
/// If not called, integrity of plaintext footers will be checked in
runtime,
@@ -271,7 +256,7 @@ class PARQUET_EXPORT FileDecryptionProperties {
/// A must when a prefix is used for file encryption, but not stored in
file.
/// If AAD prefix is stored in file, it will be compared to the explicitly
/// supplied value and an exception will be thrown if they differ.
- Builder* aad_prefix(const std::string& aad_prefix);
+ Builder* aad_prefix(std::string aad_prefix);
/// Set callback for verification of AAD Prefixes stored in file.
Builder* aad_prefix_verifier(std::shared_ptr<AADPrefixVerifier>
aad_prefix_verifier);
@@ -304,8 +289,6 @@ class PARQUET_EXPORT FileDecryptionProperties {
bool plaintext_files_allowed_;
};
- ~FileDecryptionProperties() { footer_key_.clear(); }
-
std::string column_key(const std::string& column_path) const;
std::string footer_key() const { return footer_key_; }
@@ -331,7 +314,6 @@ class PARQUET_EXPORT FileDecryptionProperties {
std::string aad_prefix_;
std::shared_ptr<AADPrefixVerifier> aad_prefix_verifier_;
- const std::string empty_string_ = "";
ColumnPathToDecryptionPropertiesMap column_decryption_properties_;
std::shared_ptr<DecryptionKeyRetriever> key_retriever_;
@@ -339,11 +321,10 @@ class PARQUET_EXPORT FileDecryptionProperties {
bool plaintext_files_allowed_;
FileDecryptionProperties(
- const std::string& footer_key,
- std::shared_ptr<DecryptionKeyRetriever> key_retriever,
- bool check_plaintext_footer_integrity, const std::string& aad_prefix,
+ std::string footer_key, std::shared_ptr<DecryptionKeyRetriever>
key_retriever,
+ bool check_plaintext_footer_integrity, std::string aad_prefix,
std::shared_ptr<AADPrefixVerifier> aad_prefix_verifier,
- const ColumnPathToDecryptionPropertiesMap& column_decryption_properties,
+ ColumnPathToDecryptionPropertiesMap column_decryption_properties,
bool plaintext_files_allowed);
};
@@ -351,10 +332,10 @@ class PARQUET_EXPORT FileEncryptionProperties {
public:
class PARQUET_EXPORT Builder {
public:
- explicit Builder(const std::string& footer_key)
+ explicit Builder(std::string footer_key)
: parquet_cipher_(kDefaultEncryptionAlgorithm),
encrypted_footer_(kDefaultEncryptedFooter) {
- footer_key_ = footer_key;
+ footer_key_ = std::move(footer_key);
store_aad_prefix_in_file_ = false;
}
@@ -374,14 +355,14 @@ class PARQUET_EXPORT FileEncryptionProperties {
/// Set a key retrieval metadata (converted from String).
/// use either footer_key_metadata or footer_key_id, not both.
- Builder* footer_key_id(const std::string& key_id);
+ Builder* footer_key_id(std::string key_id);
/// Set a key retrieval metadata.
/// use either footer_key_metadata or footer_key_id, not both.
- Builder* footer_key_metadata(const std::string& footer_key_metadata);
+ Builder* footer_key_metadata(std::string footer_key_metadata);
/// Set the file AAD Prefix.
- Builder* aad_prefix(const std::string& aad_prefix);
+ Builder* aad_prefix(std::string aad_prefix);
/// Skip storing AAD Prefix in file.
/// If not called, and if AAD Prefix is set, it will be stored.
@@ -390,8 +371,7 @@ class PARQUET_EXPORT FileEncryptionProperties {
/// Set the list of encrypted columns and their properties (keys etc).
/// If not called, all columns will be encrypted with the footer key.
/// If called, the file columns not in the list will be left unencrypted.
- Builder* encrypted_columns(
- const ColumnPathToEncryptionPropertiesMap& encrypted_columns);
+ Builder* encrypted_columns(ColumnPathToEncryptionPropertiesMap
encrypted_columns);
std::shared_ptr<FileEncryptionProperties> build() {
return std::shared_ptr<FileEncryptionProperties>(new
FileEncryptionProperties(
@@ -410,8 +390,6 @@ class PARQUET_EXPORT FileEncryptionProperties {
ColumnPathToEncryptionPropertiesMap encrypted_columns_;
};
- ~FileEncryptionProperties() { footer_key_.clear(); }
-
bool encrypted_footer() const { return encrypted_footer_; }
EncryptionAlgorithm algorithm() const { return algorithm_; }
@@ -439,10 +417,10 @@ class PARQUET_EXPORT FileEncryptionProperties {
bool store_aad_prefix_in_file_;
ColumnPathToEncryptionPropertiesMap encrypted_columns_;
- FileEncryptionProperties(ParquetCipher::type cipher, const std::string&
footer_key,
- const std::string& footer_key_metadata, bool
encrypted_footer,
- const std::string& aad_prefix, bool
store_aad_prefix_in_file,
- const ColumnPathToEncryptionPropertiesMap&
encrypted_columns);
+ FileEncryptionProperties(ParquetCipher::type cipher, std::string footer_key,
+ std::string footer_key_metadata, bool
encrypted_footer,
+ std::string aad_prefix, bool
store_aad_prefix_in_file,
+ ColumnPathToEncryptionPropertiesMap
encrypted_columns);
};
} // namespace parquet
diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc
b/cpp/src/parquet/encryption/file_system_key_material_store.cc
index 2d898c1d39..cbecf2645d 100644
--- a/cpp/src/parquet/encryption/file_system_key_material_store.cc
+++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc
@@ -34,13 +34,14 @@ constexpr const char
FileSystemKeyMaterialStore::kTempFilePrefix[];
constexpr const char FileSystemKeyMaterialStore::kKeyMaterialFileSuffix[];
FileSystemKeyMaterialStore::FileSystemKeyMaterialStore(
- const std::string& key_material_file_path,
- const std::shared_ptr<::arrow::fs::FileSystem>& file_system)
- : key_material_file_path_{key_material_file_path},
file_system_{file_system} {}
+ std::string key_material_file_path,
+ std::shared_ptr<::arrow::fs::FileSystem> file_system)
+ : key_material_file_path_{std::move(key_material_file_path)},
+ file_system_{std::move(file_system)} {}
std::shared_ptr<FileSystemKeyMaterialStore> FileSystemKeyMaterialStore::Make(
- const std::string& parquet_file_path,
- const std::shared_ptr<::arrow::fs::FileSystem>& file_system, bool
use_tmp_prefix) {
+ std::string parquet_file_path, std::shared_ptr<::arrow::fs::FileSystem>
file_system,
+ bool use_tmp_prefix) {
if (parquet_file_path.empty()) {
throw ParquetException(
"The Parquet file path must be specified when using external key
material");
@@ -50,7 +51,7 @@ std::shared_ptr<FileSystemKeyMaterialStore>
FileSystemKeyMaterialStore::Make(
"A file system must be specified when using external key material");
}
- ::arrow::fs::FileInfo file_info(parquet_file_path);
+ ::arrow::fs::FileInfo file_info(std::move(parquet_file_path));
std::stringstream key_material_file_name;
if (use_tmp_prefix) {
key_material_file_name << FileSystemKeyMaterialStore::kTempFilePrefix;
@@ -61,8 +62,8 @@ std::shared_ptr<FileSystemKeyMaterialStore>
FileSystemKeyMaterialStore::Make(
std::string key_material_file_path =
::arrow::fs::internal::ConcatAbstractPath(
file_info.dir_name(), key_material_file_name.str());
- return std::make_shared<FileSystemKeyMaterialStore>(key_material_file_path,
- file_system);
+ return
std::make_shared<FileSystemKeyMaterialStore>(std::move(key_material_file_path),
+ std::move(file_system));
}
void FileSystemKeyMaterialStore::LoadKeyMaterialMap() {
diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.h
b/cpp/src/parquet/encryption/file_system_key_material_store.h
index 896a53202f..3babfdbf82 100644
--- a/cpp/src/parquet/encryption/file_system_key_material_store.h
+++ b/cpp/src/parquet/encryption/file_system_key_material_store.h
@@ -36,8 +36,8 @@ class PARQUET_EXPORT FileSystemKeyMaterialStore : public
FileKeyMaterialStore {
static constexpr const char kKeyMaterialFileSuffix[] = ".json";
FileSystemKeyMaterialStore() {}
- FileSystemKeyMaterialStore(const std::string& key_material_file_path,
- const std::shared_ptr<::arrow::fs::FileSystem>&
file_system);
+ FileSystemKeyMaterialStore(std::string key_material_file_path,
+ std::shared_ptr<::arrow::fs::FileSystem>
file_system);
/// Creates a new file system key material store for a parquet file.
/// When use_tmp_prefix is true, files are saved with an extra _TMP prefix
so they don't
@@ -45,12 +45,12 @@ class PARQUET_EXPORT FileSystemKeyMaterialStore : public
FileKeyMaterialStore {
/// so that temporary key material files can be created while using the
existing key
/// material, before moving the key material to the non-temporary location.
static std::shared_ptr<FileSystemKeyMaterialStore> Make(
- const std::string& parquet_file_path,
- const std::shared_ptr<::arrow::fs::FileSystem>& file_system, bool
use_tmp_prefix);
+ std::string parquet_file_path, std::shared_ptr<::arrow::fs::FileSystem>
file_system,
+ bool use_tmp_prefix);
/// Add key material for one encryption key.
void AddKeyMaterial(std::string key_id_in_file, std::string key_material) {
- key_material_map_.insert({key_id_in_file, key_material});
+ key_material_map_.emplace(std::move(key_id_in_file),
std::move(key_material));
}
/// Get key material
diff --git a/cpp/src/parquet/encryption/key_toolkit.cc
b/cpp/src/parquet/encryption/key_toolkit.cc
index 81e102126d..ff3053d622 100644
--- a/cpp/src/parquet/encryption/key_toolkit.cc
+++ b/cpp/src/parquet/encryption/key_toolkit.cc
@@ -34,9 +34,8 @@ std::shared_ptr<KmsClient> KeyToolkit::GetKmsClient(
if (kms_client_factory_ == nullptr) {
throw ParquetException("No KmsClientFactory is registered.");
}
- auto kms_client_per_kms_instance_cache =
- kms_client_cache_per_token().GetOrCreateInternalCache(
- kms_connection_config.key_access_token(), cache_entry_lifetime_ms);
+ auto kms_client_per_kms_instance_cache =
kms_client_cache_.GetOrCreateInternalCache(
+ kms_connection_config.key_access_token(), cache_entry_lifetime_ms);
return kms_client_per_kms_instance_cache->GetOrInsert(
kms_connection_config.kms_instance_id, [this, kms_connection_config]() {
diff --git a/cpp/src/parquet/encryption/properties_test.cc
b/cpp/src/parquet/encryption/properties_test.cc
index 895cf6c634..1ceda7ac03 100644
--- a/cpp/src/parquet/encryption/properties_test.cc
+++ b/cpp/src/parquet/encryption/properties_test.cc
@@ -90,7 +90,7 @@ TEST(TestEncryptionProperties, EncryptFooterAndTwoColumns) {
FileEncryptionProperties::Builder builder(kFooterEncryptionKey);
builder.footer_key_metadata("kf");
- builder.encrypted_columns(encrypted_columns);
+ builder.encrypted_columns(std::move(encrypted_columns));
std::shared_ptr<FileEncryptionProperties> props = builder.build();
ASSERT_EQ(true, props->encrypted_footer());
@@ -128,13 +128,13 @@ TEST(TestEncryptionProperties,
EncryptFooterAndTwoColumns) {
TEST(TestEncryptionProperties, EncryptTwoColumnsNotFooter) {
std::shared_ptr<parquet::schema::ColumnPath> column_path_1 =
parquet::schema::ColumnPath::FromDotString("column_1");
- ColumnEncryptionProperties::Builder column_builder_1(column_path_1);
+ ColumnEncryptionProperties::Builder column_builder_1(*column_path_1);
column_builder_1.key(kColumnEncryptionKey1);
column_builder_1.key_id("kc1");
std::shared_ptr<parquet::schema::ColumnPath> column_path_2 =
parquet::schema::ColumnPath::FromDotString("column_2");
- ColumnEncryptionProperties::Builder column_builder_2(column_path_2);
+ ColumnEncryptionProperties::Builder column_builder_2(*column_path_2);
column_builder_2.key(kColumnEncryptionKey2);
column_builder_2.key_id("kc2");
@@ -145,7 +145,7 @@ TEST(TestEncryptionProperties, EncryptTwoColumnsNotFooter) {
FileEncryptionProperties::Builder builder(kFooterEncryptionKey);
builder.footer_key_metadata("kf");
builder.set_plaintext_footer();
- builder.encrypted_columns(encrypted_columns);
+ builder.encrypted_columns(std::move(encrypted_columns));
std::shared_ptr<FileEncryptionProperties> props = builder.build();
ASSERT_EQ(false, props->encrypted_footer());
@@ -220,7 +220,7 @@ TEST(TestDecryptionProperties, UseKeyRetriever) {
std::static_pointer_cast<parquet::StringKeyIdRetriever>(string_kr1);
parquet::FileDecryptionProperties::Builder builder;
- builder.key_retriever(kr1);
+ builder.key_retriever(std::move(kr1));
std::shared_ptr<parquet::FileDecryptionProperties> props = builder.build();
auto out_key_retriever = props->key_retriever();
@@ -241,7 +241,7 @@ TEST(TestDecryptionProperties, SupplyAadPrefix) {
TEST(ColumnDecryptionProperties, SetKey) {
std::shared_ptr<parquet::schema::ColumnPath> column_path_1 =
parquet::schema::ColumnPath::FromDotString("column_1");
- ColumnDecryptionProperties::Builder col_builder_1(column_path_1);
+ ColumnDecryptionProperties::Builder col_builder_1(*column_path_1);
col_builder_1.key(kColumnEncryptionKey1);
auto props = col_builder_1.build();
@@ -261,7 +261,7 @@ TEST(TestDecryptionProperties,
UsingExplicitFooterAndColumnKeys) {
parquet::FileDecryptionProperties::Builder builder;
builder.footer_key(kFooterEncryptionKey);
- builder.column_keys(decryption_cols);
+ builder.column_keys(std::move(decryption_cols));
std::shared_ptr<parquet::FileDecryptionProperties> props = builder.build();
ASSERT_EQ(kFooterEncryptionKey, props->footer_key());
diff --git a/cpp/src/parquet/encryption/read_configurations_test.cc
b/cpp/src/parquet/encryption/read_configurations_test.cc
index 2612229028..61a1296e86 100644
--- a/cpp/src/parquet/encryption/read_configurations_test.cc
+++ b/cpp/src/parquet/encryption/read_configurations_test.cc
@@ -125,7 +125,7 @@ class TestDecryptionConfiguration
parquet::FileDecryptionProperties::Builder file_decryption_builder_1;
vector_of_decryption_configurations_.push_back(
- file_decryption_builder_1.key_retriever(kr1)->build());
+ file_decryption_builder_1.key_retriever(std::move(kr1))->build());
// Decryption configuration 2: Decrypt using key retriever callback that
holds the
// keys of two encrypted columns and the footer key. Supply aad_prefix.
@@ -139,7 +139,9 @@ class TestDecryptionConfiguration
parquet::FileDecryptionProperties::Builder file_decryption_builder_2;
vector_of_decryption_configurations_.push_back(
-
file_decryption_builder_2.key_retriever(kr2)->aad_prefix(kFileName_)->build());
+ file_decryption_builder_2.key_retriever(std::move(kr2))
+ ->aad_prefix(kFileName_)
+ ->build());
// Decryption configuration 3: Decrypt using explicit column and footer
keys. Supply
// aad_prefix.
@@ -159,7 +161,7 @@ class TestDecryptionConfiguration
parquet::FileDecryptionProperties::Builder file_decryption_builder_3;
vector_of_decryption_configurations_.push_back(
file_decryption_builder_3.footer_key(kFooterEncryptionKey_)
- ->column_keys(decryption_cols)
+ ->column_keys(std::move(decryption_cols))
->build());
// Decryption Configuration 4: use plaintext footer mode, read only footer
+ plaintext
diff --git a/cpp/src/parquet/encryption/write_configurations_test.cc
b/cpp/src/parquet/encryption/write_configurations_test.cc
index f27da82694..8e3d13551d 100644
--- a/cpp/src/parquet/encryption/write_configurations_test.cc
+++ b/cpp/src/parquet/encryption/write_configurations_test.cc
@@ -117,7 +117,7 @@ TEST_F(TestEncryptionConfiguration,
EncryptTwoColumnsAndTheFooter) {
kFooterEncryptionKey_);
this->EncryptFile(file_encryption_builder_2.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols2)
+ ->encrypted_columns(std::move(encryption_cols2))
->build(),
"tmp_encrypt_columns_and_footer.parquet.encrypted");
}
@@ -141,7 +141,7 @@ TEST_F(TestEncryptionConfiguration,
EncryptTwoColumnsWithPlaintextFooter) {
kFooterEncryptionKey_);
this->EncryptFile(file_encryption_builder_3.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols3)
+ ->encrypted_columns(std::move(encryption_cols3))
->set_plaintext_footer()
->build(),
"tmp_encrypt_columns_plaintext_footer.parquet.encrypted");
@@ -165,7 +165,7 @@ TEST_F(TestEncryptionConfiguration,
EncryptTwoColumnsAndFooterWithAadPrefix) {
kFooterEncryptionKey_);
this->EncryptFile(file_encryption_builder_4.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols4)
+ ->encrypted_columns(std::move(encryption_cols4))
->aad_prefix(kFileName_)
->build(),
"tmp_encrypt_columns_and_footer_aad.parquet.encrypted");
@@ -190,7 +190,7 @@ TEST_F(TestEncryptionConfiguration,
kFooterEncryptionKey_);
this->EncryptFile(
- file_encryption_builder_5.encrypted_columns(encryption_cols5)
+ file_encryption_builder_5.encrypted_columns(std::move(encryption_cols5))
->footer_key_metadata("kf")
->aad_prefix(kFileName_)
->disable_aad_prefix_storage()
@@ -217,7 +217,7 @@ TEST_F(TestEncryptionConfiguration,
EncryptTwoColumnsAndFooterUseAES_GCM_CTR) {
EXPECT_NO_THROW(
this->EncryptFile(file_encryption_builder_6.footer_key_metadata("kf")
- ->encrypted_columns(encryption_cols6)
+ ->encrypted_columns(std::move(encryption_cols6))
->algorithm(parquet::ParquetCipher::AES_GCM_CTR_V1)
->build(),
"tmp_encrypt_columns_and_footer_ctr.parquet.encrypted"));
