From 03d4041145924ea51ac6be473e17015564e5736b Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Tue, 1 Sep 2026 17:11:27 -0500 Subject: [PATCH 1/2] GH-50314: [C++][Parquet] Reject invalid DELTA_BINARY_PACKED headers InitHeader() sizes the bit-width buffer from the header's miniblock count without tying it to the page size, so a 10-byte page claiming 2^20 miniblocks allocates 1 MiB before failing. InitBlock() reads one bit-width byte per miniblock, so such a page can never decode. Generated-by: GitHub Copilot CLI (Claude Opus 5) Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 14 ++++++++++++++ cpp/src/parquet/encoding_test.cc | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index c4d3fe5a8a5a..1ff33c110ce1 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1561,6 +1561,20 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { } total_values_remaining_ = total_value_count_; + // GH-50314: mini_blocks_per_block_ comes from the page header and sizes the + // allocation below, while InitBlock() reads one bit-width byte per miniblock. + // A count larger than the bytes left can never decode, so we reject it here + // instead of allowing it to drive a large allocation. A page holding a single + // value keeps that value in the header and never calls InitBlock(), so this + // check skips it. + if (total_value_count_ > 1 && + static_cast(mini_blocks_per_block_) > decoder_->bytes_left()) { + throw ParquetException( + "the number of miniblocks per block (" + + std::to_string(mini_blocks_per_block_) + + ") is larger than the number of bytes remaining in the page (" + + std::to_string(decoder_->bytes_left()) + ")"); + } if (delta_bit_widths_ == nullptr) { delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); } else { diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 831829e4a210..e6c09316037d 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -31,6 +32,7 @@ #include "arrow/array/builder_dict.h" #include "arrow/array/concatenate.h" #include "arrow/compute/cast.h" +#include "arrow/memory_pool.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/random.h" #include "arrow/testing/util.h" @@ -1902,6 +1904,36 @@ TYPED_TEST(TestDeltaBitPackEncoding, BasicRoundTrip) { } } +TYPED_TEST(TestDeltaBitPackEncoding, SingleValueRoundTrip) { + ASSERT_NO_FATAL_FAILURE(this->Execute(1, 1)); +} + +TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { + using T = typename TypeParam::c_type; + + // Header: 2^25 values per block, 2^20 miniblocks, 2 values, and first value 0, + // followed by min delta 0 and no miniblock bit widths. + const std::vector encoded = {0x80, 0x80, 0x80, 0x10, 0x80, + 0x80, 0x40, 0x02, 0x00, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + std::vector decoded(2); + + EXPECT_THROW_THAT( + [&] { + decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->Decode(decoded.data(), static_cast(decoded.size())); + }, + ParquetException, + ::testing::Property( + &ParquetException::what, + ::testing::HasSubstr( + "the number of miniblocks per block (1048576) is larger than the " + "number of bytes remaining in the page (1)"))); + EXPECT_EQ(pool.bytes_allocated(), 0); +} + TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) { // GH-14923: depending on the number of encoded values, some of the miniblock // bitwidths are actually padding bytes that may take non-conformant values From 21133d68ccdf6abeb6bc1558f92abd088c1520b1 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 3 Sep 2026 14:31:53 -0400 Subject: [PATCH 2/2] GH-50314: [C++][Parquet] Avoid single-value miniblock allocation Single-value pages never initialize a block, so leave the bit-width buffer unallocated. Account for the required min-delta byte when validating block metadata, and use cumulative allocation counts in the regression tests. Generated-by: GitHub Copilot CLI (Claude Opus 5) Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 35 +++++++++++------------- cpp/src/parquet/encoding_test.cc | 46 +++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index 1ff33c110ce1..ddaa4a578024 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1561,25 +1561,22 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { } total_values_remaining_ = total_value_count_; - // GH-50314: mini_blocks_per_block_ comes from the page header and sizes the - // allocation below, while InitBlock() reads one bit-width byte per miniblock. - // A count larger than the bytes left can never decode, so we reject it here - // instead of allowing it to drive a large allocation. A page holding a single - // value keeps that value in the header and never calls InitBlock(), so this - // check skips it. - if (total_value_count_ > 1 && - static_cast(mini_blocks_per_block_) > decoder_->bytes_left()) { - throw ParquetException( - "the number of miniblocks per block (" + - std::to_string(mini_blocks_per_block_) + - ") is larger than the number of bytes remaining in the page (" + - std::to_string(decoder_->bytes_left()) + ")"); - } - if (delta_bit_widths_ == nullptr) { - delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); - } else { - PARQUET_THROW_NOT_OK( - delta_bit_widths_->Resize(mini_blocks_per_block_, /*shrink_to_fit*/ false)); + if (total_value_count_ > 1) { + const int64_t bytes_left = decoder_->bytes_left(); + const int64_t bit_width_bytes_left = std::max(0, bytes_left - 1); + if (static_cast(mini_blocks_per_block_) > bit_width_bytes_left) { + throw ParquetException( + "the number of miniblocks per block (" + + std::to_string(mini_blocks_per_block_) + + ") is larger than the number of bytes available for miniblock bit widths (" + + std::to_string(bit_width_bytes_left) + ")"); + } + if (delta_bit_widths_ == nullptr) { + delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); + } else { + PARQUET_THROW_NOT_OK( + delta_bit_widths_->Resize(mini_blocks_per_block_, /*shrink_to_fit*/ false)); + } } first_block_initialized_ = false; values_remaining_current_mini_block_ = 0; diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index e6c09316037d..bb074fb3dedd 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -1904,8 +1904,21 @@ TYPED_TEST(TestDeltaBitPackEncoding, BasicRoundTrip) { } } -TYPED_TEST(TestDeltaBitPackEncoding, SingleValueRoundTrip) { - ASSERT_NO_FATAL_FAILURE(this->Execute(1, 1)); +TYPED_TEST(TestDeltaBitPackEncoding, SingleValueSkipsMiniblockAllocation) { + using T = typename TypeParam::c_type; + + // Header: 2^25 values per block, 2^20 miniblocks, 1 value, and first value 0. + const std::vector encoded = {0x80, 0x80, 0x80, 0x10, 0x80, + 0x80, 0x40, 0x01, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + T decoded = 1; + + decoder->SetData(1, encoded.data(), static_cast(encoded.size())); + ASSERT_EQ(decoder->Decode(&decoded, 1), 1); + EXPECT_EQ(decoded, 0); + EXPECT_EQ(pool.total_bytes_allocated(), 0); } TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { @@ -1930,8 +1943,33 @@ TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { &ParquetException::what, ::testing::HasSubstr( "the number of miniblocks per block (1048576) is larger than the " - "number of bytes remaining in the page (1)"))); - EXPECT_EQ(pool.bytes_allocated(), 0); + "number of bytes available for miniblock bit widths (0)"))); + EXPECT_EQ(pool.total_bytes_allocated(), 0); +} + +TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsWithoutMinDelta) { + using T = typename TypeParam::c_type; + + // Header: 128 values per block, 1 miniblock, 2 values, and first value 0, + // followed by only one byte for the min delta and miniblock bit width. + const std::vector encoded = {0x80, 0x01, 0x01, 0x02, 0x00, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + std::vector decoded(2); + + EXPECT_THROW_THAT( + [&] { + decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->Decode(decoded.data(), static_cast(decoded.size())); + }, + ParquetException, + ::testing::Property( + &ParquetException::what, + ::testing::HasSubstr( + "the number of miniblocks per block (1) is larger than the number " + "of bytes available for miniblock bit widths (0)"))); + EXPECT_EQ(pool.total_bytes_allocated(), 0); } TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) {