kou commented on code in PR #49660:
URL: https://github.com/apache/arrow/pull/49660#discussion_r3070144656
##########
cpp/src/parquet/encryption/key_toolkit_internal.cc:
##########
@@ -52,7 +52,12 @@ std::string EncryptKeyLocally(const SecureString& key_bytes,
SecureString DecryptKeyLocally(const std::string& encoded_encrypted_key,
const SecureString& master_key, const
std::string& aad) {
- std::string encrypted_key =
::arrow::util::base64_decode(encoded_encrypted_key);
+ auto result = ::arrow::util::base64_decode(encoded_encrypted_key);
+ if (!result.ok()) {
+ throw ParquetException(result.status().message());
+ }
Review Comment:
Could you use `PARQUET_ASSIGN_OR_THROW()`?
##########
cpp/src/arrow/vendored/base64.cpp:
##########
@@ -93,38 +89,65 @@ std::string base64_encode(std::string_view
string_to_encode) {
return base64_encode(bytes_to_encode, in_len);
}
-std::string base64_decode(std::string_view encoded_string) {
+Result<std::string> base64_decode(std::string_view encoded_string) {
size_t in_len = encoded_string.size();
int i = 0;
- int j = 0;
- int in_ = 0;
+ std::string_view::size_type in_ = 0;
+ int padding_count = 0;
+ int block_padding = 0;
+ bool padding_started = false;
unsigned char char_array_4[4], char_array_3[3];
std::string ret;
- while (in_len-- && ( encoded_string[in_] != '=') &&
is_base64(encoded_string[in_])) {
- char_array_4[i++] = encoded_string[in_]; in_++;
- if (i ==4) {
- for (i = 0; i <4; i++)
- char_array_4[i] = base64_chars.find(char_array_4[i]) & 0xff;
+ if (encoded_string.size() % 4 != 0) {
+ return Status::Invalid("Invalid base64 input");
Review Comment:
Can we use different error message for each invalid case?
##########
cpp/src/parquet/encryption/file_key_unwrapper.cc:
##########
@@ -122,7 +122,12 @@ KeyWithMasterId
FileKeyUnwrapper::GetDataEncryptionKey(const KeyMaterial& key_ma
});
// Decrypt the data key
- std::string aad = ::arrow::util::base64_decode(encoded_kek_id);
+ auto result = ::arrow::util::base64_decode(encoded_kek_id);
+ if (!result.ok()) {
+ throw ParquetException(result.status().message());
+ }
Review Comment:
Could you use `PARQUET_ASSIGN_OR_THROW()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
