(arrow-rs) 01/01: fix(parquet): return an error when the footer metadata length exceeds the file in ParquetMetaDataPushDecoder (#11697)

[gi...@apache.org]

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-11697-ee9f2a38f640f7147b2e849ea4fafa7f4333a45a
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git

commit 837b6703f0ea8e33b891584511bb99d84794d2ef
Author: ump45nose <52...@users.noreply.github.com>
AuthorDate: Mon Oct 19 16:55:37 2026 +0000

    fix(parquet): return an error when the footer metadata length exceeds the file in ParquetMetaDataPushDecoder (#11697)
    
    # Which issue does this PR close?
    
    - Closes #11690.
    
    # Rationale for this change
    
    `ParquetMetaDataPushDecoder::try_decode` computed the metadata start as
    `file_len - footer_len - metadata_len` without checking the footer's
    metadata length against the file length. A corrupt footer that claims
    more metadata than the file holds made that subtraction underflow: it
    panicked with overflow checks on, and without them the wrapped value
    later panicked in `Bytes::slice`. `ParquetMetaDataReader` already
    returns an EOF error for the same bytes.
    
    # What changes are included in this PR?
    
    `try_decode` in `parquet/src/file/metadata/push_decoder.rs` uses
    `checked_sub` for the metadata start and returns the same `EOF: Parquet
    file too small. Size is N but need M` error as `ParquetMetaDataReader`
    when the footer and metadata don't fit in the file.
    
    # Are these changes tested?
    
    Yes. `test_metadata_decoder_metadata_length_exceeds_file` builds the
    12-byte file from the issue (4 data bytes and a footer claiming 1000
    bytes of metadata) and checks that `try_decode` returns `EOF: Parquet
    file too small. Size is 12 but need 1008`. Without the fix the test
    panics with `attempt to subtract with overflow`.
    
    I also ran `cargo test -p parquet --lib push_decoder`, `cargo fmt -p
    parquet -- --check` and `cargo clippy -p parquet --lib --tests -- -D
    warnings`.
    
    # Are there any significant user-facing changes?
    
    No. Corrupt files that used to panic now return an error.
    
    AI disclosure: I used an AI coding assistant to write the fix, the test
    and this description, and I reviewed all of it. The part worth a
    reviewer's look is that the decoder is left in its intermediate state
    after this error, the same as for the other decode errors in
    `try_decode` (for example a bad footer magic), rather than being
    restored so the call can be retried.
---
 parquet/src/file/metadata/push_decoder.rs | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/parquet/src/file/metadata/push_decoder.rs b/parquet/src/file/metadata/push_decoder.rs
index 202fa0d163..8ad49a557d 100644
--- a/parquet/src/file/metadata/push_decoder.rs
+++ b/parquet/src/file/metadata/push_decoder.rs
@@ -427,7 +427,13 @@ impl ParquetMetaDataPushDecoder {
 
                 DecodeState::ReadingMetadata(footer_tail) => {
                     let metadata_len: u64 = footer_tail.metadata_length() as u64;
-                    let metadata_start = file_len - footer_len - metadata_len;
+                    // A corrupt footer can claim more metadata than the file holds
+                    let needed = footer_len + metadata_len;
+                    let Some(metadata_start) = file_len.checked_sub(needed) else {
+                        return Err(eof_err!(
+                            "Parquet file too small. Size is {file_len} but need {needed}"
+                        ));
+                    };
                     let metadata_end = metadata_start + metadata_len;
                     let metadata_range = metadata_start..metadata_end;
 
@@ -690,6 +696,24 @@ mod tests {
         assert!(metadata.page_index().is_some_and(|idx| idx.is_complete()));
     }
 
+    /// A footer whose metadata length exceeds the file length is an error, not a panic
+    #[test]
+    fn test_metadata_decoder_metadata_length_exceeds_file() {
+        // 4 bytes of data, then a footer claiming 1000 bytes of metadata
+        let mut file = vec![0u8; 4];
+        file.extend_from_slice(&1000u32.to_le_bytes());
+        file.extend_from_slice(b"PAR1");
+        let file_len = file.len() as u64;
+
+        let mut decoder = ParquetMetaDataPushDecoder::try_new(file_len).unwrap();
+        decoder.push_range(0..file_len, Bytes::from(file)).unwrap();
+        let err = decoder.try_decode().unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "EOF: Parquet file too small. Size is 12 but need 1008"
+        );
+    }
+
     #[test]
     fn test_range_for_page_index_respects_column_mask() {
         let mut decoder = ParquetMetaDataPushDecoder::try_new(test_file_len()).unwrap();