1fanwang commented on code in PR #51128:
URL: https://github.com/apache/arrow/pull/51128#discussion_r3928069379


##########
cpp/src/parquet/decoder.cc:
##########
@@ -1561,6 +1561,20 @@ class DeltaBitPackDecoder : public 
TypedDecoderImpl<DType> {
     }
 
     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

Review Comment:
   Done in 21133d68.



##########
cpp/src/parquet/encoding_test.cc:
##########
@@ -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<uint8_t> encoded = {0x80, 0x80, 0x80, 0x10, 0x80,
+                                        0x80, 0x40, 0x02, 0x00, 0x00};
+  ::arrow::ProxyMemoryPool pool(default_memory_pool());
+  auto decoder = MakeTypedDecoder<TypeParam>(Encoding::DELTA_BINARY_PACKED,
+                                             this->descr_.get(), &pool);
+  std::vector<T> decoded(2);
+
+  EXPECT_THROW_THAT(
+      [&] {
+        decoder->SetData(2, encoded.data(), static_cast<int>(encoded.size()));
+        decoder->Decode(decoded.data(), static_cast<int>(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);

Review Comment:
   Done in 21133d68.



-- 
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]

Reply via email to