format: read fixed array chunk indexes over Storage

FixedArrayHeader::parse_in reads the header in one read, and
read_fixed_array_chunks_in reads the data block's prefix and then the
whole block, paged or not, as one window; checksums and elements are
checked in it, with bounds errors reported as the whole-file checks did
(also in builds without the checksum feature, where the per-element
checks are the only ones). The open-ended &file_data[offset..] slices are
gone. The &[u8] functions are wrappers. New test: non-paged and paged,
filtered and unfiltered arrays, cut through the data block and with
damaged bytes, read identically through a read_at-only CountingStorage.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

crates/clawhdf5-format/src/fixed_array.rs

@@ -9,16 +9,19 @@ 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};
 
 /// Verify the Jenkins lookup3 checksum stored immediately after
-/// `data[start..end]`, as every Fixed Array structure carries one.
+/// `data[start..end]`, as every Fixed Array structure carries one. `w` is
+/// a window of the file and `start`/`end` are relative to it.
 ///
 /// A corrupt chunk index silently yields addresses pointing at the wrong
 /// bytes, so a mismatch has to be an error rather than a shrug: without this
 /// the damage surfaces as plausible-looking data from the wrong chunk.
 #[cfg(feature = "checksum")]
-fn verify_checksum(data: &[u8], start: usize, end: usize) -> Result<(), FormatError> {
+fn verify_checksum(w: &Window<'_>, start: usize, end: usize) -> Result<(), FormatError> {
-    ensure_len(data, end, 4)?;
+    w.ensure(end, 4)?;
+    let data = &w.bytes;
     let stored = u32::from_le_bytes([data[end], data[end + 1], data[end + 2], data[end + 3]]);
     let computed = crate::checksum::jenkins_lookup3(&data[start..end]);
     if computed != stored {
@@ -31,7 +34,7 @@ fn verify_checksum(data: &[u8], start: usize, end: usize) -> Result<(), FormatEr
 }
 
 #[cfg(not(feature = "checksum"))]
-fn verify_checksum(_data: &[u8], _start: usize, _end: usize) -> Result<(), FormatError> {
+fn verify_checksum(_w: &Window<'_>, _start: usize, _end: usize) -> Result<(), FormatError> {
     Ok(())
 }
 
@@ -73,19 +76,6 @@ fn read_length(data: &[u8], pos: usize, size: u8) -> Result<u64, FormatError> {
     read_offset(data, pos, size)
 }
 
-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 is_undefined(data: &[u8], pos: usize, size: u8) -> bool {
     let s = size as usize;
     if pos + s > data.len() {
@@ -101,13 +91,24 @@ impl FixedArrayHeader {
         offset: usize,
         offset_size: u8,
         length_size: u8,
+    ) -> Result<Self, FormatError> {
+        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<Self, FormatError> {
         // FAHD signature(4) + version(1) + client_id(1) + element_size(1) +
         // max_nelmts_bits(1) + num_elements(length_size) + data_block_addr(offset_size) + checksum(4)
         let min_size = 4 + 1 + 1 + 1 + 1 + length_size as usize + offset_size as usize + 4;
-        ensure_len(file_data, offset, min_size)?;
+        let w = Window::read(file, offset, min_size)?;
+        w.ensure(0, min_size)?;
 
-        let d = &file_data[offset..];
+        let d: &[u8] = &w.bytes;
         if &d[0..4] != b"FAHD" {
             return Err(FormatError::ChunkedReadError(
                 "invalid Fixed Array header signature".into(),
@@ -130,7 +131,7 @@ impl FixedArrayHeader {
         pos += length_size as usize;
         let data_block_address = read_offset(d, pos, offset_size)?;
         pos += offset_size as usize;
-        verify_checksum(file_data, offset, offset + pos)?;
+        verify_checksum(&w, 0, pos)?;
 
         Ok(FixedArrayHeader {
             client_id,
@@ -156,15 +157,39 @@ pub fn read_fixed_array_chunks(
     chunk_dimensions: &[u32],
     element_size: u32,
     offset_size: u8,
+    length_size: u8,
+) -> Result<Vec<ChunkInfo>, FormatError> {
+    read_fixed_array_chunks_in(
+        &file_data,
+        header,
+        dataset_dims,
+        max_dims,
+        chunk_dimensions,
+        element_size,
+        offset_size,
+        length_size,
+    )
+}
+
+/// [`read_fixed_array_chunks`] over any [`Storage`]: one read of the data
+/// block's prefix, one of the whole data block (pages included).
+#[allow(clippy::too_many_arguments)]
+pub fn read_fixed_array_chunks_in(
+    file: &dyn Storage,
+    header: &FixedArrayHeader,
+    dataset_dims: &[u64],
+    max_dims: Option<&[u64]>,
+    chunk_dimensions: &[u32],
+    element_size: u32,
+    offset_size: u8,
     _length_size: u8,
 ) -> Result<Vec<ChunkInfo>, FormatError> {
+    let file_len = len_usize(file);
     let db_offset = header.data_block_address as usize;
 
     // Parse data block header: FADB(4) + version(1) + client_id(1) + header_address(offset_size)
     let db_header_size = 4 + 1 + 1 + offset_size as usize;
-    ensure_len(file_data, db_offset, db_header_size)?;
+    let d = read_exact_at(file, db_offset as u64, db_header_size)?;
-
-    let d = &file_data[db_offset..];
     if &d[0..4] != b"FADB" {
         return Err(FormatError::ChunkedReadError(
             "invalid Fixed Array data block signature".into(),
@@ -178,7 +203,7 @@ pub fn read_fixed_array_chunks(
     // A chunk index cannot describe more elements than the file has bytes (each
     // element occupies at least `offset_size` bytes). Reject a corrupt count
     // before it can drive a huge loop or overflow an offset computation.
-    if num_elements > file_data.len() {
+    if num_elements > file_len {
         return Err(FormatError::ChunkedReadError(
             "Fixed Array element count exceeds file size".into(),
         ));
@@ -208,30 +233,34 @@ pub fn read_fixed_array_chunks(
         chunk_dimensions.iter().map(|&d| d as u64).product::<u64>() * element_size as u64;
 
     let mut chunks = Vec::new();
-    let push_element =
+    // `rel` is relative to the data block, whose bytes are in `w`.
-        |i: usize, abs: usize, chunks: &mut Vec<ChunkInfo>| -> Result<(), FormatError> {
+    let push_element = |w: &Window<'_>,
-            if let Some((address, chunk_size, filter_mask)) = parse_fa_element(
+                        i: usize,
-                file_data,
+                        rel: usize,
-                abs,
+                        chunks: &mut Vec<ChunkInfo>|
-                header.client_id,
+     -> Result<(), FormatError> {
-                offset_size,
+        if let Some((address, chunk_size, filter_mask)) = parse_fa_element(
-                header.element_size,
+            w,
-                chunk_byte_size,
+            rel,
-            )? {
+            header.client_id,
-                // A slot beyond the current extent is ignored, as the
+            offset_size,
-                // library does.
+            header.element_size,
-                let Some(offsets) = grid.offsets(i as u64) else {
+            chunk_byte_size,
-                    return Ok(());
+        )? {
-                };
+            // A slot beyond the current extent is ignored, as the
-                chunks.push(ChunkInfo {
+            // library does.
-                    chunk_size,
+            let Some(offsets) = grid.offsets(i as u64) else {
-                    filter_mask,
+                return Ok(());
-                    offsets,
+            };
-                    address,
+            chunks.push(ChunkInfo {
-                });
+                chunk_size,
-            }
+                filter_mask,
-            Ok(())
+                offsets,
-        };
+                address,
+            });
+        }
+        Ok(())
+    };
 
     // A data block is paged when it holds more elements than fit in one page.
     // `max_nelmts_bits` is an untrusted u8; a shift >= the pointer width would
@@ -246,10 +275,13 @@ pub fn read_fixed_array_chunks(
 
     if !is_paged {
         // Non-paged: prefix, then `num_elements` elements packed directly,
-        // then a checksum over both.
+        // then a checksum over both. One window holds all of it (or ends at
-        verify_checksum(file_data, db_offset, elem_at(elements_start, num_elements)?)?;
+        // the end of the file), so its bounds checks are the whole-file ones.
+        let end = elem_at(elements_start, num_elements)?;
+        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 {
-            push_element(i, elem_at(elements_start, i)?, &mut chunks)?;
+            push_element(&w, i, elem_at(elements_start, i)? - db_offset, &mut chunks)?;
         }
         return Ok(chunks);
     }
@@ -272,22 +304,27 @@ pub fn read_fixed_array_chunks(
         .and_then(|x| x.checked_add(4))
         .ok_or_else(stride_overflow)?;
 
-    if bitmap_start + bitmap_size > file_data.len() {
+    if bitmap_start + bitmap_size > file_len {
         return Err(FormatError::UnexpectedEof {
             expected: bitmap_start + bitmap_size,
-            available: file_data.len(),
+            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).
+    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)?;
     // The prefix and page bitmap are covered by their own checksum, and each
     // initialised page by one of its own.
-    verify_checksum(file_data, db_offset, bitmap_start + bitmap_size)?;
+    verify_checksum(&w, 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 = file_data[bitmap_start + p / 8];
+        let bit_byte = w.bytes[bitmap_start + p / 8 - db_offset];
         let bit_mask = 1u8 << (7 - (p % 8));
         if bit_byte & bit_mask == 0 {
             continue; // entire page unallocated
@@ -297,21 +334,30 @@ pub fn read_fixed_array_chunks(
             .checked_mul(page_stride)
             .and_then(|o| pages_start.checked_add(o))
             .ok_or_else(stride_overflow)?;
-        verify_checksum(file_data, page_off, elem_at(page_off, page_count)?)?;
+        verify_checksum(
+            &w,
+            page_off - db_offset,
+            elem_at(page_off, page_count)? - db_offset,
+        )?;
         for e in 0..page_count {
-            push_element(page_first + e, elem_at(page_off, e)?, &mut chunks)?;
+            push_element(
+                &w,
+                page_first + e,
+                elem_at(page_off, e)? - db_offset,
+                &mut chunks,
+            )?;
         }
     }
 
     Ok(chunks)
 }
 
-/// Parse a single Fixed Array element at absolute file offset `abs`.
+/// Parse a single Fixed Array element at offset `abs` of the window `w`.
 ///
 /// Returns `Some((address, chunk_size, filter_mask))` for an allocated chunk, or
 /// `None` if the element is undefined (an unallocated chunk, address all-`0xFF`).
 fn parse_fa_element(
-    file_data: &[u8],
+    w: &Window<'_>,
     abs: usize,
     client_id: u8,
     offset_size: u8,
@@ -321,12 +367,8 @@ fn parse_fa_element(
     let os = offset_size as usize;
     if client_id == 0 {
         // Non-filtered: element is just the chunk address.
-        if abs + os > file_data.len() {
+        w.ensure(abs, os)?;
-            return Err(FormatError::UnexpectedEof {
+        let file_data: &[u8] = &w.bytes;
-                expected: abs + os,
-                available: file_data.len(),
-            });
-        }
         if is_undefined(file_data, abs, offset_size) {
             return Ok(None);
         }
@@ -341,17 +383,14 @@ fn parse_fa_element(
             ));
         }
         let chunk_size_bytes = es - os - 4;
-        if abs + es > file_data.len() {
+        w.ensure(abs, es)?;
-            return Err(FormatError::UnexpectedEof {
+        let file_data: &[u8] = &w.bytes;
-                expected: abs + es,
-                available: file_data.len(),
-            });
-        }
         if is_undefined(file_data, abs, offset_size) {
             return Ok(None);
         }
         let address = read_offset(file_data, abs, offset_size)?;
-        let chunk_size = read_variable_length(&file_data[abs + os..], chunk_size_bytes)?;
+        let chunk_size =
+            read_variable_length(&file_data[abs + os..abs + es - 4], chunk_size_bytes)?;
         let fm_off = abs + os + chunk_size_bytes;
         let filter_mask = u32::from_le_bytes([
             file_data[fm_off],
@@ -813,4 +852,101 @@ mod tests {
             .collect();
         assert_eq!(got, expect);
     }
+
+    /// A fixed array (header at 0x100, data block at 0x200) of `n` chunks,
+    /// filtered or not, paged when `n` exceeds `1 << page_bits`; every
+    /// page initialised except page 1.
+    fn build_fixed_array(n: usize, filtered: bool, page_bits: u8) -> Vec<u8> {
+        let os = 8usize;
+        let es = if filtered { os + 4 + 4 } else { os };
+        let (fahd, db) = (0x100usize, 0x200usize);
+        let mut f = vec![0u8; 0x2000];
+        f[fahd..fahd + 4].copy_from_slice(b"FAHD");
+        f[fahd + 5] = u8::from(filtered);
+        f[fahd + 6] = es as u8;
+        f[fahd + 7] = page_bits;
+        f[fahd + 8..fahd + 16].copy_from_slice(&(n as u64).to_le_bytes());
+        f[fahd + 16..fahd + 24].copy_from_slice(&(db as u64).to_le_bytes());
+        stamp_checksum(&mut f, fahd, fahd + 24);
+        f[db..db + 4].copy_from_slice(b"FADB");
+        f[db + 5] = u8::from(filtered);
+        f[db + 6..db + 14].copy_from_slice(&(fahd as u64).to_le_bytes());
+        let elems = db + 6 + os;
+        let write = |f: &mut Vec<u8>, at: usize, i: usize| {
+            let addr = if i == 2 {
+                u64::MAX
+            } else {
+                0x1000 + i as u64 * 0x100
+            };
+            f[at..at + os].copy_from_slice(&addr.to_le_bytes());
+            if filtered {
+                f[at + os..at + os + 4].copy_from_slice(&(100 + i as u32).to_le_bytes());
+                f[at + os + 4..at + os + 8].copy_from_slice(&(i as u32 & 1).to_le_bytes());
+            }
+        };
+        let page = 1usize << page_bits;
+        if n <= page {
+            for i in 0..n {
+                write(&mut f, elems + i * es, i);
+            }
+            stamp_checksum(&mut f, db, elems + n * es);
+        } else {
+            let npages = n.div_ceil(page);
+            let bitmap = npages.div_ceil(8);
+            for p in 0..npages {
+                if p != 1 {
+                    f[elems + p / 8] |= 0x80 >> (p % 8);
+                }
+            }
+            stamp_checksum(&mut f, db, elems + bitmap);
+            let pages_start = elems + bitmap + 4;
+            for p in (0..npages).filter(|&p| p != 1) {
+                let at = pages_start + p * (page * es + 4);
+                let count = page.min(n - p * page);
+                for e in 0..count {
+                    write(&mut f, at + e * es, p * page + e);
+                }
+                stamp_checksum(&mut f, at, at + count * es);
+            }
+        }
+        f
+    }
+
+    /// Non-paged and paged, filtered and unfiltered arrays, cut at every
+    /// length through the data block and with a damaged byte, read
+    /// identically through a `read_at`-only storage.
+    #[test]
+    fn storage_reads_match_slice_reads() {
+        use crate::storage::CountingStorage;
+        for (n, filtered, bits) in [(3, false, 10), (3, true, 10), (11, false, 2), (11, true, 2)] {
+            let full = build_fixed_array(n, filtered, bits);
+            let es = if filtered { 16 } else { 8 };
+            let dims = [n as u64 * 20];
+            let h = FixedArrayHeader::parse(&full, 0x100, 8, 8).unwrap();
+            let chunks = read_fixed_array_chunks(&full, &h, &dims, None, &[20], 8, 8, 8).unwrap();
+            // Chunk 2 is unallocated, and so is page 1 of a paged array.
+            let expect = if n > 4 { n - 1 - 4 } else { n - 1 };
+            assert_eq!(chunks.len(), expect);
+            let mut files = Vec::new();
+            for cut in (0x100..0x200 + 40 + n * (es + 4) + 16).step_by(3) {
+                files.push(full[..cut].to_vec());
+            }
+            for at in [0x104, 0x210, 0x21a, 0x230] {
+                let mut damaged = full.clone();
+                damaged[at] ^= 1;
+                files.push(damaged);
+            }
+            files.push(full);
+            for f in files {
+                let storage = CountingStorage::new(f.clone());
+                let want = FixedArrayHeader::parse(&f, 0x100, 8, 8);
+                let got = FixedArrayHeader::parse_in(&storage, 0x100, 8, 8);
+                assert_eq!(format!("{got:?}"), format!("{want:?}"));
+                let Ok(h) = want else { continue };
+                let want = read_fixed_array_chunks(&f, &h, &dims, None, &[20], 8, 8, 8);
+                let got = read_fixed_array_chunks_in(&storage, &h, &dims, None, &[20], 8, 8, 8);
+                assert_eq!(format!("{got:?}"), format!("{want:?}"), "{} bytes", f.len());
+            }
+        }
+    }
 }