This is an automated email from the ASF dual-hosted git repository.
pitrou 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 f2a446b2478 GH-50971: [C++][Parquet] Fix usage of disparate length
types for metadata reading (#50972)
f2a446b2478 is described below
commit f2a446b24784d3fbf108055e9f903c77a1bb1ae2
Author: Antoine Pitrou <[email protected]>
AuthorDate: Tue Aug 25 09:33:05 2026 +0200
GH-50971: [C++][Parquet] Fix usage of disparate length types for metadata
reading (#50972)
### Rationale for this change
The usage of disparate integer types (`int64_t`, `uint32_t`) makes our
checks and computations fragile, especially with C++ adding its own integer
promotion rules across arithmetic operations.
We have had at least one report (courtesy of Ada Logics and Claude) where a
carefully crafted Parquet file can read from an invalid pointer due to
arithmetic overflow in the 32-bit domain.
### What changes are included in this PR?
Use `int64_t` throughout most internal APIs and code paths when reading
Parquet metadata. Other types such as `uint32_t` should only be used where
necessary when interacting with third-party libraries such as Thrift C++.
### Are these changes tested?
By existing tests, and manually using said hand-crafted Parquet file.
### Are there any user-facing changes?
Some APIs taking a `uint32_t*` inout-parameter are deprecated, alternatives
taking a `int64_t` value are available.
* GitHub Issue: #50971
Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/parquet/bloom_filter.cc | 12 ++--
cpp/src/parquet/column_reader.cc | 12 ++--
cpp/src/parquet/file_reader.cc | 67 ++++++++++---------
cpp/src/parquet/metadata.cc | 106 +++++++++++++++++--------------
cpp/src/parquet/metadata.h | 28 ++++++--
cpp/src/parquet/metadata_test.cc | 17 ++---
cpp/src/parquet/page_index.cc | 8 +--
cpp/src/parquet/page_index.h | 4 +-
cpp/src/parquet/page_index_test.cc | 3 +-
cpp/src/parquet/thrift_internal.h | 88 ++++++++++++++-----------
cpp/tools/parquet/parquet_dump_footer.cc | 4 +-
11 files changed, 191 insertions(+), 158 deletions(-)
diff --git a/cpp/src/parquet/bloom_filter.cc b/cpp/src/parquet/bloom_filter.cc
index c6f6bdfe55f..83e91cdcb3a 100644
--- a/cpp/src/parquet/bloom_filter.cc
+++ b/cpp/src/parquet/bloom_filter.cc
@@ -206,10 +206,10 @@ BlockSplitBloomFilter DeserializeEncryptedFromStream(
// Bloom filter header and bitset are separate encrypted modules with
different AADs.
UpdateDecryptor(decryptor, row_group_ordinal, column_ordinal,
encryption::kBloomFilterHeader);
- auto header_cipher_len = static_cast<uint32_t>(header_cipher_total_len);
+ int64_t header_cipher_len;
try {
- deserializer.DeserializeMessage(header_cipher_buf->data(),
&header_cipher_len,
- &header, decryptor);
+ header_cipher_len = deserializer.DeserializeMessage(
+ header_cipher_buf->data(), header_cipher_total_len, &header,
decryptor);
} catch (std::exception& e) {
std::stringstream ss;
ss << "Deserializing bloom filter header failed.\n" << e.what();
@@ -304,10 +304,10 @@ BlockSplitBloomFilter BlockSplitBloomFilter::Deserialize(
// Read and deserialize bloom filter header
PARQUET_ASSIGN_OR_THROW(auto header_buf,
input->Read(bloom_filter_header_read_size));
- // This gets used, then set by DeserializeThriftMsg
- uint32_t header_size = static_cast<uint32_t>(header_buf->size());
+ int64_t header_size;
try {
- deserializer.DeserializeMessage(header_buf->data(), &header_size, &header);
+ header_size =
+ deserializer.DeserializeMessage(header_buf->data(),
header_buf->size(), &header);
DCHECK_LE(header_size, header_buf->size());
} catch (std::exception& e) {
std::stringstream ss;
diff --git a/cpp/src/parquet/column_reader.cc b/cpp/src/parquet/column_reader.cc
index 162a72bd157..56c65fd233b 100644
--- a/cpp/src/parquet/column_reader.cc
+++ b/cpp/src/parquet/column_reader.cc
@@ -434,8 +434,8 @@ std::shared_ptr<Page> SerializedPageReader::NextPage() {
// Loop here because there may be unhandled page types that we skip until
// finding a page that we do know what to do with
while (seen_num_values_ < total_num_values_) {
- uint32_t header_size = 0;
- uint32_t allowed_page_size = kDefaultPageHeaderSize;
+ int64_t header_size = 0;
+ int64_t allowed_page_size = kDefaultPageHeaderSize;
// Page headers can be very large because of page statistics
// We try to deserialize a larger buffer progressively
@@ -444,8 +444,6 @@ std::shared_ptr<Page> SerializedPageReader::NextPage() {
PARQUET_ASSIGN_OR_THROW(auto view, stream_->Peek(allowed_page_size));
if (view.size() == 0) return nullptr;
- // This gets used, then set by DeserializeThriftMsg
- header_size = static_cast<uint32_t>(view.size());
try {
if (meta_decryptor_ != nullptr) {
UpdateDecryption(meta_decryptor_.get(),
encryption::kDictionaryPageHeader,
@@ -453,9 +451,9 @@ std::shared_ptr<Page> SerializedPageReader::NextPage() {
}
// Reset current page header to avoid unclearing the __isset flag.
current_page_header_ = format::PageHeader();
- deserializer.DeserializeMessage(reinterpret_cast<const
uint8_t*>(view.data()),
- &header_size, ¤t_page_header_,
- meta_decryptor_.get());
+ header_size = deserializer.DeserializeMessage(
+ reinterpret_cast<const uint8_t*>(view.data()), view.size(),
+ ¤t_page_header_, meta_decryptor_.get());
break;
} catch (std::exception& e) {
// Failed to deserialize. Double the allowed page header size and try
again
diff --git a/cpp/src/parquet/file_reader.cc b/cpp/src/parquet/file_reader.cc
index 2f46a5e296f..9214f7b4abb 100644
--- a/cpp/src/parquet/file_reader.cc
+++ b/cpp/src/parquet/file_reader.cc
@@ -88,7 +88,7 @@ bool IsColumnChunkFullyDictionaryEncoded(const
ColumnChunkMetaData& col) {
}
} // namespace
-static constexpr uint32_t kFooterSize = 8;
+static constexpr int64_t kFooterSize = 8;
// For PARQUET-816
static constexpr int64_t kMaxDictHeaderSize = 100;
@@ -441,7 +441,7 @@ class SerializedFile : public ParquetFileReader::Contents {
PARQUET_ASSIGN_OR_THROW(
auto footer_buffer,
source_->ReadAt(source_size_ - footer_read_size, footer_read_size));
- uint32_t metadata_len = ParseFooterLength(footer_buffer, footer_read_size);
+ int64_t metadata_len = ParseFooterLength(footer_buffer, footer_read_size);
int64_t metadata_start = source_size_ - kFooterSize - metadata_len;
std::shared_ptr<::arrow::Buffer> metadata_buffer;
@@ -460,12 +460,10 @@ class SerializedFile : public ParquetFileReader::Contents
{
std::shared_ptr<InternalFileDecryptor> file_decryptor;
if (is_encrypted_footer) {
// Encrypted file with Encrypted footer.
- const std::pair<int64_t, uint32_t> read_size =
+ std::tie(metadata_start, metadata_len) =
ParseMetaDataOfEncryptedFileWithEncryptedFooter(metadata_buffer,
metadata_len,
&file_decryptor);
// Read the actual footer
- metadata_start = read_size.first;
- metadata_len = read_size.second;
PARQUET_ASSIGN_OR_THROW(
metadata_buffer,
source_->ReadAt(metadata_start, metadata_len,
/*allow_short_read=*/false));
@@ -490,8 +488,8 @@ class SerializedFile : public ParquetFileReader::Contents {
}
// Validate the magic bytes and get the length of the full footer.
- uint32_t ParseFooterLength(const std::shared_ptr<::arrow::Buffer>&
footer_buffer,
- const int64_t footer_read_size) {
+ int64_t ParseFooterLength(const std::shared_ptr<::arrow::Buffer>&
footer_buffer,
+ const int64_t footer_read_size) {
// Check if all bytes are read. Check if last 4 bytes read have the magic
bits
if (footer_buffer->size() != footer_read_size ||
(memcmp(footer_buffer->data() + footer_read_size - 4, kParquetMagic,
4) != 0 &&
@@ -501,7 +499,7 @@ class SerializedFile : public ParquetFileReader::Contents {
"is not a parquet file.");
}
// Both encrypted/unencrypted footers have the same footer length check.
- uint32_t metadata_len =
+ int64_t metadata_len =
::arrow::bit_util::FromLittleEndian(::arrow::util::SafeLoadAs<uint32_t>(
reinterpret_cast<const uint8_t*>(footer_buffer->data()) +
footer_read_size -
kFooterSize));
@@ -523,7 +521,7 @@ class SerializedFile : public ParquetFileReader::Contents {
return source_->ReadAsync(source_size_ - footer_read_size,
footer_read_size)
.Then([this, footer_read_size](
const std::shared_ptr<::arrow::Buffer>& footer_buffer) ->
Future<> {
- uint32_t metadata_len;
+ int64_t metadata_len;
BEGIN_PARQUET_CATCH_EXCEPTIONS
metadata_len = ParseFooterLength(footer_buffer, footer_read_size);
END_PARQUET_CATCH_EXCEPTIONS
@@ -552,21 +550,20 @@ class SerializedFile : public ParquetFileReader::Contents
{
Future<> ParseMaybeEncryptedMetaDataAsync(
std::shared_ptr<::arrow::Buffer> footer_buffer,
std::shared_ptr<::arrow::Buffer> metadata_buffer, int64_t
footer_read_size,
- uint32_t metadata_len) {
+ int64_t metadata_len) {
// Parse the footer depending on encryption type
const bool is_encrypted_footer =
memcmp(footer_buffer->data() + footer_read_size - 4, kParquetEMagic,
4) == 0;
std::shared_ptr<InternalFileDecryptor> file_decryptor;
if (is_encrypted_footer) {
// Encrypted file with Encrypted footer.
- std::pair<int64_t, uint32_t> read_size;
+ int64_t metadata_start;
BEGIN_PARQUET_CATCH_EXCEPTIONS
- read_size = ParseMetaDataOfEncryptedFileWithEncryptedFooter(
- metadata_buffer, metadata_len, &file_decryptor);
+ std::tie(metadata_start, metadata_len) =
+ ParseMetaDataOfEncryptedFileWithEncryptedFooter(metadata_buffer,
metadata_len,
+ &file_decryptor);
END_PARQUET_CATCH_EXCEPTIONS
// Read the actual footer
- int64_t metadata_start = read_size.first;
- metadata_len = read_size.second;
return source_->ReadAsync(metadata_start, metadata_len,
/*allow_short_read=*/false)
.Then([this, metadata_len, is_encrypted_footer,
file_decryptor = std::move(file_decryptor)](
@@ -588,9 +585,9 @@ class SerializedFile : public ParquetFileReader::Contents {
// Continuation
void ParseMetaDataFinal(std::shared_ptr<::arrow::Buffer> metadata_buffer,
- uint32_t metadata_len, const bool
is_encrypted_footer,
+ int64_t metadata_len, const bool is_encrypted_footer,
std::shared_ptr<InternalFileDecryptor>
file_decryptor) {
- const uint32_t read_metadata_len = ParseUnencryptedFileMetadata(
+ const int64_t read_metadata_len = ParseUnencryptedFileMetadata(
metadata_buffer, metadata_len, std::move(file_decryptor));
auto file_decryption_properties = properties_.file_decryption_properties();
if (is_encrypted_footer) {
@@ -622,8 +619,8 @@ class SerializedFile : public ParquetFileReader::Contents {
std::unordered_map<int, std::shared_ptr<Buffer>> prebuffered_column_chunks_;
// \return The true length of the metadata in bytes
- uint32_t ParseUnencryptedFileMetadata(
- const std::shared_ptr<Buffer>& footer_buffer, const uint32_t
metadata_len,
+ int64_t ParseUnencryptedFileMetadata(
+ const std::shared_ptr<Buffer>& footer_buffer, const int64_t metadata_len,
std::shared_ptr<InternalFileDecryptor> file_decryptor);
std::string HandleAadPrefix(
@@ -632,35 +629,35 @@ class SerializedFile : public ParquetFileReader::Contents
{
void ParseMetaDataOfEncryptedFileWithPlaintextFooter(
const std::shared_ptr<FileDecryptionProperties>&
file_decryption_properties,
- const std::shared_ptr<Buffer>& metadata_buffer, uint32_t metadata_len,
- uint32_t read_metadata_len);
+ const std::shared_ptr<Buffer>& metadata_buffer, int64_t metadata_len,
+ int64_t read_metadata_len);
// \return The position and size of the actual footer
- std::pair<int64_t, uint32_t> ParseMetaDataOfEncryptedFileWithEncryptedFooter(
- const std::shared_ptr<Buffer>& crypto_metadata_buffer, uint32_t
footer_len,
+ std::pair<int64_t, int64_t> ParseMetaDataOfEncryptedFileWithEncryptedFooter(
+ const std::shared_ptr<Buffer>& crypto_metadata_buffer, int64_t
footer_len,
std::shared_ptr<InternalFileDecryptor>* file_decryptor);
};
-uint32_t SerializedFile::ParseUnencryptedFileMetadata(
- const std::shared_ptr<Buffer>& metadata_buffer, const uint32_t
metadata_len,
+int64_t SerializedFile::ParseUnencryptedFileMetadata(
+ const std::shared_ptr<Buffer>& metadata_buffer, int64_t metadata_len,
std::shared_ptr<InternalFileDecryptor> file_decryptor) {
if (metadata_buffer->size() != metadata_len) {
throw ParquetException("Failed reading metadata buffer (requested " +
std::to_string(metadata_len) + " bytes but got " +
std::to_string(metadata_buffer->size()) + "
bytes)");
}
- uint32_t read_metadata_len = metadata_len;
+ int64_t read_metadata_len = metadata_len;
// The encrypted read path falls through to here, so pass in the decryptor
- file_metadata_ = FileMetaData::Make(metadata_buffer->data(),
&read_metadata_len,
+ file_metadata_ = FileMetaData::Make(metadata_buffer->data(),
read_metadata_len,
properties_, std::move(file_decryptor));
- return read_metadata_len;
+ return file_metadata_->size();
}
-std::pair<int64_t, uint32_t>
+std::pair<int64_t, int64_t>
SerializedFile::ParseMetaDataOfEncryptedFileWithEncryptedFooter(
const std::shared_ptr<::arrow::Buffer>& crypto_metadata_buffer,
// both metadata & crypto metadata length
- const uint32_t footer_len, std::shared_ptr<InternalFileDecryptor>*
file_decryptor) {
+ const int64_t footer_len, std::shared_ptr<InternalFileDecryptor>*
file_decryptor) {
// encryption with encrypted footer
// Check if the footer_buffer contains the entire metadata
if (crypto_metadata_buffer->size() != footer_len) {
@@ -673,9 +670,9 @@
SerializedFile::ParseMetaDataOfEncryptedFileWithEncryptedFooter(
throw ParquetException(
"Could not read encrypted metadata, no decryption found in reader's
properties");
}
- uint32_t crypto_metadata_len = footer_len;
std::shared_ptr<FileCryptoMetaData> file_crypto_metadata =
- FileCryptoMetaData::Make(crypto_metadata_buffer->data(),
&crypto_metadata_len);
+ FileCryptoMetaData::Make(crypto_metadata_buffer->data(), footer_len);
+ int64_t crypto_metadata_len = file_crypto_metadata->size();
// Handle AAD prefix
EncryptionAlgorithm algo = file_crypto_metadata->encryption_algorithm();
std::string file_aad = HandleAadPrefix(file_decryption_properties, algo);
@@ -684,14 +681,14 @@
SerializedFile::ParseMetaDataOfEncryptedFileWithEncryptedFooter(
file_crypto_metadata->key_metadata(), properties_.memory_pool());
int64_t metadata_offset = source_size_ - kFooterSize - footer_len +
crypto_metadata_len;
- uint32_t metadata_len = footer_len - crypto_metadata_len;
+ int64_t metadata_len = footer_len - crypto_metadata_len;
return std::make_pair(metadata_offset, metadata_len);
}
void SerializedFile::ParseMetaDataOfEncryptedFileWithPlaintextFooter(
const std::shared_ptr<FileDecryptionProperties>&
file_decryption_properties,
- const std::shared_ptr<Buffer>& metadata_buffer, uint32_t metadata_len,
- uint32_t read_metadata_len) {
+ const std::shared_ptr<Buffer>& metadata_buffer, int64_t metadata_len,
+ int64_t read_metadata_len) {
// Providing decryption properties in plaintext footer mode is not
mandatory, for
// example when reading by legacy reader.
if (file_decryption_properties != nullptr) {
diff --git a/cpp/src/parquet/metadata.cc b/cpp/src/parquet/metadata.cc
index 98f60df63dd..183fcc82b1d 100644
--- a/cpp/src/parquet/metadata.cc
+++ b/cpp/src/parquet/metadata.cc
@@ -285,11 +285,11 @@ class ColumnChunkMetaData::ColumnChunkMetaDataImpl {
column_ordinal, /*page_ordinal=*/static_cast<int16_t>(-1));
auto decryptor = file_decryptor->GetColumnMetaDecryptor(
path->ToDotString(), key_metadata, aad_column_metadata);
- auto len =
static_cast<uint32_t>(column->encrypted_column_metadata.size());
ThriftDeserializer deserializer(properties_);
deserializer.DeserializeMessage(
reinterpret_cast<const
uint8_t*>(column->encrypted_column_metadata.c_str()),
- &len, &decrypted_metadata_, decryptor.get());
+ column->encrypted_column_metadata.size(), &decrypted_metadata_,
+ decryptor.get());
column_metadata_ = &decrypted_metadata_;
} else {
throw ParquetException(
@@ -779,7 +779,7 @@ class FileMetaData::FileMetaDataImpl {
FileMetaDataImpl() = default;
explicit FileMetaDataImpl(
- const void* metadata, uint32_t* metadata_len, ReaderProperties
properties,
+ const void* metadata, int64_t metadata_len, ReaderProperties properties,
std::shared_ptr<InternalFileDecryptor> file_decryptor = nullptr)
: properties_(std::move(properties)),
file_decryptor_(std::move(file_decryptor)) {
metadata_ = std::make_unique<format::FileMetaData>();
@@ -788,10 +788,9 @@ class FileMetaData::FileMetaDataImpl {
file_decryptor_ != nullptr ? file_decryptor_->GetFooterDecryptor() :
nullptr;
ThriftDeserializer deserializer(properties_);
- deserializer.DeserializeMessage(reinterpret_cast<const uint8_t*>(metadata),
- metadata_len, metadata_.get(),
- footer_decryptor.get());
- metadata_len_ = *metadata_len;
+ metadata_len_ = deserializer.DeserializeMessage(
+ reinterpret_cast<const uint8_t*>(metadata), metadata_len,
metadata_.get(),
+ footer_decryptor.get());
if (metadata_->__isset.created_by) {
writer_version_ = ApplicationVersion(metadata_->created_by);
@@ -810,11 +809,8 @@ class FileMetaData::FileMetaDataImpl {
throw ParquetException("Decryption not set properly. cannot verify
signature");
}
// serialize the footer
- uint8_t* serialized_data;
- uint32_t serialized_len = metadata_len_;
ThriftSerializer serializer;
- serializer.SerializeToBuffer(metadata_.get(), &serialized_len,
&serialized_data);
- std::span<const uint8_t> serialized_data_span(serialized_data,
serialized_len);
+ auto serialized_data_span = serializer.SerializeToBuffer(metadata_.get());
// encrypt with nonce
std::span<const uint8_t> nonce(reinterpret_cast<const uint8_t*>(signature),
@@ -829,7 +825,8 @@ class FileMetaData::FileMetaDataImpl {
true, false
/*write_length*/);
std::shared_ptr<Buffer> encrypted_buffer = AllocateBuffer(
- file_decryptor_->pool(),
aes_encryptor->CiphertextLength(serialized_len));
+ file_decryptor_->pool(), aes_encryptor->CiphertextLength(
+
static_cast<int64_t>(serialized_data_span.size())));
int32_t encrypted_len = aes_encryptor->SignedFooterEncrypt(
serialized_data_span, key.as_span(), str2span(aad), nonce,
encrypted_buffer->mutable_span_as<uint8_t>());
@@ -838,7 +835,7 @@ class FileMetaData::FileMetaDataImpl {
tag, encryption::kGcmTagLength);
}
- inline uint32_t size() const { return metadata_len_; }
+ inline int64_t size() const { return metadata_len_; }
inline int num_columns() const { return schema_.num_columns(); }
inline int64_t num_rows() const { return metadata_->num_rows; }
inline int num_row_groups() const {
@@ -868,17 +865,16 @@ class FileMetaData::FileMetaDataImpl {
// Only in encrypted files with plaintext footers the
// encryption_algorithm is set in footer
if (is_encryption_algorithm_set()) {
- uint8_t* serialized_data;
- uint32_t serialized_len;
- serializer.SerializeToBuffer(metadata_.get(), &serialized_len,
&serialized_data);
- std::span<const uint8_t> serialized_data_span(serialized_data,
serialized_len);
+ const auto serialized_data_span =
serializer.SerializeToBuffer(metadata_.get());
+ const auto serialized_data_len =
static_cast<int64_t>(serialized_data_span.size());
// encrypt the footer key
- std::vector<uint8_t>
encrypted_data(encryptor->CiphertextLength(serialized_len));
+ std::vector<uint8_t> encrypted_data(
+ encryptor->CiphertextLength(serialized_data_len));
int32_t encrypted_len = encryptor->Encrypt(serialized_data_span,
encrypted_data);
// write unencrypted footer
- PARQUET_THROW_NOT_OK(dst->Write(serialized_data, serialized_len));
+ PARQUET_THROW_NOT_OK(dst->Write(serialized_data_span.data(),
serialized_data_len));
// Write signature (nonce and tag)
PARQUET_THROW_NOT_OK(
dst->Write(encrypted_data.data() + 4, encryption::kNonceLength));
@@ -1000,9 +996,7 @@ class FileMetaData::FileMetaDataImpl {
return ss.str();
} else {
ThriftSerializer serializer;
- std::string out;
- serializer.SerializeToString(&md, &out);
- return out;
+ return std::string(serializer.SerializeToString(&md));
}
}
@@ -1016,7 +1010,7 @@ class FileMetaData::FileMetaDataImpl {
private:
friend FileMetaDataBuilder;
- uint32_t metadata_len_ = 0;
+ int64_t metadata_len_ = 0;
std::unique_ptr<format::FileMetaData> metadata_;
SchemaDescriptor schema_;
ApplicationVersion writer_version_;
@@ -1057,14 +1051,24 @@ class FileMetaData::FileMetaDataImpl {
};
std::shared_ptr<FileMetaData> FileMetaData::Make(
- const void* metadata, uint32_t* metadata_len, const ReaderProperties&
properties,
+ const void* metadata, int64_t metadata_len, const ReaderProperties&
properties,
std::shared_ptr<InternalFileDecryptor> file_decryptor) {
// This FileMetaData ctor is private, not compatible with std::make_shared
return std::shared_ptr<FileMetaData>(
new FileMetaData(metadata, metadata_len, properties,
std::move(file_decryptor)));
}
-FileMetaData::FileMetaData(const void* metadata, uint32_t* metadata_len,
+// (deprecated)
+std::shared_ptr<FileMetaData> FileMetaData::Make(
+ const void* metadata, uint32_t* metadata_len, const ReaderProperties&
properties,
+ std::shared_ptr<InternalFileDecryptor> file_decryptor) {
+ auto ptr =
+ FileMetaData::Make(metadata, *metadata_len, properties,
std::move(file_decryptor));
+ *metadata_len = static_cast<uint32_t>(ptr->size());
+ return ptr;
+}
+
+FileMetaData::FileMetaData(const void* metadata, int64_t metadata_len,
const ReaderProperties& properties,
std::shared_ptr<InternalFileDecryptor>
file_decryptor)
: impl_(new FileMetaDataImpl(metadata, metadata_len, properties,
@@ -1086,7 +1090,7 @@ bool FileMetaData::VerifySignature(const void* signature)
{
return impl_->VerifySignature(signature);
}
-uint32_t FileMetaData::size() const { return impl_->size(); }
+int64_t FileMetaData::size() const { return impl_->size(); }
int FileMetaData::num_columns() const { return impl_->num_columns(); }
@@ -1212,11 +1216,10 @@ class FileCryptoMetaData::FileCryptoMetaDataImpl {
public:
FileCryptoMetaDataImpl() = default;
- explicit FileCryptoMetaDataImpl(const uint8_t* metadata, uint32_t*
metadata_len,
+ explicit FileCryptoMetaDataImpl(const uint8_t* metadata, int64_t
metadata_len,
const ReaderProperties& properties) {
ThriftDeserializer deserializer(properties);
- deserializer.DeserializeMessage(metadata, metadata_len, &metadata_);
- metadata_len_ = *metadata_len;
+ metadata_len_ = deserializer.DeserializeMessage(metadata, metadata_len,
&metadata_);
}
EncryptionAlgorithm encryption_algorithm() const {
@@ -1230,10 +1233,12 @@ class FileCryptoMetaData::FileCryptoMetaDataImpl {
serializer.Serialize(&metadata_, dst);
}
+ int64_t size() const { return metadata_len_; }
+
private:
friend FileMetaDataBuilder;
format::FileCryptoMetaData metadata_;
- uint32_t metadata_len_;
+ int64_t metadata_len_;
};
EncryptionAlgorithm FileCryptoMetaData::encryption_algorithm() const {
@@ -1244,15 +1249,26 @@ const std::string& FileCryptoMetaData::key_metadata()
const {
return impl_->key_metadata();
}
+int64_t FileCryptoMetaData::size() const { return impl_->size(); }
+
std::shared_ptr<FileCryptoMetaData> FileCryptoMetaData::Make(
- const uint8_t* serialized_metadata, uint32_t* metadata_len,
+ const uint8_t* serialized_metadata, int64_t metadata_len,
const ReaderProperties& properties) {
return std::shared_ptr<FileCryptoMetaData>(
new FileCryptoMetaData(serialized_metadata, metadata_len, properties));
}
+// (deprecated)
+std::shared_ptr<FileCryptoMetaData> FileCryptoMetaData::Make(
+ const uint8_t* serialized_metadata, uint32_t* metadata_len,
+ const ReaderProperties& properties) {
+ auto ptr = FileCryptoMetaData::Make(serialized_metadata, *metadata_len,
properties);
+ *metadata_len = static_cast<uint32_t>(ptr->size());
+ return ptr;
+}
+
FileCryptoMetaData::FileCryptoMetaData(const uint8_t* serialized_metadata,
- uint32_t* metadata_len,
+ int64_t metadata_len,
const ReaderProperties& properties)
: impl_(new FileCryptoMetaDataImpl(serialized_metadata, metadata_len,
properties)) {}
@@ -1771,20 +1787,18 @@ class
ColumnChunkMetaDataBuilder::ColumnChunkMetaDataBuilderImpl {
// Serialize and encrypt ColumnMetadata separately
// Thrift-serialize the ColumnMetaData structure,
// encrypt it with the column key, and write to
encrypted_column_metadata
- uint8_t* serialized_data;
- uint32_t serialized_len;
-
- serializer.SerializeToBuffer(&column_chunk_->meta_data,
&serialized_len,
- &serialized_data);
- std::span<const uint8_t> serialized_data_span(serialized_data,
serialized_len);
-
- std::vector<uint8_t>
encrypted_data(encryptor->CiphertextLength(serialized_len));
- int32_t encrypted_len = encryptor->Encrypt(serialized_data_span,
encrypted_data);
-
- const char* temp =
- const_cast<const
char*>(reinterpret_cast<char*>(encrypted_data.data()));
- std::string encrypted_column_metadata(temp, encrypted_len);
-
column_chunk_->__set_encrypted_column_metadata(encrypted_column_metadata);
+ auto serialized_data_span =
+ serializer.SerializeToBuffer(&column_chunk_->meta_data);
+
+ std::string encrypted_metadata;
+ encrypted_metadata.resize(encryptor->CiphertextLength(
+ static_cast<int64_t>(serialized_data_span.size())));
+ int32_t encrypted_len = encryptor->Encrypt(
+ serialized_data_span,
+ std::span(reinterpret_cast<uint8_t*>(encrypted_metadata.data()),
+ encrypted_metadata.size()));
+ encrypted_metadata.resize(encrypted_len);
+
column_chunk_->__set_encrypted_column_metadata(std::move(encrypted_metadata));
if (encrypted_footer) {
column_chunk_->__isset.meta_data = false;
diff --git a/cpp/src/parquet/metadata.h b/cpp/src/parquet/metadata.h
index 5db4905beec..bab6bba1587 100644
--- a/cpp/src/parquet/metadata.h
+++ b/cpp/src/parquet/metadata.h
@@ -241,12 +241,21 @@ class FileMetaDataBuilder;
/// \brief FileMetaData is a proxy around format::FileMetaData.
class PARQUET_EXPORT FileMetaData {
public:
- /// \brief Create a FileMetaData from a serialized thrift message.
+ PARQUET_DEPRECATED("Deprecated in 26.0.0. Please pass metadata length as a
int64_t.")
static std::shared_ptr<FileMetaData> Make(
const void* serialized_metadata, uint32_t* inout_metadata_len,
const ReaderProperties& properties = default_reader_properties(),
std::shared_ptr<InternalFileDecryptor> file_decryptor = NULLPTR);
+ /// \brief Create a FileMetaData from a serialized Thrift message.
+ ///
+ /// The actual size in bytes of the metadata buffer can be obtained using
+ /// the `size()` method.
+ static std::shared_ptr<FileMetaData> Make(
+ const void* serialized_metadata, int64_t metadata_len,
+ const ReaderProperties& properties = default_reader_properties(),
+ std::shared_ptr<InternalFileDecryptor> file_decryptor = NULLPTR);
+
~FileMetaData();
bool Equals(const FileMetaData& other) const;
@@ -311,7 +320,7 @@ class PARQUET_EXPORT FileMetaData {
const ApplicationVersion& writer_version() const;
/// \brief Size of the original thrift encoded metadata footer.
- uint32_t size() const;
+ int64_t size() const;
/// \brief Indicate if all of the FileMetaData's RowGroups can be
decompressed.
///
@@ -377,7 +386,7 @@ class PARQUET_EXPORT FileMetaData {
friend class SerializedFile;
friend class SerializedRowGroup;
- explicit FileMetaData(const void* serialized_metadata, uint32_t*
metadata_len,
+ explicit FileMetaData(const void* serialized_metadata, int64_t metadata_len,
const ReaderProperties& properties,
std::shared_ptr<InternalFileDecryptor> file_decryptor
= NULLPTR);
@@ -397,20 +406,29 @@ class PARQUET_EXPORT FileMetaData {
class PARQUET_EXPORT FileCryptoMetaData {
public:
- // API convenience to get a MetaData accessor
+ PARQUET_DEPRECATED("Deprecated in 26.0.0. Please pass metadata length as a
int64_t.")
static std::shared_ptr<FileCryptoMetaData> Make(
const uint8_t* serialized_metadata, uint32_t* metadata_len,
const ReaderProperties& properties = default_reader_properties());
+
+ /// \brief Create a FileMetaData from a serialized Thrift message.
+ ///
+ /// The actual size in bytes of the metadata buffer can be obtained using
+ /// the `size()` method.
+ static std::shared_ptr<FileCryptoMetaData> Make(
+ const uint8_t* serialized_metadata, int64_t metadata_len,
+ const ReaderProperties& properties = default_reader_properties());
~FileCryptoMetaData();
EncryptionAlgorithm encryption_algorithm() const;
const std::string& key_metadata() const;
+ int64_t size() const;
void WriteTo(::arrow::io::OutputStream* dst) const;
private:
friend FileMetaDataBuilder;
- FileCryptoMetaData(const uint8_t* serialized_metadata, uint32_t*
metadata_len,
+ FileCryptoMetaData(const uint8_t* serialized_metadata, int64_t metadata_len,
const ReaderProperties& properties);
// PIMPL Idiom
diff --git a/cpp/src/parquet/metadata_test.cc b/cpp/src/parquet/metadata_test.cc
index ac45be1fac3..e6ff1f929d4 100644
--- a/cpp/src/parquet/metadata_test.cc
+++ b/cpp/src/parquet/metadata_test.cc
@@ -119,15 +119,13 @@ TEST(Metadata, TestBuildAccess) {
auto f_accessor = GenerateTableMetaData(schema, props, nrows, stats_int,
stats_float);
std::string f_accessor_serialized_metadata = f_accessor->SerializeToString();
- uint32_t expected_len =
static_cast<uint32_t>(f_accessor_serialized_metadata.length());
+ const auto expected_len =
static_cast<int64_t>(f_accessor_serialized_metadata.length());
- // decoded_len is an in-out parameter
- uint32_t decoded_len = expected_len;
auto f_accessor_copy =
- FileMetaData::Make(f_accessor_serialized_metadata.data(), &decoded_len);
+ FileMetaData::Make(f_accessor_serialized_metadata.data(), expected_len);
// Check that all of the serialized data is consumed
- ASSERT_EQ(expected_len, decoded_len);
+ ASSERT_EQ(expected_len, f_accessor_copy->size());
// Run this block twice, one for f_accessor, one for f_accessor_copy.
// To make sure SerializedMetadata was deserialized correctly.
@@ -284,14 +282,11 @@ std::string EncodeInt32(int32_t value) {
constexpr int32_t kLegacyMin = 100, kLegacyMax = 200;
std::string SerializeMetadata(const format::FileMetaData& thrift_metadata) {
- std::string out;
- ThriftSerializer{}.SerializeToString(&thrift_metadata, &out);
- return out;
+ return std::string(ThriftSerializer{}.SerializeToString(&thrift_metadata));
}
-std::shared_ptr<FileMetaData> ParseMetadata(std::string serialized_metadata) {
- uint32_t decoded_len = static_cast<uint32_t>(serialized_metadata.size());
- return FileMetaData::Make(serialized_metadata.data(), &decoded_len);
+std::shared_ptr<FileMetaData> ParseMetadata(std::string_view
serialized_metadata) {
+ return FileMetaData::Make(serialized_metadata.data(),
serialized_metadata.size());
}
format::FileMetaData SingleInt32MetadataWithStats() {
diff --git a/cpp/src/parquet/page_index.cc b/cpp/src/parquet/page_index.cc
index 1d2faebd251..69ab0b5f616 100644
--- a/cpp/src/parquet/page_index.cc
+++ b/cpp/src/parquet/page_index.cc
@@ -966,13 +966,13 @@ RowGroupIndexReadRange
PageIndexReader::DeterminePageIndexRangesInRowGroup(
std::unique_ptr<ColumnIndex> ColumnIndex::Make(const ColumnDescriptor& descr,
const void* serialized_index,
- uint32_t index_len,
+ int64_t index_len,
const ReaderProperties&
properties,
Decryptor* decryptor) {
format::ColumnIndex column_index;
ThriftDeserializer deserializer(properties);
deserializer.DeserializeMessage(reinterpret_cast<const
uint8_t*>(serialized_index),
- &index_len, &column_index, decryptor);
+ index_len, &column_index, decryptor);
if (ARROW_PREDICT_FALSE(LoadEnumSafe(&column_index.boundary_order) ==
BoundaryOrder::UNDEFINED)) {
// Guard against UB when moving column_index
@@ -1011,13 +1011,13 @@ std::unique_ptr<ColumnIndex> ColumnIndex::Make(const
ColumnDescriptor& descr,
}
std::unique_ptr<OffsetIndex> OffsetIndex::Make(const void* serialized_index,
- uint32_t index_len,
+ int64_t index_len,
const ReaderProperties&
properties,
Decryptor* decryptor) {
format::OffsetIndex offset_index;
ThriftDeserializer deserializer(properties);
deserializer.DeserializeMessage(reinterpret_cast<const
uint8_t*>(serialized_index),
- &index_len, &offset_index, decryptor);
+ index_len, &offset_index, decryptor);
return std::make_unique<OffsetIndexImpl>(offset_index);
}
diff --git a/cpp/src/parquet/page_index.h b/cpp/src/parquet/page_index.h
index 67e68288532..7bc341d77be 100644
--- a/cpp/src/parquet/page_index.h
+++ b/cpp/src/parquet/page_index.h
@@ -34,7 +34,7 @@ class PARQUET_EXPORT ColumnIndex {
/// \brief Create a ColumnIndex from a serialized thrift message.
static std::unique_ptr<ColumnIndex> Make(const ColumnDescriptor& descr,
const void* serialized_index,
- uint32_t index_len,
+ int64_t index_len,
const ReaderProperties& properties,
Decryptor* decryptor = NULLPTR);
@@ -132,7 +132,7 @@ class PARQUET_EXPORT OffsetIndex {
public:
/// \brief Create a OffsetIndex from a serialized thrift message.
static std::unique_ptr<OffsetIndex> Make(const void* serialized_index,
- uint32_t index_len,
+ int64_t index_len,
const ReaderProperties& properties,
Decryptor* decryptor = NULLPTR);
diff --git a/cpp/src/parquet/page_index_test.cc
b/cpp/src/parquet/page_index_test.cc
index 3a7308c1c6b..bfd5c90bdef 100644
--- a/cpp/src/parquet/page_index_test.cc
+++ b/cpp/src/parquet/page_index_test.cc
@@ -309,8 +309,7 @@ std::shared_ptr<FileMetaData> ConstructFakeMetaData(
auto sink = CreateOutputStream();
ThriftSerializer{}.Serialize(&metadata, sink.get());
auto buffer = sink->Finish().MoveValueUnsafe();
- uint32_t len = static_cast<uint32_t>(buffer->size());
- return FileMetaData::Make(buffer->data(), &len);
+ return FileMetaData::Make(buffer->data(), buffer->size());
}
/// Validates that 'DeterminePageIndexRangesInRowGroup()' selects the expected
file
diff --git a/cpp/src/parquet/thrift_internal.h
b/cpp/src/parquet/thrift_internal.h
index 971e6ccebc9..a74d97c1e28 100644
--- a/cpp/src/parquet/thrift_internal.h
+++ b/cpp/src/parquet/thrift_internal.h
@@ -25,6 +25,7 @@
#include <span>
#include <sstream>
#include <string>
+#include <string_view>
#include <type_traits>
#include <utility>
#include <vector>
@@ -586,35 +587,35 @@ class ThriftDeserializer {
container_size_limit_(container_size_limit) {}
// Deserialize a thrift message from buf/len. buf/len must at least contain
- // all the bytes needed to store the thrift message. On return, len will be
- // set to the actual length of the header.
+ // all the bytes needed to store the thrift message.
+ // The actual length of the header is returned.
template <class T>
- void DeserializeMessage(const uint8_t* buf, uint32_t* len, T*
deserialized_msg,
- Decryptor* decryptor = NULLPTR) {
+ int64_t DeserializeMessage(const uint8_t* buf, int64_t len, T*
deserialized_msg,
+ Decryptor* decryptor = NULLPTR) {
if (decryptor == NULLPTR) {
// thrift message is not encrypted
- DeserializeUnencryptedMessage(buf, len, deserialized_msg);
+ return DeserializeUnencryptedMessage(buf, len, deserialized_msg);
} else {
// thrift message is encrypted
- uint32_t clen;
- clen = *len;
- if (clen > static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
+ if (len > std::numeric_limits<int32_t>::max()) {
std::stringstream ss;
- ss << "Cannot decrypt buffer with length " << clen << ", which
overflows int32\n";
+ ss << "Cannot decrypt buffer with length " << len << ", which
overflows int32\n";
throw ParquetException(ss.str());
}
// decrypt
auto decrypted_buffer = AllocateBuffer(
- decryptor->pool(),
decryptor->PlaintextLength(static_cast<int32_t>(clen)));
- std::span<const uint8_t> cipher_buf(buf, clen);
- uint32_t decrypted_buffer_len =
+ decryptor->pool(),
decryptor->PlaintextLength(static_cast<int32_t>(len)));
+ std::span<const uint8_t> cipher_buf(buf, len);
+ int32_t decrypted_buffer_len =
decryptor->Decrypt(cipher_buf,
decrypted_buffer->mutable_span_as<uint8_t>());
if (decrypted_buffer_len <= 0) {
throw ParquetException("Couldn't decrypt buffer\n");
}
- *len =
decryptor->CiphertextLength(static_cast<int32_t>(decrypted_buffer_len));
- DeserializeUnencryptedMessage(decrypted_buffer->data(),
&decrypted_buffer_len,
+ int64_t read_bytes = decryptor->CiphertextLength(decrypted_buffer_len);
+ ARROW_DCHECK_LE(read_bytes, len);
+ DeserializeUnencryptedMessage(decrypted_buffer->data(),
decrypted_buffer_len,
deserialized_msg);
+ return read_bytes;
}
}
@@ -622,21 +623,28 @@ class ThriftDeserializer {
// On Thrift 0.14.0+, we want to use TConfiguration to raise the max message
size
// limit (ARROW-13655). If we wanted to protect against huge messages, we
could
// do it ourselves since we know the message size up front.
- std::shared_ptr<ThriftBuffer> CreateReadOnlyMemoryBuffer(uint8_t* buf,
uint32_t len) {
+ std::shared_ptr<ThriftBuffer> CreateReadOnlyMemoryBuffer(uint8_t* buf,
int64_t len) {
+ if (len >= static_cast<int64_t>(std::numeric_limits<uint32_t>::max())) {
+ std::stringstream ss;
+ ss << "Cannot deserialize Thrift message with length " << len
+ << ", which overflows uint32\n";
+ throw ParquetException(ss.str());
+ }
#if PARQUET_THRIFT_VERSION_MAJOR > 0 || PARQUET_THRIFT_VERSION_MINOR >= 14
auto conf = std::make_shared<apache::thrift::TConfiguration>();
conf->setMaxMessageSize(std::numeric_limits<int>::max());
- return std::make_shared<ThriftBuffer>(buf, len, ThriftBuffer::OBSERVE,
conf);
+ return std::make_shared<ThriftBuffer>(buf, static_cast<uint32_t>(len),
+ ThriftBuffer::OBSERVE, conf);
#else
- return std::make_shared<ThriftBuffer>(buf, len);
+ return std::make_shared<ThriftBuffer>(buf, static_cast<uint32_t>(len));
#endif
}
template <class T>
- void DeserializeUnencryptedMessage(const uint8_t* buf, uint32_t* len,
- T* deserialized_msg) {
+ int64_t DeserializeUnencryptedMessage(const uint8_t* buf, int64_t len,
+ T* deserialized_msg) {
// Deserialize msg bytes into c++ thrift msg using memory transport.
- auto tmem_transport =
CreateReadOnlyMemoryBuffer(const_cast<uint8_t*>(buf), *len);
+ auto tmem_transport =
CreateReadOnlyMemoryBuffer(const_cast<uint8_t*>(buf), len);
auto tproto = apache::thrift::protocol::TCompactProtocolT<ThriftBuffer>(
tmem_transport, string_size_limit_, container_size_limit_);
try {
@@ -648,8 +656,7 @@ class ThriftDeserializer {
ss << "Couldn't deserialize thrift: " << e.what() << "\n";
throw ParquetException(ss.str());
}
- uint32_t bytes_left = tmem_transport->available_read();
- *len = *len - bytes_left;
+ return len - static_cast<int64_t>(tmem_transport->available_read());
}
const int32_t string_size_limit_;
@@ -672,30 +679,35 @@ class ThriftSerializer {
/// memory returned is owned by this object and will be invalid when another
object
/// is serialized.
template <class T>
- void SerializeToBuffer(const T* obj, uint32_t* len, uint8_t** buffer) {
+ std::span<const uint8_t> SerializeToBuffer(const T* obj) {
SerializeObject(obj);
- mem_buffer_->getBuffer(buffer, len);
+ uint8_t* data;
+ uint32_t data_len;
+ mem_buffer_->getBuffer(&data, &data_len);
+ return std::span(data, data_len);
}
template <class T>
- void SerializeToString(const T* obj, std::string* result) {
+ std::string_view SerializeToString(const T* obj) {
SerializeObject(obj);
- *result = mem_buffer_->getBufferAsString();
+ uint8_t* data;
+ uint32_t data_len;
+ mem_buffer_->getBuffer(&data, &data_len);
+ return std::string_view(reinterpret_cast<const char*>(data), data_len);
}
template <class T>
int64_t Serialize(const T* obj, ArrowOutputStream* out,
Encryptor* encryptor = NULLPTR) {
- uint8_t* out_buffer;
- uint32_t out_length;
- SerializeToBuffer(obj, &out_length, &out_buffer);
+ auto out_buffer = SerializeToBuffer(obj);
// obj is not encrypted
if (encryptor == NULLPTR) {
- PARQUET_THROW_NOT_OK(out->Write(out_buffer, out_length));
- return static_cast<int64_t>(out_length);
+ PARQUET_THROW_NOT_OK(
+ out->Write(out_buffer.data(),
static_cast<int64_t>(out_buffer.size())));
+ return static_cast<int64_t>(out_buffer.size());
} else { // obj is encrypted
- return SerializeEncryptedObj(out, out_buffer, out_length, encryptor);
+ return SerializeEncryptedObj(out, out_buffer, encryptor);
}
}
@@ -712,16 +724,16 @@ class ThriftSerializer {
}
}
- int64_t SerializeEncryptedObj(ArrowOutputStream* out, const uint8_t*
out_buffer,
- uint32_t out_length, Encryptor* encryptor) {
+ int64_t SerializeEncryptedObj(ArrowOutputStream* out,
+ std::span<const uint8_t> serialized,
+ Encryptor* encryptor) {
auto cipher_buffer =
- AllocateBuffer(encryptor->pool(),
encryptor->CiphertextLength(out_length));
- std::span<const uint8_t> out_span(out_buffer, out_length);
+ AllocateBuffer(encryptor->pool(),
encryptor->CiphertextLength(serialized.size()));
int32_t cipher_buffer_len =
- encryptor->Encrypt(out_span,
cipher_buffer->mutable_span_as<uint8_t>());
+ encryptor->Encrypt(serialized,
cipher_buffer->mutable_span_as<uint8_t>());
PARQUET_THROW_NOT_OK(out->Write(cipher_buffer->data(), cipher_buffer_len));
- return static_cast<int64_t>(cipher_buffer_len);
+ return cipher_buffer_len;
}
std::shared_ptr<ThriftBuffer> mem_buffer_;
diff --git a/cpp/tools/parquet/parquet_dump_footer.cc
b/cpp/tools/parquet/parquet_dump_footer.cc
index 4dd7476bc8e..8e03f684dbd 100644
--- a/cpp/tools/parquet/parquet_dump_footer.cc
+++ b/cpp/tools/parquet/parquet_dump_footer.cc
@@ -57,7 +57,7 @@ int DoIt(std::string in, bool scrub, bool debug, std::string
out) {
std::cerr << "Not a Parquet file: " << in << "\n";
return 4;
}
- uint32_t metadata_len = ReadLE32(data + tail_len - 8);
+ int64_t metadata_len = ReadLE32(data + tail_len - 8);
if (tail_len >= metadata_len + 8) {
// The footer is entirely in the initial read. Trim to size.
tail = tail.substr(tail_len - (metadata_len + 8));
@@ -72,7 +72,7 @@ int DoIt(std::string in, bool scrub, bool debug, std::string
out) {
data = tail.data();
file->ReadAt(file_len - tail_len, tail_len, data).ValueOrDie();
}
- auto md = FileMetaData::Make(tail.data(), &metadata_len);
+ auto md = FileMetaData::Make(tail.data(), metadata_len);
std::string ser = md->SerializeUnencrypted(scrub, debug);
if (!debug) {
AppendLE32(static_cast<uint32_t>(ser.size()), &ser);
