This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11700-70d8190350ffb2fd3a34c92d8844d505eaf0af75 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 193b9d30a65a791b1b2d5e7f2b3df215195dfade Author: Efe <[email protected]> AuthorDate: Thu Oct 29 05:09:06 2026 +0000 fix(parquet): reject truncated definition level runs (#11700) # Which issue does this PR close? - Closes #11691 # Rationale for this change A truncated bit-packed definition-level run currently panics while building a bitmap instead of returning a Parquet EOF error. # What changes are included in this PR? Validate the packed payload before `read` or `skip` consumes it. Add decoder tests for both paths and a Parquet reader regression based on the issue reproduction. AI assistance was used to draft the implementation and tests. # Are these changes tested? The Parquet library suite passed (1,434 tests), as did formatting. The reader regression panics on unchanged main and returns EOF with this fix. Local Clippy completed with an unknown-lint warning because the installed Rust 1.98.1 is older than the repository’s pinned 1.99.1. # Are there any significant user-facing changes? Malformed input now returns EOF instead of panicking; valid files are unaffected. --- .../src/arrow/arrow_reader/tests/column_reader.rs | 40 ++++++++++++++++++++++ .../src/arrow/record_reader/definition_levels.rs | 32 +++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/parquet/src/arrow/arrow_reader/tests/column_reader.rs b/parquet/src/arrow/arrow_reader/tests/column_reader.rs index 7ecb2fd61e..5b9c9ab1e4 100644 --- a/parquet/src/arrow/arrow_reader/tests/column_reader.rs +++ b/parquet/src/arrow/arrow_reader/tests/column_reader.rs @@ -19,6 +19,46 @@ use super::*; +#[test] +fn test_truncated_definition_level_bitpacked_run_returns_error() { + let array: Int32Array = (0..1000).map(|i| (i % 2 == 0).then_some(i)).collect(); + let schema = Arc::new(Schema::new(vec![Field::new( + "c", + ArrowDataType::Int32, + true, + )])); + let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(array)]).unwrap(); + let props = WriterProperties::builder() + .set_dictionary_enabled(false) + .build(); + + let mut buf = Vec::new(); + let mut writer = ArrowWriter::try_new(&mut buf, schema, Some(props)).unwrap(); + writer.write(&batch).unwrap(); + writer.close().unwrap(); + + // Shorten the definition-level payload while leaving its packed-run header intact. + let pattern = [0x7F, 0, 0, 0, 0x7F, 0x55]; + let pos = buf + .windows(pattern.len()) + .position(|w| w == pattern) + .unwrap(); + buf[pos] = 3; + + let mut reader = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(buf)) + .unwrap() + .build() + .unwrap(); + let error = reader.next().unwrap().unwrap_err(); + + assert!( + error + .to_string() + .contains("unexpected end of file whilst decoding definition levels bit-packed run"), + "{error}" + ); +} + #[test] fn test_null_column_reader_test() { let mut file = tempfile::tempfile().unwrap(); diff --git a/parquet/src/arrow/record_reader/definition_levels.rs b/parquet/src/arrow/record_reader/definition_levels.rs index 46def4e798..88b2febb75 100644 --- a/parquet/src/arrow/record_reader/definition_levels.rs +++ b/parquet/src/arrow/record_reader/definition_levels.rs @@ -333,6 +333,15 @@ impl PackedDecoder { let indicator_value = self.decode_header()?; let num_values = run_len(indicator_value as u64)? as usize; if indicator_value & 1 == 1 { + let packed_bytes = num_values / 8; + // For a bit width of 1, each group of 8 levels occupies one byte. + // Validate the whole run before either read() or skip() accesses it. + let remaining = self.data.len().saturating_sub(self.data_offset); + if packed_bytes > remaining { + return Err(ParquetError::EOF(format!( + "unexpected end of file whilst decoding definition levels bit-packed run: expected {packed_bytes} bytes, found {remaining}" + ))); + } self.packed_count = num_values; self.packed_offset = 0; } else { @@ -616,6 +625,29 @@ mod tests { assert_eq!(read_value + skip_value, total_value); } + #[test] + fn test_packed_decoder_truncated_run_returns_error() { + // The header declares 504 levels (63 bytes), but only three packed bytes follow. + let mut decoder = PackedDecoder::new(); + decoder.set_data(Encoding::RLE, Bytes::from_static(&[0x7F, 0x55, 0x55, 0x55])); + + let mut decoded = BooleanBufferBuilder::new(504); + let result = decoder.read(&mut decoded, 504); + + assert!(matches!(result, Err(ParquetError::EOF(_)))); + } + + #[test] + fn test_packed_decoder_skip_truncated_run_returns_error() { + // The skip path must validate the same source range as the read path. + let mut decoder = PackedDecoder::new(); + decoder.set_data(Encoding::RLE, Bytes::from_static(&[0x7F, 0x55, 0x55, 0x55])); + + let result = decoder.skip(504); + + assert!(matches!(result, Err(ParquetError::EOF(_)))); + } + #[test] fn test_try_consume_all_valid() { // Test with all-valid data (all 1s) - single RLE run
