From 7e2e06bab2d64cde7ee03eb0a7efbd3abbebcd12 Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Wed, 15 Apr 2026 19:46:57 +0800 Subject: [PATCH 1/4] fix: ParquetError when reading corrupt parquet file with truncated data instead of Panic --- parquet/src/file/metadata/reader.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/parquet/src/file/metadata/reader.rs b/parquet/src/file/metadata/reader.rs index 2be9dcbd4b01..60c57129fd24 100644 --- a/parquet/src/file/metadata/reader.rs +++ b/parquet/src/file/metadata/reader.rs @@ -530,7 +530,13 @@ impl ParquetMetaDataReader { let remainder_start = *remainder_start as u64; let offset = usize::try_from(range.start - remainder_start)?; let end = usize::try_from(range.end - remainder_start)?; - assert!(end <= remainder.len()); + if end > remainder.len() { + return Err(general_err!( + "Corrupted parquet file: index data range ({:?}) exceeds remainder length ({})", + range, + remainder.len() + )); + } remainder.slice(offset..end) } // Note: this will potentially fetch data already in remainder, this keeps things simple @@ -538,7 +544,13 @@ impl ParquetMetaDataReader { }; // Sanity check - assert_eq!(bytes.len() as u64, range.end - range.start); + if bytes.len() as u64 != range.end - range.start { + return Err(general_err!( + "Corrupted parquet file: index data length mismatch, expected {}, got {}", + range.end - range.start, + bytes.len() + )); + } push_decoder.push_range(range.clone(), bytes)?; let metadata = parse_index_data(&mut push_decoder)?; self.metadata = Some(metadata); From 0bb99427f62f36fbeaf680c62265b200881c1549 Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Thu, 16 Apr 2026 10:45:38 +0800 Subject: [PATCH 2/4] Added test case --- parquet/src/file/metadata/reader.rs | 75 +++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/parquet/src/file/metadata/reader.rs b/parquet/src/file/metadata/reader.rs index 60c57129fd24..6bda3da91b41 100644 --- a/parquet/src/file/metadata/reader.rs +++ b/parquet/src/file/metadata/reader.rs @@ -1434,4 +1434,79 @@ mod async_tests { read_and_check(f.as_file(), PageIndexPolicy::Optional).unwrap(); read_and_check(f.as_file(), PageIndexPolicy::Skip).unwrap(); } + + /// Test that corrupted parquet files return ParquetError instead of panicking + /// This test verifies that when page index data is corrupted/truncated, + /// the reader returns an error rather than panicking with an assertion failure + #[tokio::test] + async fn test_corrupted_parquet_file_returns_error() { + // Create a normal parquet file with page indexes + let file = write_parquet_file(false).unwrap(); + let mut file_reader = file.reopen().unwrap(); + let file_len = file_reader.len(); + + // Read the full file + let mut full_data = Vec::new(); + file_reader.read_to_end(&mut full_data).unwrap(); + let full_data = Bytes::from(full_data); + + // First, load the metadata normally to get the page index locations + let mut metadata_reader = ParquetMetaDataReader::new().with_page_index_policy(PageIndexPolicy::Required); + let mut fetch = |range: Range| { + futures::future::ready(Ok::<_, ParquetError>( + full_data.slice(range.start as usize..range.end as usize), + )) + }; + let f = MetadataFetchFn(&mut fetch); + metadata_reader.try_load(f, file_len).await.unwrap(); + let metadata = metadata_reader.finish().unwrap(); + + // Create a push decoder to determine what ranges are needed for page indexes + let mut push_decoder = ParquetMetaDataPushDecoder::try_new_with_metadata(file_len, metadata) + .unwrap() + .with_offset_index_policy(PageIndexPolicy::Required) + .with_column_index_policy(PageIndexPolicy::Required); + + // Get the ranges needed for page index data + let needs_data = needs_index_data(&mut push_decoder).unwrap(); + match needs_data { + NeedsIndexData::No(_) => { + // If no index data needed, skip test + return; + } + NeedsIndexData::Yes(_) => { + // Continue with test + } + }; + + // Now create a corrupted fetcher that returns insufficient data for the page index + // This simulates the scenario from the issue where the file is truncated + let mut corrupted_fetch = |range: Range| { + // Return less data than requested to trigger the error + let requested_len = (range.end - range.start) as usize; + let bad_data = Bytes::from(vec![0u8; requested_len / 2]); // Return half the data + futures::future::ready(Ok::<_, ParquetError>(bad_data)) + }; + + // Try to load metadata with corrupted data + let f2 = MetadataFetchFn(&mut corrupted_fetch); + let result = ParquetMetaDataReader::new() + .with_page_index_policy(PageIndexPolicy::Required) + .load_and_finish(f2, file_len) + .await; + + // Should return an error (either our new error or a parse error), not panic + assert!( + result.is_err(), + "Expected error when reading corrupted parquet file, but got Ok" + ); + + let err_msg = result.unwrap_err().to_string(); + // The error should not be a panic assertion failure + assert!( + !err_msg.contains("assertion failed"), + "Got assertion panic instead of error: {}", + err_msg + ); + } } From d96cdc372ed34ac891c1df9e656725e0461dfcee Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Thu, 16 Apr 2026 10:48:34 +0800 Subject: [PATCH 3/4] Fix styles --- parquet/src/file/metadata/reader.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/parquet/src/file/metadata/reader.rs b/parquet/src/file/metadata/reader.rs index 6bda3da91b41..690b18ca9b66 100644 --- a/parquet/src/file/metadata/reader.rs +++ b/parquet/src/file/metadata/reader.rs @@ -1451,7 +1451,8 @@ mod async_tests { let full_data = Bytes::from(full_data); // First, load the metadata normally to get the page index locations - let mut metadata_reader = ParquetMetaDataReader::new().with_page_index_policy(PageIndexPolicy::Required); + let mut metadata_reader = + ParquetMetaDataReader::new().with_page_index_policy(PageIndexPolicy::Required); let mut fetch = |range: Range| { futures::future::ready(Ok::<_, ParquetError>( full_data.slice(range.start as usize..range.end as usize), @@ -1462,10 +1463,11 @@ mod async_tests { let metadata = metadata_reader.finish().unwrap(); // Create a push decoder to determine what ranges are needed for page indexes - let mut push_decoder = ParquetMetaDataPushDecoder::try_new_with_metadata(file_len, metadata) - .unwrap() - .with_offset_index_policy(PageIndexPolicy::Required) - .with_column_index_policy(PageIndexPolicy::Required); + let mut push_decoder = + ParquetMetaDataPushDecoder::try_new_with_metadata(file_len, metadata) + .unwrap() + .with_offset_index_policy(PageIndexPolicy::Required) + .with_column_index_policy(PageIndexPolicy::Required); // Get the ranges needed for page index data let needs_data = needs_index_data(&mut push_decoder).unwrap(); From 35898c0b27a42253fd45324b7840d5e708f6d44f Mon Sep 17 00:00:00 2001 From: xuzifu666 <1206332514@qq.com> Date: Thu, 16 Apr 2026 21:31:51 +0800 Subject: [PATCH 4/4] Addressed --- parquet/src/file/metadata/reader.rs | 77 ----------------------------- 1 file changed, 77 deletions(-) diff --git a/parquet/src/file/metadata/reader.rs b/parquet/src/file/metadata/reader.rs index 690b18ca9b66..60c57129fd24 100644 --- a/parquet/src/file/metadata/reader.rs +++ b/parquet/src/file/metadata/reader.rs @@ -1434,81 +1434,4 @@ mod async_tests { read_and_check(f.as_file(), PageIndexPolicy::Optional).unwrap(); read_and_check(f.as_file(), PageIndexPolicy::Skip).unwrap(); } - - /// Test that corrupted parquet files return ParquetError instead of panicking - /// This test verifies that when page index data is corrupted/truncated, - /// the reader returns an error rather than panicking with an assertion failure - #[tokio::test] - async fn test_corrupted_parquet_file_returns_error() { - // Create a normal parquet file with page indexes - let file = write_parquet_file(false).unwrap(); - let mut file_reader = file.reopen().unwrap(); - let file_len = file_reader.len(); - - // Read the full file - let mut full_data = Vec::new(); - file_reader.read_to_end(&mut full_data).unwrap(); - let full_data = Bytes::from(full_data); - - // First, load the metadata normally to get the page index locations - let mut metadata_reader = - ParquetMetaDataReader::new().with_page_index_policy(PageIndexPolicy::Required); - let mut fetch = |range: Range| { - futures::future::ready(Ok::<_, ParquetError>( - full_data.slice(range.start as usize..range.end as usize), - )) - }; - let f = MetadataFetchFn(&mut fetch); - metadata_reader.try_load(f, file_len).await.unwrap(); - let metadata = metadata_reader.finish().unwrap(); - - // Create a push decoder to determine what ranges are needed for page indexes - let mut push_decoder = - ParquetMetaDataPushDecoder::try_new_with_metadata(file_len, metadata) - .unwrap() - .with_offset_index_policy(PageIndexPolicy::Required) - .with_column_index_policy(PageIndexPolicy::Required); - - // Get the ranges needed for page index data - let needs_data = needs_index_data(&mut push_decoder).unwrap(); - match needs_data { - NeedsIndexData::No(_) => { - // If no index data needed, skip test - return; - } - NeedsIndexData::Yes(_) => { - // Continue with test - } - }; - - // Now create a corrupted fetcher that returns insufficient data for the page index - // This simulates the scenario from the issue where the file is truncated - let mut corrupted_fetch = |range: Range| { - // Return less data than requested to trigger the error - let requested_len = (range.end - range.start) as usize; - let bad_data = Bytes::from(vec![0u8; requested_len / 2]); // Return half the data - futures::future::ready(Ok::<_, ParquetError>(bad_data)) - }; - - // Try to load metadata with corrupted data - let f2 = MetadataFetchFn(&mut corrupted_fetch); - let result = ParquetMetaDataReader::new() - .with_page_index_policy(PageIndexPolicy::Required) - .load_and_finish(f2, file_len) - .await; - - // Should return an error (either our new error or a parse error), not panic - assert!( - result.is_err(), - "Expected error when reading corrupted parquet file, but got Ok" - ); - - let err_msg = result.unwrap_err().to_string(); - // The error should not be a panic assertion failure - assert!( - !err_msg.contains("assertion failed"), - "Got assertion panic instead of error: {}", - err_msg - ); - } }