aep container: review fixes (#3275)

- read.rs: reserve with try_reserve_exact, so a raised chunk limit on a
  small target cannot abort before parsing; the capped reservation and a
  deep, retry-heavy nest of broken lists are pinned by tests.
- write.rs: a tree past the 32-bit container size fails with TooLarge
  before anything is serialised.
- aepx.rs: drop an unneeded local and lifetime annotation.

crates/aep/src/container/aepx.rs

@@ -122,7 +122,7 @@ struct Reader<'d> {
     trailer: Option<Vec<u8>>,
 }
 
-impl<'d> Reader<'d> {
+impl Reader<'_> {
     fn run(mut self) -> Result<AepFile<'static>, AepError> {
         if self.data.starts_with(&[0xef, 0xbb, 0xbf]) {
             self.pos = 3;
@@ -227,8 +227,7 @@ impl<'d> Reader<'d> {
         let at = self.pos;
         self.pos = self.pos.saturating_add(1);
         let name = self.name()?;
-        let data = self.data;
-        let mut attrs: Vec<((usize, usize), Cow<'d, [u8]>)> = Vec::new();
+        let mut attrs = Vec::new();
         let self_closing = loop {
             self.skip_ws();
             let rest = self.rest();
@@ -256,7 +255,7 @@ impl<'d> Reader<'d> {
             }
             let vstart = self.pos.saturating_add(1);
             let vend = self.find(vstart, &[quote]).ok_or(xml(vstart, "an attribute value is not closed"))?;
-            let raw = data.get(vstart..vend).unwrap_or(&[]);
+            let raw = self.data.get(vstart..vend).unwrap_or(&[]);
             if raw.contains(&b'<') {
                 return Err(xml(vstart, "`<` inside an attribute value"));
             }

crates/aep/src/container/read.rs

@@ -70,10 +70,10 @@ struct Parser<'a> {
 impl<'a> Parser<'a> {
     /// Parses the sibling chunks that exactly fill `start..end`.
     fn sequence(&mut self, start: usize, end: usize, depth: usize) -> Result<Vec<Chunk<'a>>, AepError> {
-        // Sized once: a well-formed sequence fills exactly this many slots. The count is bounded by
-        // the input (one per 8 bytes at most) and by the chunks the limit still allows.
-        let capacity = self.count_siblings(start, end).min(self.limits.max_chunks.saturating_sub(self.count));
-        let mut out = Vec::with_capacity(capacity);
+        // Sized once: a well-formed sequence fills exactly this many slots. `try_reserve` so a
+        // limit raised past what the target can allocate fails nothing here (the Vec then grows).
+        let mut out = Vec::new();
+        let _ = out.try_reserve_exact(self.capacity_for(start, end));
         let mut pos = start;
         while pos < end {
             let short = AepError::Malformed { offset: pos, what: "bytes left over that are too short for a chunk header" };
@@ -102,6 +102,12 @@ impl<'a> Parser<'a> {
         Ok(out)
     }
 
+    /// The slots to reserve for the sequence `start..end`: its header count, bounded by the input
+    /// (one per 8 bytes at most) and by the chunks the limit still allows.
+    fn capacity_for(&self, start: usize, end: usize) -> usize {
+        self.count_siblings(start, end).min(self.limits.max_chunks.saturating_sub(self.count))
+    }
+
     /// The number of chunk headers that tile `start..end` the way [`Self::sequence`] walks them,
     /// stopping at the first that does not fit. Never more than `(end - start) / 8`.
     fn count_siblings(&self, start: usize, end: usize) -> usize {
@@ -182,6 +188,8 @@ mod tests {
         let mut vecs = vec![&parsed.chunks];
         child_vecs(&parsed.chunks, &mut vecs);
         assert_eq!(vecs.len(), 11);
+        // `try_reserve_exact` gives exactly the requested capacity with std's allocator today; a
+        // Vec grown push by push would have rounded up to a power of two (37 -> 64, 9 -> 16).
         for v in vecs {
             assert_eq!(v.capacity(), v.len(), "a sibling Vec grew past its length");
         }
@@ -208,5 +216,53 @@ mod tests {
         // Under a tighter limit the parse fails as before, without sizing for more than allowed.
         let tight = Limits { max_depth: 64, max_chunks: 10 };
         assert_eq!(parse_with_limits(&data, tight), Err(AepError::TooManyChunks));
+        // The reservation never exceeds what the chunk limit still allows.
+        let mut parser = Parser { data: &data, limits: tight, count: 0 };
+        assert_eq!(parser.capacity_for(12, data.len()), 10);
+        parser.count = 7;
+        assert_eq!(parser.capacity_for(12, data.len()), 3);
+        parser.count = 11;
+        assert_eq!(parser.capacity_for(12, data.len()), 0);
+        let parser = Parser { data: &data, limits: Limits::default(), count: 0 };
+        assert_eq!(parser.capacity_for(12, data.len()), 1000);
+    }
+
+    /// `LIST Fold` around `inner`, followed by 6 bytes too short for a header, so the list's
+    /// sequence fails and the list is kept opaque.
+    fn broken_level(inner: &[u8]) -> Vec<u8> {
+        let mut body = b"Fold".to_vec();
+        body.extend_from_slice(inner);
+        body.extend_from_slice(&[0xee; 6]);
+        let mut out = b"LIST".to_vec();
+        out.extend_from_slice(&u32::try_from(body.len()).expect("small").to_be_bytes());
+        out.extend_from_slice(&body);
+        out
+    }
+
+    #[test]
+    fn nested_lists_with_broken_tails_parse_each_header_once() {
+        // A wide, well-formed list at the bottom, then 50 levels that each fail and fall back to
+        // opaque: every level re-parses its child before giving up.
+        let mut wide = b"Fold".to_vec();
+        wide.extend(std::iter::repeat_n(*b"abcd\0\0\0\0", 1000).flatten());
+        let mut level = b"LIST".to_vec();
+        level.extend_from_slice(&u32::try_from(wide.len()).expect("small").to_be_bytes());
+        level.extend_from_slice(&wide);
+        for _ in 0..50 {
+            level = broken_level(&level);
+        }
+        let mut data = b"RIFX\0\0\0\0Egg!".to_vec();
+        data.extend_from_slice(&level);
+        let declared = u32::try_from(data.len() - 8).expect("small");
+        data[4..8].copy_from_slice(&declared.to_be_bytes());
+        // 1 + 50 + 1000 headers in the file: the parse counts each exactly once (the count spans
+        // every attempt), so the work stays linear in the input.
+        let headers = 1 + 50 + 1000;
+        let f = parse_with_limits(&data, Limits { max_depth: 64, max_chunks: headers }).expect("parse");
+        assert_eq!(parse_with_limits(&data, Limits { max_depth: 64, max_chunks: headers - 1 }), Err(AepError::TooManyChunks));
+        // The outermost list keeps its bytes, and the file writes back unchanged.
+        let [top] = f.chunks.as_slice() else { panic!("one top-level chunk") };
+        assert!(matches!(&top.body, Body::OpaqueList { list_type, data } if list_type == b"Fold" && data.len() + 12 == level.len()));
+        assert_eq!(crate::container::write(&f).expect("write"), data);
     }
 }

crates/aep/src/container/write.rs

@@ -17,14 +17,17 @@ const MAX_WRITE_DEPTH: usize = 256;
 /// [`AepError::TooDeep`] for an absurdly deep hand-built tree.
 pub fn write(file: &AepFile<'_>) -> Result<Vec<u8>, AepError> {
     let mut out = Vec::new();
-    // Sized once to the exact output when the tree fits a container (a tree that does not fails
-    // below anyway). `try_reserve` so a hand-built tree that repeats one large borrowed body many
-    // times cannot abort on the allocation; the writer then grows as it goes, as before.
-    if let Some(len) = sequence_len(&file.chunks, 1)
-        && u32::try_from(len.saturating_add(4)).is_ok()
-        && let Some(total) = len.checked_add(12).and_then(|n| n.checked_add(file.trailer.len()))
-    {
-        let _ = out.try_reserve_exact(total);
+    // Sized once to the exact output. A tree past the 32-bit container size fails at once (the
+    // same error the write below would give after serialising it). `try_reserve` so a hand-built
+    // tree that repeats one large borrowed body many times cannot abort on the allocation; the
+    // writer then grows as it goes, as before.
+    if let Some(len) = sequence_len(&file.chunks, 1) {
+        if u32::try_from(len.saturating_add(4)).is_err() {
+            return Err(AepError::TooLarge);
+        }
+        if let Some(total) = len.checked_add(12).and_then(|n| n.checked_add(file.trailer.len())) {
+            let _ = out.try_reserve_exact(total);
+        }
     }
     out.extend_from_slice(b"RIFX");
     out.extend_from_slice(&[0; 4]);
@@ -143,10 +146,23 @@ mod tests {
         for file in &trees {
             let out = write(file).expect("write");
             assert_eq!(sequence_len(&file.chunks, 1).map(|n| n + 12 + file.trailer.len()), Some(out.len()));
+            // Exact with std's allocator today; grown push by push it would be rounded up.
             assert_eq!(out.capacity(), out.len(), "the output grew past its size");
         }
     }
 
+    #[cfg(target_pointer_width = "64")]
+    #[test]
+    fn a_tree_past_the_container_size_fails_before_serialising() {
+        // 4097 chunks borrowing one 1 MiB body: over 4 GiB of output, none of it written.
+        let body = vec![0u8; 1 << 20];
+        let chunks: Vec<Chunk<'_>> = (0..4097).map(|_| Chunk::data(*b"cdat", &body[..])).collect();
+        let file = AepFile { chunks, trailer: Cow::Borrowed(&[]) };
+        assert_eq!(write(&file), Err(AepError::TooLarge));
+        // Just under the limit still sizes, and fails nothing up front.
+        assert!(sequence_len(&file.chunks[..4000], 1).is_some_and(|n| u32::try_from(n + 4).is_ok()));
+    }
+
     #[test]
     fn a_tree_past_the_depth_limit_has_no_size() {
         let mut deep = Chunk::data(*b"cdat", vec![0]);