From 6dcaea2ac6ab079542e1f6ca486eb832dd5fe393 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 8 Nov 2026 18:49:19 +0100 Subject: [PATCH] #3300: read_aepx keeps its own owned-tree reader read_aepx, read_aepx_with_limits and parse_any build the owned chunk tree element by element again, as before the .aepx index, instead of converting the index (which held the decoded buffer and the owned tree at the same time). Import keeps the index reader. The tests now compare the two production readers. --- crates/aep/src/container.rs | 2 - crates/aep/src/container/aepx.rs | 80 ++++++++++--------- .../container/aepx/{reference.rs => tree.rs} | 10 +-- crates/aep/src/container/index.rs | 17 ++-- crates/aep/src/import.rs | 8 +- docs/aep-format.md | 10 +-- 6 files changed, 61 insertions(+), 66 deletions(-) rename crates/aep/src/container/aepx/{reference.rs => tree.rs} (97%) diff --git a/crates/aep/src/container.rs b/crates/aep/src/container.rs index 3701647c4..35f6f5954 100644 --- a/crates/aep/src/container.rs +++ b/crates/aep/src/container.rs @@ -37,8 +37,6 @@ mod values; mod version; mod write; -#[cfg(test)] -pub(crate) use aepx::reference as aepx_reference; pub(crate) use aepx::{AepxDrafts, index_aepx}; pub use aepx::{is_aepx, parse_any, read_aepx, read_aepx_with_limits}; pub use arbitrary::{ diff --git a/crates/aep/src/container/aepx.rs b/crates/aep/src/container/aepx.rs index 99c969011..c4d2865ff 100644 --- a/crates/aep/src/container/aepx.rs +++ b/crates/aep/src/container/aepx.rs @@ -13,22 +13,21 @@ //! recursion; the nesting depth and the element count are bounded by [`Limits`], and everything //! it allocates is bounded by the size of the input. //! -//! It does not build the owned tree element by element. It decodes every body (hex, text, the -//! alias JSON, the numbers) into one buffer and records the chunks as entries of the flat index -//! ([`Draft`]s, in depth-first order, bodies as ranges of the buffer). The importer indexes that -//! buffer ([`TreeIndex::from_drafts`]) and reads it like an `.aep`; [`read_aepx`] converts the -//! index into the owned tree. The reader this replaced is kept in the tests as the reference -//! (`aepx/reference.rs`): both give the same tree and the same errors. +//! There are two readers. [`read_aepx`] builds the owned tree element by element +//! (`aepx/tree.rs`). The importer, which only reads, uses `index_aepx`: it decodes every body +//! (hex, text, the alias JSON, the numbers) into one buffer and records the chunks as entries of +//! the flat index ([`Draft`]s, in depth-first order, bodies as ranges of the buffer), which +//! [`crate::container::TreeIndex::from_drafts`] turns into an index that borrows the buffer. +//! The tests hold both to the same tree and the same errors. use std::borrow::Cow; use crate::container::chunk::{AepFile, LIST, NESTED_IDS}; use crate::container::error::AepError; -use crate::container::index::{Draft, Kind as EntryKind, TreeIndex}; +use crate::container::index::{Draft, Kind as EntryKind}; use crate::container::read::{Limits, parse}; -#[cfg(test)] -pub(crate) mod reference; +mod tree; /// The root element of an `.aepx`. const ROOT: &[u8] = b"AfterEffectsProject"; @@ -77,18 +76,14 @@ pub fn read_aepx(data: &[u8]) -> Result, AepError> { /// element's children are depth 1), `max_chunks` the number of elements. /// /// # Errors -/// See [`read_aepx`]. Two errors are only reachable when memory runs out, where the reader -/// before the flat index would abort or could have succeeded: [`AepError::TooLarge`] when the -/// chunk list cannot grow, and [`AepError::Xml`] "bdata is too large to hold in memory" when the -/// shared body buffer cannot grow. +/// See [`read_aepx`]. pub fn read_aepx_with_limits(data: &[u8], limits: Limits) -> Result, AepError> { - let AepxDrafts { buffer, nodes, trailer } = index_aepx(data, limits)?; - Ok(TreeIndex::from_drafts(&buffer, nodes, trailer).to_owned_file()) + tree::read(data, limits) } /// An `.aepx` read for the flat index: every body decoded into one buffer, and the chunks as /// [`Draft`]s in depth-first order with their bodies as ranges of that buffer. -/// [`TreeIndex::from_drafts`] makes the index, which borrows the buffer. +/// [`crate::container::TreeIndex::from_drafts`] makes the index, which borrows the buffer. pub(crate) struct AepxDrafts { /// The decoded bodies (hex, text, alias JSON, numbers) and the XMP packet. pub(crate) buffer: Vec, @@ -101,7 +96,10 @@ pub(crate) struct AepxDrafts { /// errors, without one allocation per chunk. /// /// # Errors -/// See [`read_aepx_with_limits`]. +/// See [`read_aepx_with_limits`]. When memory runs out, two errors come up in more cases than +/// there: [`AepError::TooLarge`] when the chunk list cannot grow (the owned-tree reader would +/// abort), and [`AepError::Xml`] "bdata is too large to hold in memory" also when the shared body +/// buffer cannot grow (the owned-tree reader reserves each value on its own). pub(crate) fn index_aepx(data: &[u8], limits: Limits) -> Result { let text = match std::str::from_utf8(data) { Ok(t) => t.as_bytes(), @@ -120,7 +118,8 @@ pub(crate) fn index_aepx(data: &[u8], limits: Limits) -> Result AepError { @@ -604,7 +603,6 @@ fn decode_entities(raw: &[u8], offset: usize, out: &mut Vec) -> Result<(), A } /// Decodes a `bdata` value (hex digits, two per byte) into a new `Vec` of exactly its size. -#[cfg(test)] fn decode_hex(text: &[u8], offset: usize) -> Result, AepError> { decode_hex_at_most(text, offset, MAX_BDATA) } @@ -614,13 +612,13 @@ fn decode_hex(text: &[u8], offset: usize) -> Result, AepError> { const MAX_BDATA: usize = u32::MAX as usize; /// [`decode_hex`] with the decoded-size limit as a parameter (so tests can reach it). -#[cfg(test)] fn decode_hex_at_most(text: &[u8], offset: usize, max: usize) -> Result, AepError> { - // Checked before anything is reserved, then sized once. + // Checked before anything is reserved, then sized once (collecting into `Option` has no + // size hint and regrows step by step), and without aborting when the memory is not there. let pairs = check_hex(text, offset, max)?; let mut out = Vec::new(); out.try_reserve_exact(pairs).map_err(|_| xml(offset, "bdata is too large to hold in memory"))?; - decode_hex_into(text, offset, max, &mut out)?; + decode_checked(text, offset, &mut out)?; Ok(out) } @@ -628,15 +626,21 @@ fn decode_hex_at_most(text: &[u8], offset: usize, max: usize) -> Result, /// On an error `out` is unchanged. fn decode_hex_into(text: &[u8], offset: usize, max: usize, out: &mut Vec) -> Result<(), AepError> { let pairs = check_hex(text, offset, max)?; + // Grown at most once per value (amortised over a shared buffer), and without aborting when the + // memory is not there. + out.try_reserve(pairs).map_err(|_| xml(offset, "bdata is too large to hold in memory"))?; + decode_checked(text, offset, out) +} + +/// Appends the bytes of a `bdata` value that [`check_hex`] accepted. On an error (a digit the +/// check let through, which it does not) `out` is unchanged. +fn decode_checked(text: &[u8], offset: usize, out: &mut Vec) -> Result<(), AepError> { let digit = |b: u8| match b { b'0'..=b'9' => Some(b - b'0'), b'a'..=b'f' => Some(b - b'a' + 10), b'A'..=b'F' => Some(b - b'A' + 10), _ => None, }; - // Grown at most once per value (amortised over a shared buffer), and without aborting when the - // memory is not there. - out.try_reserve(pairs).map_err(|_| xml(offset, "bdata is too large to hold in memory"))?; let mark = out.len(); for [hi, lo] in text.as_chunks::<2>().0 { match (digit(*hi), digit(*lo)) { @@ -713,6 +717,7 @@ fn json_str(out: &mut String, s: &str) { #[cfg(test)] mod tests { use super::*; + use crate::container::index::TreeIndex; fn doc(body: &str) -> String { format!("\n{body}\n") @@ -1035,11 +1040,11 @@ mod tests { ] } - /// The reader and the reference give the same `Result` (the tree, or the error), and the index - /// of the reader is the index of the reference's tree. + /// The index reader and the owned-tree reader give the same `Result` (the tree, or the error), + /// and the index walks like the index of the owned tree. fn same(bytes: &[u8], limits: Limits) { - let old = reference::read(bytes, limits); - let new = read_aepx_with_limits(bytes, limits); + let old = read_aepx_with_limits(bytes, limits); + let new = index_aepx(bytes, limits).map(|d| TreeIndex::from_drafts(&d.buffer, d.nodes, d.trailer).to_file().into_owned()); assert_eq!(new, old, "{}", String::from_utf8_lossy(bytes)); if let Ok(file) = &old { let AepxDrafts { buffer, nodes, trailer } = index_aepx(bytes, limits).unwrap(); @@ -1053,9 +1058,9 @@ mod tests { } } - /// The reader gives the reference's tree and errors on generated and damaged documents. + /// The two readers give the same tree and errors on generated and damaged documents. #[test] - fn the_index_reader_matches_the_reference_on_generated_and_damaged_documents() { + fn the_two_readers_agree_on_generated_and_damaged_documents() { let mut rng = Rng(0x9e37_79b9_7f4a_7c15); let noise = b"<>/=\"'&;#x0aZ \n![]-?"; for _ in 0..600 { @@ -1154,9 +1159,9 @@ mod tests { same(deep.as_bytes(), Limits::default()); } - /// Hostile input is refused with the reference's errors, without allocating from what it declares. + /// Hostile input is refused by both readers with the same errors. #[test] - fn hostile_input_is_refused_like_the_reference() { + fn hostile_input_is_refused_alike_by_both_readers() { let hostile = [ format!("{}{}", "".repeat(100_000), "".repeat(100_000)), format!("{}", "".repeat(1000)), @@ -1179,10 +1184,10 @@ mod tests { } } - /// Every `.aepx` of the fixture directory gives the reference's tree (skipped without - /// `EFFECTCRAFT_AEP_FIXTURES`). + /// Every `.aepx` of the fixture directory gives the same tree through both readers (skipped + /// without `EFFECTCRAFT_AEP_FIXTURES`). #[test] - fn every_fixture_aepx_gives_the_reference_tree() { + fn every_fixture_aepx_gives_the_same_tree_through_both_readers() { let Some(dir) = std::env::var_os("EFFECTCRAFT_AEP_FIXTURES").filter(|d| !d.is_empty()) else { return }; let Ok(entries) = std::fs::read_dir(dir) else { return }; let mut paths: Vec = @@ -1191,7 +1196,8 @@ mod tests { for path in paths { let bytes = std::fs::read(&path).unwrap(); for l in limit_sets() { - assert_eq!(read_aepx_with_limits(&bytes, l), reference::read(&bytes, l), "{}", path.display()); + let index = index_aepx(&bytes, l).map(|d| TreeIndex::from_drafts(&d.buffer, d.nodes, d.trailer).to_file().into_owned()); + assert_eq!(index, read_aepx_with_limits(&bytes, l), "{}", path.display()); } } } diff --git a/crates/aep/src/container/aepx/reference.rs b/crates/aep/src/container/aepx/tree.rs similarity index 97% rename from crates/aep/src/container/aepx/reference.rs rename to crates/aep/src/container/aepx/tree.rs index dca6b77c4..ba7e20ed5 100644 --- a/crates/aep/src/container/aepx/reference.rs +++ b/crates/aep/src/container/aepx/tree.rs @@ -1,6 +1,6 @@ -//! The `.aepx` reader as it was before it fed the flat index: it builds the owned chunk tree -//! element by element. Kept for the tests only, as the reference the index reader is compared -//! with (the same tree and the same errors on generated, damaged and corpus input). +//! The `.aepx` reader of the owned chunk tree ([`super::read_aepx`]): it builds the tree element by +//! element. The importer uses the index reader in `aepx.rs` instead; the tests compare the two on +//! generated, damaged and corpus input (the same tree and the same errors). use std::borrow::Cow; @@ -9,8 +9,8 @@ use crate::container::chunk::{AepFile, Body, Chunk, LIST, NESTED_IDS}; use crate::container::error::AepError; use crate::container::read::Limits; -/// [`super::read_aepx_with_limits`] as it was. -pub(crate) fn read(data: &[u8], limits: Limits) -> Result, AepError> { +/// [`super::read_aepx_with_limits`]. +pub(super) fn read(data: &[u8], limits: Limits) -> Result, AepError> { let text = match std::str::from_utf8(data) { Ok(t) => t.as_bytes(), Err(e) => return Err(xml(e.valid_up_to(), "the file is not valid UTF-8")), diff --git a/crates/aep/src/container/index.rs b/crates/aep/src/container/index.rs index 3ae8b97ac..695d1e9f3 100644 --- a/crates/aep/src/container/index.rs +++ b/crates/aep/src/container/index.rs @@ -232,14 +232,7 @@ impl<'a> TreeIndex<'a> { /// The owned chunk tree (the bodies stay borrowed from the input): what /// [`crate::container::parse`] gives for the same file. pub fn to_file(&self) -> AepFile<'a> { - AepFile { chunks: self.build(0, u32::try_from(self.nodes.len()).unwrap_or(u32::MAX), Cow::Borrowed), trailer: Cow::Borrowed(self.trailer) } - } - - /// [`Self::to_file`] with every body copied, in one pass without recursion (what - /// `to_file().into_owned()` gives, without the borrowed tree in between). - pub(crate) fn to_owned_file(&self) -> AepFile<'static> { - let owned = |d: &[u8]| Cow::Owned(d.to_vec()); - AepFile { chunks: self.build(0, u32::try_from(self.nodes.len()).unwrap_or(u32::MAX), owned), trailer: Cow::Owned(self.trailer.to_vec()) } + AepFile { chunks: self.build(0, u32::try_from(self.nodes.len()).unwrap_or(u32::MAX)), trailer: Cow::Borrowed(self.trailer) } } /// The entry at `at`, `None` past the end. @@ -254,7 +247,7 @@ impl<'a> TreeIndex<'a> { /// The owned chunks for the siblings that start at `next` and fill `next..end`, without /// recursion. - fn build<'o>(&self, next: u32, end: u32, body_of: impl Fn(&'a [u8]) -> Cow<'o, [u8]>) -> Vec> { + fn build(&self, next: u32, end: u32) -> Vec> { struct Frame<'a> { /// The list or nested chunk being filled; `None` for the outermost range. at: Option, @@ -277,9 +270,9 @@ impl<'a> TreeIndex<'a> { let sub_end = self.end_of(at); stack.push(Frame { at: Some(at), kids: Vec::new(), next: at.saturating_add(1), end: sub_end }); } - Kind::Data => top.kids.push(Chunk { id: e.id, body: Body::Data(body_of(e.data)), pad: e.pad }), + Kind::Data => top.kids.push(Chunk { id: e.id, body: Body::Data(Cow::Borrowed(e.data)), pad: e.pad }), Kind::OpaqueList => { - top.kids.push(Chunk { id: e.id, body: Body::OpaqueList { list_type: e.list_type, data: body_of(e.data) }, pad: e.pad }); + top.kids.push(Chunk { id: e.id, body: Body::OpaqueList { list_type: e.list_type, data: Cow::Borrowed(e.data) }, pad: e.pad }); } } } else { @@ -497,7 +490,7 @@ impl<'f, 'a> Node<'f, 'a> { pub fn to_chunk(self) -> Chunk<'a> { match self.0 { NodeRepr::Chunk(c) => c.clone(), - NodeRepr::Index(tree, at) => tree.build(at, tree.end_of(at), Cow::Borrowed).into_iter().next().unwrap_or_else(|| Chunk::data(self.id(), &[][..])), + NodeRepr::Index(tree, at) => tree.build(at, tree.end_of(at)).into_iter().next().unwrap_or_else(|| Chunk::data(self.id(), &[][..])), } } diff --git a/crates/aep/src/import.rs b/crates/aep/src/import.rs index a2f4a4415..3d34b966f 100644 --- a/crates/aep/src/import.rs +++ b/crates/aep/src/import.rs @@ -246,14 +246,12 @@ mod tests { } } - /// Imports an `.aepx` through the owned tree of the reader this replaced and through the - /// index ([`import`]): the same project, report and settings, or the same error. + /// Imports an `.aepx` through the owned tree ([`crate::read_aepx`]) and through the index + /// ([`import`]): the same project, report and settings, or the same error. fn both_aepx(bytes: &[u8], name: &str) -> Result { let exists = |_: &str| true; let options = ImportOptions { allow_losses: true, ..ImportOptions::new(&exists) }; - let a = crate::container::aepx_reference::read(bytes, Limits::default()) - .map_err(AepImportError::from) - .and_then(|file| import_root(&mut Project::default(), Root::from(&file), &options)); + let a = crate::read_aepx(bytes).map_err(AepImportError::from).and_then(|file| import_root(&mut Project::default(), Root::from(&file), &options)); let b = import(&mut Project::default(), bytes, &options); assert!(a == b, "{name}: the .aepx import differs through the index"); b diff --git a/docs/aep-format.md b/docs/aep-format.md index c8708bf90..21ba31ae1 100644 --- a/docs/aep-format.md +++ b/docs/aep-format.md @@ -119,11 +119,11 @@ counterpart (a name that is not 1 to 4 characters) are skipped with their conten with the byte offset. It runs without recursion; the nesting depth (`Limits::max_depth`) and the element count (`Limits::max_chunks`) are bounded, and everything it allocates is bounded by the size of the input. -- The reader decodes every body (hex, text, the alias JSON, the numbers) into one buffer and records - the chunks as a flat depth-first list whose bodies are ranges of that buffer. The importer - indexes that buffer (`TreeIndex`) as it indexes an `.aep`; `read_aepx` converts it into the - owned tree. The tree is the one the element-by-element reader gave, which the crate's tests keep - as the reference. +- There are two readers. `read_aepx` builds the owned tree element by element. The importer, which + only reads, uses a second reader that decodes every body (hex, text, the alias JSON, the + numbers) into one buffer and records the chunks as a flat depth-first list whose bodies are + ranges of that buffer; it indexes that buffer (`TreeIndex`) as it indexes an `.aep`. The tests + hold both readers to the same tree and the same errors. - Tests: `tests/aepx.rs` compares the five fixture pairs chunk by chunk and byte by byte, compares the typed view of both forms, and runs truncated, bit-flipped, deeply nested and oversized inputs; `crates/aep/tests/it/import/aepx.rs` imports both forms of each pair and expects the same