Skip to content

GH-50971: [C++][Parquet] Fix usage of disparate length types for metadata reading #50972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pitrou merged 2 commits into from
Aug 25, 2026
Merged

GH-50971: [C++][Parquet] Fix usage of disparate length types for metadata reading #50972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44a6a60
GH-50971: [C++][Parquet] Fix usage of disparate length types for meta…
pitrou Aug 24, 2026
671358f
Address review comments
pitrou Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions cpp/src/parquet/bloom_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 5 additions & 7 deletions cpp/src/parquet/column_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -444,18 +444,16 @@ 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,
&data_page_header_aad_);
}
// 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, &current_page_header_,
meta_decryptor_.get());
header_size = deserializer.DeserializeMessage(
reinterpret_cast<const uint8_t*>(view.data()), view.size(),
&current_page_header_, meta_decryptor_.get());
break;
} catch (std::exception& e) {
// Failed to deserialize. Double the allowed page header size and try again
Expand Down
67 changes: 32 additions & 35 deletions cpp/src/parquet/file_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand All @@ -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 &&
Expand All @@ -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));
Expand All @@ -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
Expand Down Expand Up @@ -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)](
Expand All @@ -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) {
Expand Down Expand Up @@ -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(
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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) {
Expand Down
Loading
Loading