format: bound and batch every chunk fetch over Storage

Only the full read split its chunk fetches into 64 MiB batches. The selection path, the indexed read and the parallel_read decoders fetched every chunk's stored bytes in one read_ranges call, each extent bounded only by the file length, so a crafted chunk index pointing many chunks at one large extent made File::open_storage hold chunks x extent bytes (3.3 GB from a 16.8 MB file) before the first decode error. - storage::for_each_extent_batch is now the one way raw-data reads fetch chunk bytes: batches of at most RAW_BATCH_BYTES (now pub), each decoded before the next is fetched. Used by the full, cached, indexed, selection and parallel_read paths; the sweep read uses read_extent per chunk. - ExtentReq carries each chunk's claimed extent (bounds-checked as before, same errors) and the prefix actually fetched: filters::stored_chunk_limit — the chunk size if unfiltered, else each applied filter's worst-case growth (n + n/4 + 4096 per codec; unbounded only for an application-registered codec). The in-memory path cuts the slice it decodes the same way, so both paths still agree. - tests/raw_fetch_bounds.rs: a crafted chunked_large.h5 (ten chunks all claiming 20 MiB at one padding blob) read through every path over a storage that records the largest single fetch; and 160 MiB of legitimate unfiltered chunks fetched batch by batch. Before: one 80 MiB fetch (selection) and one 160 MiB fetch; after: within the budget. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
2026-09-26 18:30:30 -05:00
parent f191dc09d5
commit 7d629f49e3
10 changed files with 825 additions and 285 deletions
@@ -337,14 +337,57 @@ pub fn read_upto<S: Storage + ?Sized>(
 }

 /// Most stored bytes fetched by one [`Storage::read_ranges`] call when a
-/// read gathers many extents (a chunked dataset's chunks): a larger read is
-/// fetched and decoded batch by batch, so a remote backend never holds more
-/// than this much undecoded data per read.
-pub(crate) const RAW_BATCH_BYTES: usize = 64 << 20;
+/// read gathers many extents (a chunked dataset's chunks, a selection's
+/// runs): a larger read is fetched and decoded batch by batch, so a backend
+/// without the file in memory never holds more than this much undecoded
+/// data per read (or one extent, when a single one is larger — and every
+/// chunk's extent is bounded by what the chunk can need, see
+/// [`crate::filters::stored_chunk_limit`]).
+pub const RAW_BATCH_BYTES: usize = 64 << 20;

-/// The stored bytes of a list of extents (chunks, contiguous runs), fetched
-/// together: [`Storage::read_ranges`] is called once for all of them, so a
-/// remote backend can coalesce and parallelise the requests.
+/// One extent of a raw-data read: `len` bytes stored at `addr`, whose
+/// bounds are checked against the file, of which the first `fetch` bytes
+/// are read (`None`: only checked, not read — its bytes are not needed).
+///
+/// `fetch` below `len` bounds what a crafted size field can make a read
+/// fetch: a chunk never needs more of its stored bytes than its decoded
+/// size allows, however large its index entry says it is.
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct ExtentReq {
+    pub addr: u64,
+    pub len: usize,
+    pub fetch: Option<usize>,
+}
+
+impl ExtentReq {
+    /// How many bytes are read for this extent.
+    #[inline]
+    fn fetch_len(&self) -> usize {
+        self.fetch.map_or(0, |f| f.min(self.len))
+    }
+}
+
+/// One extent's bytes on their own (see [`ExtentReq`]): the whole extent's
+/// bounds checked as [`read_exact_at`] checks them, and its first
+/// `req.fetch` bytes read (none when `fetch` is `None`).
+pub(crate) fn read_extent<'a, S: Storage + ?Sized>(
+    file: &'a S,
+    req: &ExtentReq,
+) -> Result<Cow<'a, [u8]>, FormatError> {
+    let start = usize::try_from(req.addr).unwrap_or(usize::MAX);
+    match start.checked_add(req.len) {
+        Some(end) if end <= len_usize(file) => read_exact_at(file, req.addr, req.fetch_len()),
+        _ => Err(FormatError::UnexpectedEof {
+            expected: start.saturating_add(req.len),
+            available: len_usize(file),
+        }),
+    }
+}
+
+/// The stored bytes of one batch of extents (chunks, contiguous runs),
+/// fetched together: [`Storage::read_ranges`] is called once for the batch,
+/// so a remote backend can coalesce and parallelise the requests. See
+/// [`for_each_extent_batch`], which is how every raw-data read gets them.
 ///
 /// With the whole file in memory nothing is fetched: [`Self::get`] slices
 /// it, as the slice readers did. Either way an extent that does not lie in
