From 6a9bb02f37c0b9c5b2fef7dc84285383ffa038fe Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 12:49:05 -0500 Subject: [PATCH] format: read local heaps over Storage LocalHeap::parse_in reads the header in one bounded read, validate_free_list_in reads each free block's two lengths, and read_string_in reads from the string to the end of the data segment once and looks for the terminator there. The &[u8] methods are wrappers. New test: a heap without free space, with a valid free block and with a free block overrunning the segment, cut at every length, parse, validate and read strings identically through a read_at-only CountingStorage. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/local_heap.rs | 108 ++++++++++++++++------- 1 file changed, 78 insertions(+), 30 deletions(-) diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index 39e9b26..a4e3480 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -4,6 +4,7 @@ use alloc::string::String; use crate::error::FormatError; +use crate::storage::{Storage, len_usize, read_exact_at}; /// Parsed HDF5 Local Heap header. #[derive(Debug, Clone)] @@ -16,21 +17,6 @@ pub struct LocalHeap { pub data_segment_address: u64, } -/// Checks that `[offset, offset + needed)` fits within `data`, guarding the -/// addition against `usize` overflow from a crafted near-`usize::MAX` offset. -fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> { - if offset - .checked_add(needed) - .is_none_or(|end| end > data.len()) - { - return Err(FormatError::UnexpectedEof { - expected: offset.saturating_add(needed), - available: data.len(), - }); - } - Ok(()) -} - fn read_offset(data: &[u8], pos: usize, size: u8) -> Result { let s = size as usize; if pos.checked_add(s).is_none_or(|end| end > data.len()) { @@ -57,12 +43,24 @@ impl LocalHeap { offset: usize, offset_size: u8, length_size: u8, + ) -> Result { + Self::parse_in(&file_data, offset as u64, offset_size, length_size) + } + + /// [`Self::parse`] over any [`Storage`]: one read of the header. + pub fn parse_in( + file: &dyn Storage, + offset: u64, + offset_size: u8, + length_size: u8, ) -> Result { // signature(4) + version(1) + reserved(3) = 8, then length_size*2 + offset_size let ls = length_size as usize; let os = offset_size as usize; let total = 8 + ls * 2 + os; - ensure_len(file_data, offset, total)?; + let header = read_exact_at(file, offset, total)?; + let file_data: &[u8] = &header; + let offset = 0usize; if &file_data[offset..offset + 4] != b"HEAP" { return Err(FormatError::InvalidLocalHeapSignature); @@ -99,6 +97,16 @@ impl LocalHeap { /// The end of the list is `H5HL_FREE_NULL` (1); an all-ones value (the /// undefined address) is accepted as "no free list" too. pub fn validate_free_list(&self, file_data: &[u8], length_size: u8) -> Result<(), FormatError> { + self.validate_free_list_in(&file_data, length_size) + } + + /// [`Self::validate_free_list`] over any [`Storage`]: two small reads + /// per free block. + pub fn validate_free_list_in( + &self, + file: &dyn Storage, + length_size: u8, + ) -> Result<(), FormatError> { const FREE_NULL: u64 = 1; let ls = length_size as usize; let undefined = if ls >= 8 { @@ -123,11 +131,12 @@ impl LocalHeap { .and_then(|a| usize::try_from(a).ok()) .ok_or(FormatError::InvalidLocalHeapFreeList)?; let block_offset = next; - next = read_offset(file_data, at, length_size)?; + next = read_offset(&read_exact_at(file, at as u64, ls)?, 0, length_size)?; if next == 0 { return Err(FormatError::InvalidLocalHeapFreeList); } - let block_size = read_offset(file_data, at + ls, length_size)?; + let block_size = + read_offset(&read_exact_at(file, (at + ls) as u64, ls)?, 0, length_size)?; if block_offset .checked_add(block_size) .is_none_or(|end| end > size) @@ -140,6 +149,17 @@ impl LocalHeap { /// Read a null-terminated string from the heap's data segment at the given byte offset. pub fn read_string(&self, file_data: &[u8], string_offset: u64) -> Result { + self.read_string_in(&file_data, string_offset) + } + + /// [`Self::read_string`] over any [`Storage`]: one read, from the + /// string to the end of the data segment. + pub fn read_string_in( + &self, + file: &dyn Storage, + string_offset: u64, + ) -> Result { + let file_len = len_usize(file); let seg_addr = self.data_segment_address as usize; let str_start = seg_addr @@ -153,28 +173,24 @@ impl LocalHeap { "local heap seg_addr + data_segment_size overflow".into(), ))?; - if str_start >= file_data.len() || str_start >= seg_end { + if str_start >= file_len || str_start >= seg_end { return Err(FormatError::UnexpectedEof { expected: str_start + 1, - available: file_data.len(), + available: file_len, }); } // Find null terminator - let search_end = seg_end.min(file_data.len()); - let mut end = str_start; - while end < search_end && file_data[end] != 0 { - end += 1; - } - - if end >= search_end { + let search_end = seg_end.min(file_len); + let rest = read_exact_at(file, str_start as u64, search_end - str_start)?; + let Some(len) = rest.iter().position(|&b| b == 0) else { return Err(FormatError::UnexpectedEof { - expected: end + 1, + expected: search_end + 1, available: search_end, }); - } + }; - let s = core::str::from_utf8(&file_data[str_start..end]) + let s = core::str::from_utf8(&rest[..len]) .map_err(|_| FormatError::InvalidLocalHeapSignature)?; Ok(String::from(s)) } @@ -345,4 +361,36 @@ mod tests { let err = LocalHeap::parse(&file, 0, 8, 8).unwrap_err(); assert_eq!(err, FormatError::InvalidLocalHeapVersion(1)); } + + /// Header, free list and strings read identically through a + /// `read_at`-only storage, for every truncation of the file. + #[test] + fn storage_reads_match_slice_reads() { + use crate::storage::CountingStorage; + let plain = build_heap_file(0, 64, &["", "alpha", "beta"], 8, 8); + // A free block of 16 bytes at segment offset 12, ending the list. + let mut free = build_heap_file(0, 64, &["", "alpha", "beta", &"x".repeat(20)], 8, 8); + free[16..24].copy_from_slice(&12u64.to_le_bytes()); + free[64 + 12..64 + 20].copy_from_slice(&1u64.to_le_bytes()); + free[64 + 20..64 + 28].copy_from_slice(&16u64.to_le_bytes()); + let mut bad_free = free.clone(); + bad_free[64 + 20..64 + 28].copy_from_slice(&99u64.to_le_bytes()); + for full in [plain, free, bad_free] { + for cut in 0..=full.len() { + let f = &full[..cut]; + let storage = CountingStorage::new(f.to_vec()); + let want = LocalHeap::parse(f, 0, 8, 8); + let got = LocalHeap::parse_in(&storage, 0, 8, 8); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + let Ok(heap) = want else { continue }; + assert_eq!( + heap.validate_free_list_in(&storage, 8), + heap.validate_free_list(f, 8) + ); + for off in [0u64, 1, 2, 6, 7, 11, 100] { + assert_eq!(heap.read_string_in(&storage, off), heap.read_string(f, off)); + } + } + } + } }