diff --git a/crates/clawhdf5-format/src/extensible_array.rs b/crates/clawhdf5-format/src/extensible_array.rs index a95c124..c2f23f8 100644 --- a/crates/clawhdf5-format/src/extensible_array.rs +++ b/crates/clawhdf5-format/src/extensible_array.rs @@ -12,7 +12,7 @@ use alloc::{format, vec, vec::Vec}; use crate::chunk_grid::ChunkGrid; use crate::chunked_read::ChunkInfo; use crate::error::FormatError; -use crate::storage::{Storage, Window, read_exact_at}; +use crate::storage::{PAGED_BLOCK_ONE_READ_MAX, Storage, Window, read_exact_at}; /// Verify the Jenkins lookup3 checksum stored immediately after /// `data[start..end]`, as every Extensible Array structure carries one. `w` @@ -373,6 +373,9 @@ fn read_data_block_elements( .checked_mul(elem_bytes) .and_then(|b| pos.checked_add(b)) .ok_or_else(|| FormatError::Overflow("Extensible Array data block span".into()))?; + // The checksum's bounds check comes first: make it before reading. + #[cfg(feature = "checksum")] + Window::check_extent(file, db_offset, end, 4)?; let w = Window::read(file, db_offset, end.saturating_add(4))?; verify_checksum(&w, 0, end)?; read_run(&w, pos, nelmts, start_index, &mut chunks)?; @@ -384,13 +387,26 @@ fn read_data_block_elements( // clear were never written; their slot still occupies the file, so stride // over it rather than reading zeros as addresses. let npages = nelmts.div_ceil(page); - // The whole data block in one window: every position checked below lies - // inside it (or past the end of the file). + // The whole data block in one window when it is small: every position + // checked below lies inside it (or past the end of the file). A larger + // block is read as its prefix, then each page in use on its own. let block_len = pos .saturating_add(4) .saturating_add(npages.saturating_mul(page.saturating_mul(elem_bytes).saturating_add(4))); - let w = Window::read(file, db_offset, block_len)?; - verify_checksum(&w, 0, pos)?; + let whole = if block_len <= PAGED_BLOCK_ONE_READ_MAX { + Some(Window::read(file, db_offset, block_len)?) + } else { + None + }; + let head_w; + let head = match &whole { + Some(w) => w, + None => { + head_w = Window::read(file, db_offset, pos + 4)?; + &head_w + } + }; + verify_checksum(head, 0, pos)?; pos += 4; let page_stride = page .checked_mul(elem_bytes) @@ -405,10 +421,20 @@ fn read_data_block_elements( .is_some_and(|byte| byte & (0x80 >> (bit % 8)) != 0); if initialised { let count = core::cmp::min(page, nelmts - p * page); + // `w` holds the page from `base` on (positions below are + // relative to it, and `pos` to the data block). + let page_w; + let (w, base) = match &whole { + Some(w) => (w, 0), + None => { + page_w = Window::read(file, db_offset.saturating_add(pos as u64), page_stride)?; + (&page_w, pos) + } + }; // Each page carries its own checksum, over a full page's worth of // slots even when the last one holds fewer live elements. - verify_checksum(&w, pos, pos + page * elem_bytes)?; - read_run(&w, pos, count, start_index + p * page, &mut chunks)?; + verify_checksum(w, pos - base, pos - base + page * elem_bytes)?; + read_run(w, pos - base, count, start_index + p * page, &mut chunks)?; } pos = pos .checked_add(page_stride) @@ -544,6 +570,9 @@ pub fn read_extensible_array_chunks_in( .ok_or_else(|| FormatError::Overflow("Extensible Array index block span".into()))?; // The whole index block in one window: every position read below is // before `ib_end`. + // The checksum's bounds check comes first: make it before reading. + #[cfg(feature = "checksum")] + Window::check_extent(file, ib_offset, ib_end, 4)?; let w = Window::read(file, ib_offset, ib_end.saturating_add(4))?; verify_checksum(&w, 0, ib_end)?; @@ -682,26 +711,25 @@ fn read_super_block( // Positions below are relative to the super block, whose bytes (up to // its checksum) are all in one window. let bitmap_start = sb_header_size; - let w = Window::read( - file, - sb_offset, - bitmap_start - .saturating_add(bitmap_bytes) - .saturating_add(ndblks.saturating_mul(os)) - .saturating_add(4), - )?; - w.ensure(bitmap_start, bitmap_bytes)?; - let bitmap = &w.bytes[bitmap_start..bitmap_start + bitmap_bytes]; - + // The bitmap's bounds check, then (with checksums) the checksum's, come + // before anything else is read from the block: make them before reading + // it, so size fields stretching it past the end of the file cost no read. + Window::check_extent(file, sb_offset, bitmap_start, bitmap_bytes)?; let mut pos = bitmap_start + bitmap_bytes; - let mut chunks = Vec::new(); - let mut global_idx = start_index; // One checksum covers the prefix, the bitmap and every data block address. let sb_end = ndblks .checked_mul(os) .and_then(|b| pos.checked_add(b)) .ok_or_else(|| FormatError::Overflow("Extensible Array super block span".into()))?; + #[cfg(feature = "checksum")] + Window::check_extent(file, sb_offset, sb_end, 4)?; + let w = Window::read(file, sb_offset, sb_end.saturating_add(4))?; + w.ensure(bitmap_start, bitmap_bytes)?; + let bitmap = &w.bytes[bitmap_start..bitmap_start + bitmap_bytes]; + + let mut chunks = Vec::new(); + let mut global_idx = start_index; verify_checksum(&w, 0, sb_end)?; for i in 0..ndblks { diff --git a/crates/clawhdf5-format/src/fixed_array.rs b/crates/clawhdf5-format/src/fixed_array.rs index e6be63b..711aa53 100644 --- a/crates/clawhdf5-format/src/fixed_array.rs +++ b/crates/clawhdf5-format/src/fixed_array.rs @@ -9,7 +9,7 @@ use alloc::{format, vec, vec::Vec}; use crate::chunk_grid::ChunkGrid; use crate::chunked_read::ChunkInfo; use crate::error::FormatError; -use crate::storage::{Storage, Window, len_usize, read_exact_at}; +use crate::storage::{PAGED_BLOCK_ONE_READ_MAX, Storage, Window, len_usize, read_exact_at}; /// Verify the Jenkins lookup3 checksum stored immediately after /// `data[start..end]`, as every Fixed Array structure carries one. `w` is @@ -278,6 +278,9 @@ pub fn read_fixed_array_chunks_in( // then a checksum over both. One window holds all of it (or ends at // the end of the file), so its bounds checks are the whole-file ones. let end = elem_at(elements_start, num_elements)?; + // The checksum's bounds check comes first: make it before reading. + #[cfg(feature = "checksum")] + Window::check_extent(file, db_offset as u64, end - db_offset, 4)?; let w = Window::read(file, db_offset as u64, end.saturating_add(4) - db_offset)?; verify_checksum(&w, 0, end - db_offset)?; for i in 0..num_elements { @@ -310,21 +313,34 @@ pub fn read_fixed_array_chunks_in( available: file_len, }); } - // The whole data block in one window: every page slot is at most - // `page_stride` bytes, so every position checked below lies inside it - // (or past the end of the file). + // The whole data block in one window when it is small: every page slot + // is at most `page_stride` bytes, so every position checked below lies + // inside it (or past the end of the file). A larger block is read as its + // prefix and bitmap, then each page in use on its own. let block_len = (pages_start - db_offset).saturating_add(npages.saturating_mul(page_stride)); - let w = Window::read(file, db_offset as u64, block_len)?; + let whole = if block_len <= PAGED_BLOCK_ONE_READ_MAX { + Some(Window::read(file, db_offset as u64, block_len)?) + } else { + None + }; + let head_w; + let head = match &whole { + Some(w) => w, + None => { + head_w = Window::read(file, db_offset as u64, pages_start - db_offset)?; + &head_w + } + }; // The prefix and page bitmap are covered by their own checksum, and each // initialised page by one of its own. - verify_checksum(&w, 0, bitmap_start + bitmap_size - db_offset)?; + verify_checksum(head, 0, bitmap_start + bitmap_size - db_offset)?; for p in 0..npages { let page_first = p * page_nelmts; // < num_elements, cannot overflow let page_count = core::cmp::min(page_nelmts, num_elements - page_first); // Check the page-init bit (MSB-first within each byte). - let bit_byte = w.bytes[bitmap_start + p / 8 - db_offset]; + let bit_byte = head.bytes[bitmap_start + p / 8 - db_offset]; let bit_mask = 1u8 << (7 - (p % 8)); if bit_byte & bit_mask == 0 { continue; // entire page unallocated @@ -334,18 +350,21 @@ pub fn read_fixed_array_chunks_in( .checked_mul(page_stride) .and_then(|o| pages_start.checked_add(o)) .ok_or_else(stride_overflow)?; - verify_checksum( - &w, - page_off - db_offset, - elem_at(page_off, page_count)? - db_offset, - )?; + let page_end = elem_at(page_off, page_count)?; + // `w` holds the page from `base` on (positions below are relative + // to it). + let page_w; + let (w, base) = match &whole { + Some(w) => (w, db_offset), + None => { + page_w = + Window::read(file, page_off as u64, page_end.saturating_add(4) - page_off)?; + (&page_w, page_off) + } + }; + verify_checksum(w, page_off - base, page_end - base)?; for e in 0..page_count { - push_element( - &w, - page_first + e, - elem_at(page_off, e)? - db_offset, - &mut chunks, - )?; + push_element(w, page_first + e, elem_at(page_off, e)? - base, &mut chunks)?; } } @@ -949,4 +968,29 @@ mod tests { } } } + + /// A header whose element count stretches its data block (one checksum + /// over the whole block) far past the end of a 16 MiB file: the + /// checksum's bounds check fails before the block is read, with the + /// slice read's error. + #[cfg(feature = "checksum")] + #[test] + fn oversized_block_fails_before_reading() { + use crate::storage::CountingStorage; + let mut f = build_fixed_array(3, false, 10); + f.resize(16 << 20, 0); + let mut h = FixedArrayHeader::parse(&f, 0x100, 8, 8).unwrap(); + h.max_nelmts_bits = 30; + h.num_elements = 4 << 20; + let dims = [h.num_elements * 20]; + let want = read_fixed_array_chunks(&f, &h, &dims, None, &[20], 8, 8, 8); + assert!( + matches!(want, Err(FormatError::UnexpectedEof { .. })), + "{want:?}" + ); + let storage = CountingStorage::new(f); + let got = read_fixed_array_chunks_in(&storage, &h, &dims, None, &[20], 8, 8, 8); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + assert!(storage.bytes_read() < 64, "{} bytes", storage.bytes_read()); + } } diff --git a/crates/clawhdf5-format/src/fractal_heap.rs b/crates/clawhdf5-format/src/fractal_heap.rs index e47c2c9..e1f88ca 100644 --- a/crates/clawhdf5-format/src/fractal_heap.rs +++ b/crates/clawhdf5-format/src/fractal_heap.rs @@ -669,15 +669,20 @@ impl FractalHeapHeader { let iblock_header = 5 + offset_size as usize + block_offset_bytes; let tw = self.table_width as u64; let nrows_usize = nrows as usize; - let mut current_heap_offset = iblock_heap_offset; // Rows below max_direct_rows hold direct blocks; rows at/above hold // child indirect blocks. (NOT the FRHP "starting rows" field.) let start_indirect = self.max_direct_rows(); let max_direct_rows = nrows_usize.min(start_indirect); - // The block up to its last child entry, in one window: every - // position read below lies inside it, so its bounds checks are the + // The block up to its last child entry. The walk below reads + // entries in order and stops at the one covering the target, which + // the geometry alone locates, so the first window ends there: a + // header claiming a huge table costs a read of the entries in front + // of the target, not of the rest of the file. Only when that entry + // is unallocated (or none covers the target) does the walk go on, + // over the whole block. Either window holds what it was asked for or + // ends at the end of the file, so its bounds checks are the // whole-file ones. let direct_entry = usize::from(offset_size) + if self.filter_pipeline.is_some() { @@ -685,21 +690,129 @@ impl FractalHeapHeader { } else { 0 }; - let entries = - |rows: usize, entry: usize| rows.saturating_mul(tw as usize).saturating_mul(entry); - let block_len = iblock_header - .saturating_add(entries(max_direct_rows, direct_entry)) - .saturating_add(entries( - nrows_usize.saturating_sub(start_indirect), - usize::from(offset_size), - )); - let w = Window::read(file, iblock_addr as u64, block_len)?; + let direct_entries = max_direct_rows.saturating_mul(tw as usize); + let entries_len = |n: usize| { + n.min(direct_entries) + .saturating_mul(direct_entry) + .saturating_add( + n.saturating_sub(direct_entries) + .saturating_mul(usize::from(offset_size)), + ) + }; + let all_entries = direct_entries.saturating_add( + nrows_usize + .saturating_sub(start_indirect) + .saturating_mul(tw as usize), + ); + let block_len = iblock_header.saturating_add(entries_len(all_entries)); + let target_entry = self.indirect_entry_for(nrows_usize, iblock_heap_offset, target_offset); + let first_len = target_entry.map_or(block_len, |i| { + iblock_header + .saturating_add(entries_len(i.saturating_add(1))) + .min(block_len) + }); + let mut next = self.walk_indirect_block( + &Window::read(file, iblock_addr as u64, first_len)?, + nrows_usize, + iblock_heap_offset, + target_offset, + offset_size, + target_entry.map_or(usize::MAX, |i| i.saturating_add(1)), + )?; + if next.is_none() && first_len < block_len { + next = self.walk_indirect_block( + &Window::read(file, iblock_addr as u64, block_len)?, + nrows_usize, + iblock_heap_offset, + target_offset, + offset_size, + usize::MAX, + )?; + } + match next { + Some(IndirectChild::Direct(block)) => { + self.read_from_direct_block(file, block, target_offset, length) + } + Some(IndirectChild::Indirect { + addr, + nrows, + heap_offset, + }) => self.read_from_indirect_block( + file, + addr, + nrows, + heap_offset, + target_offset, + length, + offset_size, + depth_remaining - 1, + ), + None => Err(FormatError::UnexpectedEof { + expected: target_offset as usize + length, + available: len_usize(file), + }), + } + } + + /// Which child entry of an indirect block (numbered in walk order: + /// direct rows, then indirect rows) covers `target_offset`, from the + /// doubling-table geometry alone — the entry + /// [`Self::walk_indirect_block`] stops at if it is allocated. `None` + /// when no entry does. + fn indirect_entry_for(&self, nrows: usize, heap_offset: u64, target: u64) -> Option { + // The walk adds block sizes with saturation; in u128 the same test + // is `cur <= target < cur + size` without it (a target of u64::MAX + // is never inside a saturated range). + if target == u64::MAX { + return None; + } + let (tw, target) = (u128::from(self.table_width), u128::from(target)); + let mut cur = u128::from(heap_offset); + let mut before = 0usize; + for row in 0..nrows { + if target < cur { + return None; + } + // Direct and indirect rows alike span this row's block size per + // entry. + let size = u128::from(self.block_size_for_row(row)); + let span = size * tw; + if size > 0 && target < cur + span { + let col = usize::try_from((target - cur) / size).ok()?; + return before.checked_add(col); + } + cur += span; + before = before.saturating_add(self.table_width as usize); + } + None + } + + /// Walk an indirect block's child entries in order, in the window `w` + /// (the block from its signature on), and return the allocated child + /// covering `target_offset`, or `None` when no entry among the first + /// `limit` does. + fn walk_indirect_block( + &self, + w: &Window<'_>, + nrows: usize, + iblock_heap_offset: u64, + target_offset: u64, + offset_size: u8, + limit: usize, + ) -> Result, FormatError> { let ensure_len = |_: &[u8], pos: usize, needed: usize| w.ensure(pos, needed); let read_offset = |_: &[u8], pos: usize, size: u8| { w.ensure(pos, usize::from(size))?; read_offset(&w.bytes, pos, size) }; let file_data: &[u8] = &w.bytes; + let block_offset_bytes = (self.max_heap_size as usize).div_ceil(8); + let iblock_header = 5 + offset_size as usize + block_offset_bytes; + let tw = self.table_width as u64; + let mut current_heap_offset = iblock_heap_offset; + let start_indirect = self.max_direct_rows(); + let max_direct_rows = nrows.min(start_indirect); + let mut walked = 0usize; // Parse indirect block header ensure_len(file_data, 0, 4)?; @@ -712,6 +825,10 @@ impl FractalHeapHeader { let block_size = self.block_size_for_row(row); for _col in 0..tw { + if walked == limit { + return Ok(None); + } + walked += 1; let child_addr = read_offset(file_data, pos, offset_size)?; pos += offset_size as usize; @@ -738,18 +855,13 @@ impl FractalHeapHeader { && target_offset >= current_heap_offset && target_offset < block_end { - return self.read_from_direct_block( - file, - DirectBlock { - addr: child_addr as usize, - size: block_size, - heap_offset: current_heap_offset, - filtered_size, - filter_mask, - }, - target_offset, - length, - ); + return Ok(Some(IndirectChild::Direct(DirectBlock { + addr: child_addr as usize, + size: block_size, + heap_offset: current_heap_offset, + filtered_size, + filter_mask, + }))); } current_heap_offset = block_end; } @@ -758,11 +870,15 @@ impl FractalHeapHeader { // Rows at and above `start_indirect` hold child indirect blocks. A // child in row r spans exactly that row's block size of heap space, // so it has as many rows as a table of that total size needs. - for row in start_indirect..nrows_usize { + for row in start_indirect..nrows { let child_space = self.block_size_for_row(row); let child_nrows = self.rows_for_size(child_space); for _col in 0..tw { + if walked == limit { + return Ok(None); + } + walked += 1; let child_addr = read_offset(file_data, pos, offset_size)?; pos += offset_size as usize; @@ -771,25 +887,16 @@ impl FractalHeapHeader { && target_offset >= current_heap_offset && target_offset < block_end { - return self.read_from_indirect_block( - file, - child_addr as usize, - child_nrows, - current_heap_offset, - target_offset, - length, - offset_size, - depth_remaining - 1, - ); + return Ok(Some(IndirectChild::Indirect { + addr: child_addr as usize, + nrows: child_nrows, + heap_offset: current_heap_offset, + })); } current_heap_offset = block_end; } } - - Err(FormatError::UnexpectedEof { - expected: target_offset as usize + length, - available: len_usize(file), - }) + Ok(None) } /// Number of rows in the doubling table whose block size is at most the @@ -833,6 +940,16 @@ impl FractalHeapHeader { /// A managed direct block's location, extent and (for a filtered heap) its /// stored size and filter mask. +/// The child of an indirect block that covers a heap offset. +enum IndirectChild { + Direct(DirectBlock), + Indirect { + addr: usize, + nrows: u16, + heap_offset: u64, + }, +} + struct DirectBlock { addr: usize, size: u64, @@ -1151,6 +1268,50 @@ mod tests { } } + /// A header claiming a huge doubling table (width 0xFFFF, 0xFFFF rows in + /// the root indirect block) in a 16 MiB file: reading an object from the + /// table's first block reads the entries up to it, not the rest of the + /// file, and gives what the slice read gives. When the covering entry is + /// unallocated the walk goes on over the whole block, still identically. + #[test] + fn huge_table_claims_read_only_what_the_walk_needs() { + use crate::storage::CountingStorage; + let (mut file, _) = build_simple_heap(8, 8); + file.resize(16 << 20, 0); + file[600..604].copy_from_slice(b"FHIB"); + let first_entry = 600 + 5 + 8 + 2; + file[first_entry..first_entry + 8].copy_from_slice(&256u64.to_le_bytes()); + let mut hdr = FractalHeapHeader::parse(&file, 0, 8, 8).unwrap(); + hdr.table_width = 0xFFFF; + hdr.root_block_address = 600; + hdr.current_rows_in_root_indirect_block = 0xFFFF; + let managed_id = |offset: u64, len: u64| { + let payload = offset | (len << 16); + let mut id = vec![0u8]; + id.extend_from_slice(&payload.to_le_bytes()[..6]); + id + }; + let storage = CountingStorage::new(file.clone()); + let id = managed_id(15, 13); + let want = hdr.read_managed_object(&file, &id, 8); + assert!(want.is_ok(), "{want:?}"); + storage.reset(); + assert_eq!(hdr.read_managed_object_in(&storage, &id, 8), want); + assert!( + storage.bytes_read() < 1024, + "{} bytes in {} reads", + storage.bytes_read(), + storage.reads() + ); + // The second entry (heap offsets 128..256) is unallocated (zero is + // not the undefined address, so make it all ones). + file[first_entry + 8..first_entry + 16].fill(0xFF); + let storage = CountingStorage::new(file.clone()); + let id = managed_id(130, 4); + let want = hdr.read_managed_object(&file, &id, 8); + assert_eq!(hdr.read_managed_object_in(&storage, &id, 8), want); + } + /// A huge object found through the huge-object B-tree needs the whole /// file in memory until the B-tree reader is converted: a clean error /// on other storage. diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index 952a9d4..1215562 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -36,6 +36,10 @@ fn read_offset(data: &[u8], pos: usize, size: u8) -> Result { }) } +/// First read of a name on a backend without the file in memory: most link +/// names are shorter than this. +const NAME_READ_START: usize = 64; + impl LocalHeap { /// Parse a local heap header at the given offset in the file data. pub fn parse( @@ -152,8 +156,9 @@ impl LocalHeap { 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. + /// [`Self::read_string`] over any [`Storage`]: one read of up to 64 + /// bytes for a short name, more (each four times the last) up to the end + /// of the data segment for a longer one. pub fn read_string_in( &self, file: &S, @@ -180,19 +185,33 @@ impl LocalHeap { }); } - // Find null terminator + // Find the null terminator, which lies before the end of the data + // segment (or of the file). In memory that is one borrowed slice; + // otherwise the bytes are read in growing pieces, so a name costs a + // read of about its own length, not of the rest of the segment + // (whose size is an untrusted header field). 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: search_end + 1, - available: search_end, - }); + let total = search_end - str_start; + let mut want = if file.as_contiguous().is_some() { + total + } else { + total.min(NAME_READ_START) }; - - let s = core::str::from_utf8(&rest[..len]) - .map_err(|_| FormatError::InvalidLocalHeapSignature)?; - Ok(String::from(s)) + loop { + let rest = read_exact_at(file, str_start as u64, want)?; + if let Some(len) = rest.iter().position(|&b| b == 0) { + let s = core::str::from_utf8(&rest[..len]) + .map_err(|_| FormatError::InvalidLocalHeapSignature)?; + return Ok(String::from(s)); + } + if want == total { + return Err(FormatError::UnexpectedEof { + expected: search_end + 1, + available: search_end, + }); + } + want = want.saturating_mul(4).min(total); + } } } @@ -393,4 +412,45 @@ mod tests { } } } + + /// Names of every length around the first read's size, and one with no + /// terminator, read identically through a `read_at`-only storage; a + /// short name in a heap whose header claims a huge data segment costs + /// one small read, not a read of the rest of the file. + #[test] + fn long_names_and_hostile_segment_sizes() { + use crate::storage::CountingStorage; + let names: Vec = [0usize, 1, 63, 64, 65, 255, 256, 257, 1000, 5000] + .iter() + .map(|&n| "n".repeat(n)) + .collect(); + let refs: Vec<&str> = names.iter().map(String::as_str).collect(); + let mut file = build_heap_file(0, 64, &refs, 8, 8); + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + let storage = CountingStorage::new(file.clone()); + let mut off = 0u64; + for name in &names { + let got = heap.read_string_in(&storage, off); + assert_eq!(got, heap.read_string(&file, off)); + assert_eq!(got.unwrap(), *name); + off += name.len() as u64 + 1; + } + // The last name loses its terminator: both report the same error. + let seg_end = 64 + heap.data_segment_size as usize; + file[seg_end - 1] = b'n'; + let storage = CountingStorage::new(file.clone()); + let last = off - names[names.len() - 1].len() as u64 - 1; + let want = heap.read_string(&file, last); + assert!(want.is_err()); + assert_eq!(heap.read_string_in(&storage, last), want); + + // A 64 MiB file whose heap claims a data segment reaching its end. + let mut big = build_heap_file(0, 64, &["short", "names"], 8, 8); + big.resize(64 << 20, 0); + big[8..16].copy_from_slice(&((64u64 << 20) - 64).to_le_bytes()); + let heap = LocalHeap::parse(&big, 0, 8, 8).unwrap(); + let storage = CountingStorage::new(big.clone()); + assert_eq!(heap.read_string_in(&storage, 6).unwrap(), "names"); + assert_eq!((storage.reads(), storage.bytes_read()), (1, 64)); + } } diff --git a/crates/clawhdf5-format/src/storage.rs b/crates/clawhdf5-format/src/storage.rs index 6d09e7e..555ac78 100644 --- a/crates/clawhdf5-format/src/storage.rs +++ b/crates/clawhdf5-format/src/storage.rs @@ -235,6 +235,12 @@ fn short_read() -> FormatError { ) } +/// Largest paged data block (fixed or extensible array) read in one piece. +/// A bigger one is read as its prefix and then page by page, only the pages +/// in use, so a block whose size fields claim more than the file holds +/// costs no more than the pages it really has. +pub(crate) const PAGED_BLOCK_ONE_READ_MAX: usize = 1 << 20; + /// A window of the file: up to `max` bytes read at `base`, fewer only at /// the end of the file. Its [`Window::ensure`] reports a bounds failure /// exactly as the whole-file check `ensure_len(file_data, base + rel, n)` @@ -272,6 +278,28 @@ impl<'a> Window<'a> { } } + /// [`Window::ensure`] for a window at `base` that has not been read: + /// whether `[rel, rel + needed)` lies in the file, with the same error. + /// Lets a parser whose first step is to check a structure's whole extent + /// (a checksum at its end) fail before reading a structure that a + /// hostile size field has stretched past the end of the file. + pub fn check_extent( + file: &S, + base: u64, + rel: usize, + needed: usize, + ) -> Result<(), FormatError> { + let base = usize::try_from(base).unwrap_or(usize::MAX); + let file_len = len_usize(file); + match base.checked_add(rel).and_then(|p| p.checked_add(needed)) { + Some(end) if end <= file_len => Ok(()), + _ => Err(FormatError::UnexpectedEof { + expected: base.saturating_add(rel).saturating_add(needed), + available: file_len, + }), + } + } + /// Check that `[rel, rel + needed)` (relative to `base`) is in the file. #[inline] pub fn ensure(&self, rel: usize, needed: usize) -> Result<(), FormatError> { diff --git a/crates/clawhdf5-format/tests/storage_equivalence.rs b/crates/clawhdf5-format/tests/storage_equivalence.rs index 2f31a8b..5cc71b9 100644 --- a/crates/clawhdf5-format/tests/storage_equivalence.rs +++ b/crates/clawhdf5-format/tests/storage_equivalence.rs @@ -113,6 +113,10 @@ struct Tally { contiguous_required: usize, reads: u64, bytes: u64, + /// Chunk indexes (fixed and extensible arrays) read, and the most bytes + /// one of them took through the storage. + chunk_indexes: usize, + max_chunk_index_bytes: u64, } struct Walk<'a> { @@ -152,6 +156,12 @@ impl Walk<'_> { ); } + fn index_read(&mut self, bytes_before: u64) { + self.tally.chunk_indexes += 1; + let bytes = self.storage.bytes_read() - bytes_before; + self.tally.max_chunk_index_bytes = self.tally.max_chunk_index_bytes.max(bytes); + } + fn st(&self) -> &dyn Storage { self.storage } @@ -403,6 +413,7 @@ impl Walk<'_> { let Ok(h) = h else { return }; let want = read_fixed_array_chunks(slice, &h, &ds.dimensions, max, dims, es, os, ls); + let before = self.storage.bytes_read(); let got = read_fixed_array_chunks_in( self.st(), &h, @@ -413,6 +424,7 @@ impl Walk<'_> { os, ls, ); + self.index_read(before); self.same("fixed array chunks", &want, &got); } else { let h = ExtensibleArrayHeader::parse(slice, *addr as usize, os, ls); @@ -429,6 +441,7 @@ impl Walk<'_> { os, ls, ); + let before = self.storage.bytes_read(); let got = read_extensible_array_chunks_in( self.st(), &h, @@ -439,6 +452,7 @@ impl Walk<'_> { os, ls, ); + self.index_read(before); self.same("extensible array chunks", &want, &got); } } @@ -451,7 +465,11 @@ fn check_file(path: &Path, tally: &mut Tally) { let Ok(bytes) = std::fs::read(path) else { return; }; - let Ok((_, hdf5)) = split_user_block(&bytes) else { + check_bytes(&path.display().to_string(), &bytes, tally); +} + +fn check_bytes(name: &str, bytes: &[u8], tally: &mut Tally) { + let Ok((_, hdf5)) = split_user_block(bytes) else { return; }; let storage = CountingStorage::new(hdf5.to_vec()); @@ -459,7 +477,7 @@ fn check_file(path: &Path, tally: &mut Tally) { let mut walk = Walk { slice: hdf5, storage: &storage, - name: path.display().to_string(), + name: name.to_string(), tally, }; walk.run(); @@ -473,7 +491,7 @@ fn check_file(path: &Path, tally: &mut Tally) { tally.checks - before.0, storage.reads(), storage.bytes_read(), - path.display() + name ); } } @@ -581,6 +599,16 @@ with h5py.File(p('v1_groups.h5'), 'w', libver='earliest', userblock_size=512) as g.attrs['n'] = i f.create_dataset('x', data=np.arange(10)) +# Paged chunk indexes whose data blocks are bigger than the storage reads +# in one piece (1 MiB), with two chunks written: a fixed array of 300 000 +# chunks (a 2.4 MB data block) and an extensible array grown to 1.2e9 (its +# last data block holds 131 072 chunks: over 1 MiB). +with h5py.File(p('big_paged.h5'), 'w', libver='latest') as f: + d = f.create_dataset('fa', shape=(300000,), chunks=(1,), dtype='u1') + d[5] = 1; d[250000] = 2 + e = f.create_dataset('ea', shape=(1,), maxshape=(None,), chunks=(1,), dtype='u1') + e.resize((1200000000,)); e[10] = 1; e[1100000000] = 3 + libs = glob.glob(os.path.join(os.path.dirname(h5py.__file__), '..', 'h5py.libs', 'libhdf5-*.so*')) if libs: lib = ctypes.CDLL(libs[0]) @@ -633,7 +661,7 @@ fn h5py_files_parse_identically_through_storage() { .iter() .map(|f| f.file_name().unwrap().to_string_lossy().into_owned()) .collect(); - for want in ["ea.h5", "v1_groups.h5"] { + for want in ["ea.h5", "v1_groups.h5", "big_paged.h5"] { assert!(names.iter().any(|n| n == want), "{names:?}"); } if interop_required() { @@ -641,6 +669,29 @@ fn h5py_files_parse_identically_through_storage() { } let mut tally = Tally::default(); for f in &files { + if f.ends_with("big_paged.h5") { + // Only the pages in use are read, not the whole data blocks: + // read in one piece, the fixed array took 2.4 MB and the + // extensible array 1.2 MB (its super block's page bitmap and + // block addresses are most of what remains). + let mut big = Tally::default(); + check_file(f, &mut big); + eprintln!("big paged blocks: {big:?}"); + assert_eq!(big.chunk_indexes, 2, "{big:?}"); + assert!(big.max_chunk_index_bytes < 256 << 10, "{big:?}"); + // Truncated anywhere, the page-by-page reads still agree with + // the slice reads (errors included). + let bytes = std::fs::read(f).unwrap(); + for cut in (0..bytes.len()).step_by(bytes.len() / 97) { + check_bytes( + &format!("big_paged.h5 cut at {cut}"), + &bytes[..cut], + &mut big, + ); + } + eprintln!("big paged blocks, truncated: {big:?}"); + assert!(big.chunk_indexes > 100, "{big:?}"); + } check_file(f, &mut tally); } eprintln!("h5py files: {tally:?}");