@@ -356,8 +399,12 @@ pub(crate) const RAW_BATCH_BYTES: usize = 64 << 20;
 pub(crate) enum ExtentBytes<'a> {
    /// The whole file.
    Contiguous(&'a [u8]),
-    /// Each extent's bytes, or its bounds error.
-    Fetched(Vec<Extent<'a>>),
+    /// Each extent's bytes, or its bounds error; the first is extent
+    /// `base` of the read.
+    Fetched {
+        base: usize,
+        extents: Vec<Extent<'a>>,
+    },
 }

 /// One extent of [`ExtentBytes::Fetched`].
@@ -371,33 +418,35 @@ pub(crate) enum Extent<'a> {
 }

 impl<'a> ExtentBytes<'a> {
-    /// Fetch `extents` (`(address, length, wanted)`): the bytes of those
-    /// `wanted`, and the bounds check of all of them.
-    pub(crate) fn fetch<S: Storage + ?Sized>(
+    /// Fetch `reqs`, extents `base..base + reqs.len()` of the read: the
+    /// bytes of those wanted, and the bounds check of all of them.
+    fn fetch<S: Storage + ?Sized>(
        file: &'a S,
-        extents: &[(u64, usize, bool)],
+        reqs: &[ExtentReq],
+        base: usize,
    ) -> Result<Self, FormatError> {
        if let Some(all) = file.as_contiguous() {
            return Ok(ExtentBytes::Contiguous(all));
        }
        let file_len = len_usize(file);
        let mut ranges = Vec::new();
-        let mut out = Vec::with_capacity(extents.len());
+        let mut out = Vec::with_capacity(reqs.len());
        // Positions in `out` of the extents being read, in `ranges` order.
        let mut slots = Vec::new();
-        for &(addr, len, wanted) in extents {
-            let checked =
-                crate::addr::to_usize(addr).and_then(|start| match start.checked_add(len) {
+        for req in reqs {
+            let checked = crate::addr::to_usize(req.addr).and_then(|start| {
+                match start.checked_add(req.len) {
                    Some(end) if end <= file_len => Ok(()),
                    _ => Err(FormatError::UnexpectedEof {
-                        expected: start.saturating_add(len),
+                        expected: start.saturating_add(req.len),
                        available: file_len,
                    }),
-                });
+                }
+            });
            match checked {
-                Ok(()) if wanted => {
+                Ok(()) if req.fetch.is_some() => {
                    slots.push(out.len());
-                    ranges.push(addr..addr + len as u64);
+                    ranges.push(req.addr..req.addr + req.fetch_len() as u64);
                    out.push(Extent::NotFetched);
                }
                Ok(()) => out.push(Extent::NotFetched),
@@ -418,41 +467,46 @@ impl<'a> ExtentBytes<'a> {
                out[slot] = Extent::Bytes(bytes);
            }
        }
-        Ok(ExtentBytes::Fetched(out))
+        Ok(ExtentBytes::Fetched { base, extents: out })
    }

-    /// Whether extent `i` (at `addr`, `len` bytes, as passed to
-    /// [`Self::fetch`]) lies in the file: its bounds error if not.
-    pub(crate) fn check(&self, i: usize, addr: u64, len: usize) -> Result<(), FormatError> {
+    /// Whether extent `i` of the read (`req`) lies in the file: its bounds
+    /// error if not.
+    pub(crate) fn check(&self, i: usize, req: &ExtentReq) -> Result<(), FormatError> {
        match self {
-            ExtentBytes::Contiguous(_) => self.get(i, addr, len).map(|_| ()),
-            ExtentBytes::Fetched(v) => match v.get(i) {
-                Some(Extent::Err(e)) => Err(e.clone()),
-                Some(_) => Ok(()),
-                None => Err(not_fetched()),
-            },
+            ExtentBytes::Contiguous(_) => self.get(i, req).map(|_| ()),
+            ExtentBytes::Fetched { base, extents } => {
+                match i.checked_sub(*base).and_then(|j| extents.get(j)) {
+                    Some(Extent::Err(e)) => Err(e.clone()),
+                    Some(_) => Ok(()),
+                    None => Err(not_fetched()),
+                }
+            }
        }
    }

-    /// Extent `i`'s bytes (at `addr`, `len` bytes, as passed to
-    /// [`Self::fetch`]).
-    pub(crate) fn get(&self, i: usize, addr: u64, len: usize) -> Result<&[u8], FormatError> {
+    /// Extent `i` of the read (`req`): its first `req.fetch` bytes, the
+    /// same whether the file is in memory or not.
+    pub(crate) fn get(&self, i: usize, req: &ExtentReq) -> Result<&[u8], FormatError> {
        match self {
            ExtentBytes::Contiguous(all) => {
-                let start = crate::addr::to_usize(addr)?;
+                let start = crate::addr::to_usize(req.addr)?;
                start
-                    .checked_add(len)
+                    .checked_add(req.len)
                    .and_then(|end| all.get(start..end))
+                    .map(|b| &b[..req.fetch_len()])
                    .ok_or(FormatError::UnexpectedEof {
-                        expected: start.saturating_add(len),
+                        expected: start.saturating_add(req.len),
                        available: <[u8]>::len(all),
                    })
            }
-            ExtentBytes::Fetched(v) => match v.get(i) {
-                Some(Extent::Bytes(b)) => Ok(b),
-                Some(Extent::Err(e)) => Err(e.clone()),
-                _ => Err(not_fetched()),
-            },
+            ExtentBytes::Fetched { base, extents } => {
+                match i.checked_sub(*base).and_then(|j| extents.get(j)) {
+                    Some(Extent::Bytes(b)) => Ok(b),
+                    Some(Extent::Err(e)) => Err(e.clone()),
+                    _ => Err(not_fetched()),
+                }
+            }
        }
    }
 }
@@ -462,6 +516,30 @@ fn not_fetched() -> FormatError {
    FormatError::Storage("an extent that was not fetched was asked for".into())
 }

+/// The one way raw-data reads fetch stored bytes: `reqs` are split into
+/// consecutive batches of at most [`RAW_BATCH_BYTES`] of fetched bytes (at
+/// least one extent each — and no extent fetches more than its
+/// [`ExtentReq::fetch`]), and for each batch in turn its bytes are fetched
+/// with one [`Storage::read_ranges`] call and `f(batch, &bytes)` is called,
+/// with `bytes` indexed by the extent's position in `reqs`. A batch's bytes
+/// are dropped before the next batch is fetched, and an error from `f`
+/// stops the read before anything more is fetched.
+///
+/// With the whole file in memory there is nothing to fetch: one call, over
+/// all of `reqs`, that slices the file.
+pub(crate) fn for_each_extent_batch<'a, S: Storage + ?Sized>(
+    file: &'a S,
+    reqs: &[ExtentReq],
+    mut f: impl FnMut(Range<usize>, &ExtentBytes<'a>) -> Result<(), FormatError>,
+) -> Result<(), FormatError> {
+    let contiguous = file.as_contiguous().is_some();
+    for batch in raw_batches(reqs.len(), contiguous, |i| reqs[i].fetch_len()) {
+        let bytes = ExtentBytes::fetch(file, &reqs[batch.clone()], batch.start)?;
+        f(batch, &bytes)?;
+    }
+    Ok(())
+}
+
 /// Split `n` extents, whose sizes `size(i)` gives, into consecutive batches
 /// of at most [`RAW_BATCH_BYTES`] (at least one extent each): the ranges of
 /// `0..n` to fetch together. With the whole file in memory (`contiguous`)