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