From 06625b7470c72ccac3ff764577975d26e587aece Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 12:50:25 -0500 Subject: [PATCH] format: read global heap collections over Storage GlobalHeapCollection::parse_in / parse_index_in read the header, check the collection against the end of the file, and read the collection in one bounded read; objects are indexed in it with file offsets, as before. The &[u8] functions are wrappers. New test: collections with 4- and 8-byte lengths, one whose size runs past the file and one whose object overruns it, at two offsets and cut at every length, give identical results through a read_at-only CountingStorage. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/global_heap.rs | 133 ++++++++++++++++++---- 1 file changed, 109 insertions(+), 24 deletions(-) diff --git a/crates/clawhdf5-format/src/global_heap.rs b/crates/clawhdf5-format/src/global_heap.rs index 474c7fd..c6de363 100644 --- a/crates/clawhdf5-format/src/global_heap.rs +++ b/crates/clawhdf5-format/src/global_heap.rs @@ -1,9 +1,12 @@ //! HDF5 Global Heap collection parsing. #[cfg(not(feature = "std"))] -use alloc::{format, string::String, vec::Vec}; +use alloc::{borrow::Cow, format, string::String, vec::Vec}; +#[cfg(feature = "std")] +use std::borrow::Cow; use crate::error::FormatError; +use crate::storage::{Storage, len_usize, read_exact_at}; /// Magic signature for global heap collections. const GCOL_SIGNATURE: [u8; 4] = *b"GCOL"; @@ -28,19 +31,20 @@ pub struct GlobalHeapObject { pub data: Vec, } -fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> { +/// Checks that `[offset, offset + needed)` ends by `data_len`. +fn ensure_len(data_len: usize, offset: usize, needed: usize) -> Result<(), FormatError> { match offset.checked_add(needed) { - Some(end) if end <= data.len() => Ok(()), + Some(end) if end <= data_len => Ok(()), _ => Err(FormatError::UnexpectedEof { expected: offset.saturating_add(needed), - available: data.len(), + available: data_len, }), } } fn read_length(data: &[u8], offset: usize, length_size: u8) -> Result { let s = length_size as usize; - ensure_len(data, offset, s)?; + ensure_len(data.len(), offset, s)?; let slice = &data[offset..offset + s]; Ok(match length_size { 2 => u16::from_le_bytes([slice[0], slice[1]]) as u64, @@ -95,7 +99,17 @@ impl GlobalHeapCollection { offset: usize, length_size: u8, ) -> Result { - let index = Self::parse_index(file_data, offset, length_size)?; + Self::parse_in(&file_data, offset as u64, length_size) + } + + /// [`Self::parse`] over any [`Storage`]: one read of the header, one of + /// the collection. + pub fn parse_in( + file: &dyn Storage, + offset: u64, + length_size: u8, + ) -> Result { + let (bytes, base, index) = Self::read_collection(file, offset, length_size)?; Ok(GlobalHeapCollection { collection_size: index.collection_size, objects: index @@ -104,7 +118,7 @@ impl GlobalHeapCollection { .map(|o| GlobalHeapObject { index: o.index, reference_count: o.reference_count, - data: file_data[o.offset..o.offset + o.size].to_vec(), + data: bytes[o.offset - base..o.offset - base + o.size].to_vec(), }) .collect(), }) @@ -122,43 +136,72 @@ impl GlobalHeapCollection { offset: usize, length_size: u8, ) -> Result { + Self::parse_index_in(&file_data, offset as u64, length_size) + } + + /// [`Self::parse_index`] over any [`Storage`]: one read of the header, + /// one of the collection. The object offsets are file offsets. + pub fn parse_index_in( + file: &dyn Storage, + offset: u64, + length_size: u8, + ) -> Result { + Ok(Self::read_collection(file, offset, length_size)?.2) + } + + /// Read the collection at `offset` and index its objects: the + /// collection's bytes, its offset as a `usize`, and the index (with + /// file offsets). + fn read_collection( + file: &dyn Storage, + offset: u64, + length_size: u8, + ) -> Result<(Cow<'_, [u8]>, usize, GlobalHeapIndex), FormatError> { + let file_len = len_usize(file); // signature(4) + version(1) + reserved(3) + collection_size(length_size), // padded to a multiple of 8 as libhdf5 lays it out (`H5HG_SIZEOF_HDR`). // With 8-byte lengths the padding is 0; with 4-byte lengths it is 4, // and reading without it put every object 4 bytes early. let header_size = pad8(8 + length_size as usize); - ensure_len(file_data, offset, header_size)?; + let header = read_exact_at(file, offset, header_size)?; + let offset = usize::try_from(offset).map_err(|_| FormatError::UnexpectedEof { + expected: usize::MAX, + available: file_len, + })?; - if file_data[offset..offset + 4] != GCOL_SIGNATURE { + if header[..4] != GCOL_SIGNATURE { return Err(FormatError::InvalidGlobalHeapSignature); } - let version = file_data[offset + 4]; + let version = header[4]; if version != 1 { return Err(FormatError::InvalidGlobalHeapVersion(version)); } - let collection_size = read_length(file_data, offset + 8, length_size)?; + let collection_size = read_length(&header, 8, length_size)?; let collection_end = usize::try_from(collection_size) .ok() .and_then(|size| offset.checked_add(size)) .ok_or(FormatError::UnexpectedEof { expected: usize::MAX, - available: file_data.len(), + available: file_len, })?; - if collection_end > file_data.len() { + if collection_end > file_len { return Err(FormatError::UnexpectedEof { expected: collection_end, - available: file_data.len(), + available: file_len, }); } + let collection = read_exact_at(file, offset as u64, collection_end - offset)?; + // Positions below are file offsets; `file_data(p)` is the byte at `p`. + let file_data = |p: usize| collection[p - offset]; let mut pos = offset + header_size; let mut objects = Vec::new(); // Parse objects until we hit index 0 (free space) or run out of space while pos + 2 <= collection_end { - let object_index = u16::from_le_bytes([file_data[pos], file_data[pos + 1]]); + let object_index = u16::from_le_bytes([file_data(pos), file_data(pos + 1)]); if object_index == 0 { // Free space marker — done @@ -168,11 +211,12 @@ impl GlobalHeapCollection { // object_index(2) + reference_count(2) + reserved(4) + // object_size(length_size), padded to 8 (`H5HG_SIZEOF_OBJHDR`). let obj_header_size = pad8(8 + length_size as usize); - ensure_len(&file_data[..collection_end], pos, obj_header_size)?; + ensure_len(collection_end, pos, obj_header_size)?; - let reference_count = u16::from_le_bytes([file_data[pos + 2], file_data[pos + 3]]); - let object_size = usize::try_from(read_length(file_data, pos + 8, length_size)?) - .map_err(|_| FormatError::Overflow("global heap object size".into()))?; + let reference_count = u16::from_le_bytes([file_data(pos + 2), file_data(pos + 3)]); + let object_size = + usize::try_from(read_length(&collection[pos - offset..], 8, length_size)?) + .map_err(|_| FormatError::Overflow("global heap object size".into()))?; pos += obj_header_size; if pos @@ -197,10 +241,11 @@ impl GlobalHeapCollection { pos = pos.saturating_add(pad8(object_size)); } - Ok(GlobalHeapIndex { + let index = GlobalHeapIndex { collection_size, objects, - }) + }; + Ok((collection, offset, index)) } /// Get an object by its index. @@ -226,7 +271,7 @@ mod tests { let mut obj_size_total = 0usize; for (_, _, data) in objects { let obj_header = pad8(8 + ls); - obj_size_total += obj_header + pad8(data.len()); + obj_size_total += obj_header + pad8(<[u8]>::len(data)); } // Free space marker (2 bytes for index 0) obj_size_total += 2; @@ -258,8 +303,8 @@ mod tests { buf.resize(buf.len() + (pad8(8 + ls) - (8 + ls)), 0); buf.extend_from_slice(data); // Pad to 8 bytes - let padded = pad8(data.len()); - buf.resize(buf.len() + (padded - data.len()), 0); + let padded = pad8(<[u8]>::len(data)); + buf.resize(buf.len() + (padded - <[u8]>::len(data)), 0); } // Free space marker @@ -327,4 +372,44 @@ mod tests { assert_eq!(coll.objects.len(), 1); assert_eq!(coll.objects[0].data, b"test"); } + + /// Collections, and every truncation of them, index and parse + /// identically through a `read_at`-only storage: two reads each. + #[test] + fn storage_parse_matches_slice_parse() { + use crate::storage::CountingStorage; + let objs: &[(u16, u16, &[u8])] = &[(1, 1, b"hello"), (2, 3, b"a longer object")]; + for ls in [4u8, 8] { + let coll = build_collection(objs, ls); + let mut corrupt = coll.clone(); + corrupt[8] = 200; // collection size past the end of the file + let mut overrun = coll.clone(); + let size_at = pad8(8 + ls as usize) + 8; + overrun[size_at] = 250; // first object runs past the collection + for full in [coll, corrupt, overrun] { + for at in [0usize, 5] { + for cut in 0..=full.len() { + let mut f = vec![0u8; at]; + f.extend_from_slice(&full[..cut]); + let storage = CountingStorage::new(f.clone()); + let want = GlobalHeapCollection::parse(&f, at, ls); + let got = GlobalHeapCollection::parse_in(&storage, at as u64, ls); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + let want = GlobalHeapCollection::parse_index(&f, at, ls); + let got = GlobalHeapCollection::parse_index_in(&storage, at as u64, ls); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + } + } + } + } + let storage = CountingStorage::new(build_collection(objs, 8)); + assert_eq!( + GlobalHeapCollection::parse_in(&storage, 0, 8) + .unwrap() + .objects + .len(), + 2 + ); + assert_eq!(storage.reads(), 2); + } }