pmoust commented on code in PR #11419:
URL: https://github.com/apache/arrow-rs/pull/11419#discussion_r4201550479
##########
parquet/src/file/metadata/thrift/mod.rs:
##########
@@ -820,7 +820,23 @@ pub(crate) fn parquet_metadata_from_bytes(
let list_ident = prot.read_list_begin()?;
// check for list of struct
validate_list_type(ElementType::Struct, &list_ident)?;
- let mut rg_vec = Vec::with_capacity(list_ident.size as usize);
+ let size = list_ident.size as usize;
+ // This list is decoded inline rather than through
+ // `read_thrift_vec`, so it needs the same bound: each row
+ // group occupies at least one byte on the wire, so a declared
+ // count larger than the remaining input is malformed. Reject
+ // it before reserving, so a tiny footer cannot drive a
+ // multi-gigabyte allocation.
+ if let Some(remaining) = prot.remaining_bytes()
Review Comment:
Done. Extracted `check_list_size` in `parquet_thrift.rs`; both
`read_thrift_vec` and the row-group site call it now, so the bound lives in one
place.
##########
parquet/src/file/metadata/thrift/mod.rs:
##########
@@ -820,7 +820,23 @@ pub(crate) fn parquet_metadata_from_bytes(
let list_ident = prot.read_list_begin()?;
// check for list of struct
validate_list_type(ElementType::Struct, &list_ident)?;
- let mut rg_vec = Vec::with_capacity(list_ident.size as usize);
+ let size = list_ident.size as usize;
+ // This list is decoded inline rather than through
+ // `read_thrift_vec`, so it needs the same bound: each row
+ // group occupies at least one byte on the wire, so a declared
+ // count larger than the remaining input is malformed. Reject
+ // it before reserving, so a tiny footer cannot drive a
+ // multi-gigabyte allocation.
Review Comment:
Applied: the row-group site now calls `check_list_size` with this comment.
##########
parquet/src/file/metadata/thrift/mod.rs:
##########
@@ -2095,4 +2111,51 @@ pub(crate) mod tests {
vec![None, Some(1), Some(2)],
);
}
+
+ /// A footer whose `row_groups` list declares more elements than the
+ /// remaining input could hold is rejected before the reserve, so a tiny
+ /// footer cannot drive a large allocation. This list is decoded inline
+ /// rather than through `read_thrift_vec`, so it carries its own bound.
Review Comment:
Applied.
--
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]
