diff --git a/src/formats/docx/content.rs b/src/formats/docx/content.rs index ab03109..e1df1cc 100644 --- a/src/formats/docx/content.rs +++ b/src/formats/docx/content.rs @@ -633,7 +633,7 @@ impl<'a, 'b, 'e> InlineWalker<'a, 'b, 'e> { let Some((part, bytes)) = self.ctx.rel_part(rel_id)? else { return Ok(Vec::new()); }; - match crate::package::xml::parse_xml(&bytes) { + match self.ctx.pkg.borrow_mut().parse_part(&part, &bytes) { Ok(root) => Ok(crate::shared::drawingml::chart_blocks(&root)), Err(e) if e.is_fatal() => Err(e), Err(e) => { @@ -648,7 +648,7 @@ impl<'a, 'b, 'e> InlineWalker<'a, 'b, 'e> { let Some((part, bytes)) = self.ctx.rel_part(rel_id)? else { return Ok(Vec::new()); }; - match crate::package::xml::parse_xml(&bytes) { + match self.ctx.pkg.borrow_mut().parse_part(&part, &bytes) { Ok(root) => Ok(crate::shared::drawingml::diagram_blocks(&root)), Err(e) if e.is_fatal() => Err(e), Err(e) => { diff --git a/src/formats/pptx/mod.rs b/src/formats/pptx/mod.rs index 800c26e..bee49ec 100644 --- a/src/formats/pptx/mod.rs +++ b/src/formats/pptx/mod.rs @@ -13,7 +13,7 @@ use crate::model::{ use crate::package::relationships::{ RelTarget, Relationships, TargetMode, read_rels, rel_target_bytes, rel_type, rels_part_for, }; -use crate::package::xml::{Element, ns, parse_xml}; +use crate::package::xml::{Element, ns}; use crate::package::{Package, archive::probe_ole, path}; use crate::shared::assets::AssetSink; use crate::shared::delta::rebase_emphasis; @@ -580,7 +580,7 @@ fn parse_graphic_frame( && let Some(rid) = chart_ref.attr_qualified(ns::R, "id") && let Some((part, bytes)) = ctx.rel_part(rid)? { - match parse_xml(&bytes) { + match ctx.pkg.borrow_mut().parse_part(&part, &bytes) { Ok(root) => blocks.extend(crate::shared::drawingml::chart_blocks(&root)), Err(e) if e.is_fatal() => return Err(e), Err(e) => log::warn!("skipping corrupt chart part {part}: {e}"), @@ -591,7 +591,7 @@ fn parse_graphic_frame( && let Some(rid) = rel_ids.attr_qualified(ns::R, "dm") && let Some((part, bytes)) = ctx.rel_part(rid)? { - match parse_xml(&bytes) { + match ctx.pkg.borrow_mut().parse_part(&part, &bytes) { Ok(root) => blocks.extend(crate::shared::drawingml::diagram_blocks(&root)), Err(e) if e.is_fatal() => return Err(e), Err(e) => log::warn!("skipping corrupt diagram part {part}: {e}"), diff --git a/src/package/archive.rs b/src/package/archive.rs index ce6637b..fe97c0c 100644 --- a/src/package/archive.rs +++ b/src/package/archive.rs @@ -2,7 +2,7 @@ use crate::error::ConvertError; use crate::package::limits; -use crate::package::xml::{Element, parse_xml}; +use crate::package::xml::{Element, parse_xml_counted}; use std::collections::HashMap; use std::io::{Cursor, Read}; use std::rc::Rc; @@ -17,6 +17,19 @@ pub struct Package<'a> { /// reference one part many times). Bounded by `MAX_TOTAL_BYTES`. Buffers /// are shared (`Rc`), so a cache hit never copies the bytes. cache: HashMap>, + /// Parsed part trees by normalized name, shared (`Rc`) with callers: a + /// part referenced many times parses once. Holds up to + /// `MAX_CACHED_XML_NODES`; past that a part is returned uncached and + /// re-parses on its next reference. + trees: HashMap>, + /// Nodes currently held in `trees`, against `MAX_CACHED_XML_NODES`. + cached_nodes: u64, + /// XML nodes parsed out of this package so far, counting each parse + /// separately. The tree cache removes the repeats a document actually + /// has; this bounds the ones it cannot hold, so a package pointing at an + /// uncacheable part many times still terminates. Bounded by + /// `MAX_DOCUMENT_XML_NODES`. + parsed_nodes: u64, } impl<'a> Package<'a> { @@ -29,7 +42,14 @@ impl<'a> Package<'a> { detail: format!("archive contains {} entries", zip.len()), }); } - Ok(Package { zip, total_read: 0, cache: HashMap::new() }) + Ok(Package { + zip, + total_read: 0, + cache: HashMap::new(), + trees: HashMap::new(), + cached_nodes: 0, + parsed_nodes: 0, + }) } /// Read a part's bytes. `Ok(None)` means the part is absent (a valid @@ -116,11 +136,11 @@ impl<'a> Package<'a> { /// Read and parse an optional XML part under the unified recovery policy: /// absent -> `Ok(None)`; unreadable or corrupt -> skipped with a log; /// fatal resource-limit errors always propagate. - pub fn optional_xml_part(&mut self, name: &str) -> Result, ConvertError> { + pub fn optional_xml_part(&mut self, name: &str) -> Result>, ConvertError> { let Some(bytes) = self.optional_part(name)? else { return Ok(None); }; - match parse_xml(&bytes) { + match self.parse_part(name, &bytes) { Ok(tree) => Ok(Some(tree)), Err(e) if e.is_fatal() => Err(e), Err(e) => { @@ -132,9 +152,28 @@ impl<'a> Package<'a> { /// Read and parse an XML part that must exist and parse for any /// meaningful output. - pub fn required_xml_part(&mut self, name: &str) -> Result { + pub fn required_xml_part(&mut self, name: &str) -> Result, ConvertError> { let bytes = self.required_part(name)?; - parse_xml(&bytes) + self.parse_part(name, &bytes) + } + + /// Parse one of this package's parts, serving a part already parsed from + /// the tree cache. Parts reached by relationship (charts, diagrams) come + /// through here with the bytes the relationship resolved to, so that they + /// share the cache and the node total with the rest of the package. + pub fn parse_part(&mut self, name: &str, bytes: &[u8]) -> Result, ConvertError> { + let name = name.trim_start_matches('/'); + if let Some(tree) = self.trees.get(name) { + return Ok(Rc::clone(tree)); + } + let before = self.parsed_nodes; + let tree = Rc::new(parse_xml_counted(bytes, &mut self.parsed_nodes)?); + let nodes = self.parsed_nodes - before; + if self.cached_nodes + nodes <= limits::MAX_CACHED_XML_NODES { + self.cached_nodes += nodes; + self.trees.insert(name.to_string(), Rc::clone(&tree)); + } + Ok(tree) } } @@ -179,6 +218,45 @@ mod tests { assert_eq!(pkg.total_read, 4096, "repeated reads must not re-charge the budget"); } + #[test] + fn a_part_referenced_repeatedly_is_parsed_once() { + let data = one_part_zip("a.xml", b""); + let mut pkg = Package::open(&data).unwrap(); + let first = pkg.required_xml_part("a.xml").unwrap(); + let per_parse = pkg.parsed_nodes; + assert!(per_parse > 0); + for _ in 0..64 { + let again = pkg.required_xml_part("a.xml").unwrap(); + assert!(Rc::ptr_eq(&first, &again), "a repeat reference must share the parsed tree"); + } + assert_eq!(pkg.parsed_nodes, per_parse, "a cached tree must not re-parse"); + // The leading slash OPC part URIs carry must not miss the cache. + assert!(Rc::ptr_eq(&first, &pkg.required_xml_part("/a.xml").unwrap())); + } + + #[test] + fn a_part_too_large_to_cache_re_parses_and_hits_the_document_total() { + let data = one_part_zip("a.xml", b""); + let mut pkg = Package::open(&data).unwrap(); + // A package whose cache is full of earlier parts: this one is served + // uncached, so every reference to it parses again. The per-part cap + // resets on each of those parses and never fires; the running total + // is what stops it. + pkg.cached_nodes = limits::MAX_CACHED_XML_NODES; + pkg.required_xml_part("a.xml").unwrap(); + assert!(pkg.trees.is_empty(), "a full cache must not grow past its budget"); + let per_parse = pkg.parsed_nodes; + pkg.required_xml_part("a.xml").unwrap(); + assert_eq!(pkg.parsed_nodes, per_parse * 2, "an uncached repeat must be charged again"); + + pkg.parsed_nodes = limits::MAX_DOCUMENT_XML_NODES; + let err = pkg.required_xml_part("a.xml").unwrap_err(); + assert!( + matches!(err, ConvertError::ResourceLimit { limit: "max_document_xml_nodes", .. }), + "expected max_document_xml_nodes, got: {err}" + ); + } + #[test] fn total_budget_exhaustion_reports_max_total_bytes() { let mut w = zip::ZipWriter::new(Cursor::new(Vec::new())); diff --git a/src/package/limits.rs b/src/package/limits.rs index f6dba39..d06e5fe 100644 --- a/src/package/limits.rs +++ b/src/package/limits.rs @@ -23,6 +23,25 @@ pub const MAX_XML_DEPTH: usize = 256; /// memory test) so a saturating part stays around the archive budget. pub const MAX_XML_NODES: usize = 2_000_000; +/// Maximum XML nodes parsed across one document, summed over every part and +/// every time a part is parsed. The per-part cap above resets on each parse, +/// so a package that references one part many times (an EPUB reading order, +/// a presentation's slide list) can multiply it without bound; this is the +/// ceiling on that product. Sized against `MAX_TOTAL_BYTES`: XML runs about +/// 30-60 bytes per node, so a document that reads its whole decompression +/// budget as markup lands here, and one that parses more than that is +/// re-reading parts rather than covering new ones. +pub const MAX_DOCUMENT_XML_NODES: u64 = 16_000_000; + +/// Maximum XML nodes held in a package's parsed-tree cache, which is what +/// keeps a part referenced many times from being parsed many times. At the +/// measured DOM cost (~430 bytes/node) this is ~86 MiB, and it is far above +/// the parts documents actually repeat: style tables, slide layouts and +/// masters, and chapter files all sit in the thousands of nodes. A part too +/// large to cache re-parses per reference and is bounded by the document +/// total above. +pub const MAX_CACHED_XML_NODES: u64 = 200_000; + /// Maximum content-bearing cells a repeat expansion may produce per table. pub const MAX_EXPANSION: u64 = 4_000_000; diff --git a/src/package/xml.rs b/src/package/xml.rs index 42af1d8..49cd292 100644 --- a/src/package/xml.rs +++ b/src/package/xml.rs @@ -192,8 +192,10 @@ impl<'a> Iterator for DescendantNodes<'a> { /// Parse an XML part into a synthetic root element containing the top-level /// nodes. Encoding comes from the BOM or XML declaration; the part is /// transcoded to UTF-8 before parsing so namespace resolution sees one -/// consistent encoding. -pub fn parse_xml(bytes: &[u8]) -> Result { +/// consistent encoding. Nodes are charged against `document_nodes` as well +/// as the per-part cap, so that a package re-parsing one part for every +/// reference to it stays bounded. +pub fn parse_xml_counted(bytes: &[u8], document_nodes: &mut u64) -> Result { let utf8 = to_utf8(bytes); let mut reader = NsReader::from_reader(utf8.as_ref()); reader.config_mut().check_end_names = false; @@ -202,7 +204,7 @@ pub fn parse_xml(bytes: &[u8]) -> Result { let mut root = Element { ns: None, local: String::new(), attrs: Vec::new(), children: Vec::new() }; let mut stack: Vec = Vec::new(); - let mut nodes: usize = 0; + let mut nodes = NodeCount { part: 0, document: document_nodes }; let mut recovered = false; loop { @@ -310,14 +312,34 @@ fn declared_encoding(head: &[u8]) -> Option { Some(rest[..end].to_string()) } -fn bump_nodes(nodes: &mut usize) -> Result<(), ConvertError> { - *nodes += 1; - if *nodes > limits::MAX_XML_NODES { +/// [`parse_xml_counted`] against a throwaway document total. +#[cfg(test)] +pub fn parse_xml(bytes: &[u8]) -> Result { + parse_xml_counted(bytes, &mut 0) +} + +/// Node counters for one parse: the part's own count, and the running total +/// for the document the part belongs to. +struct NodeCount<'a> { + part: usize, + document: &'a mut u64, +} + +fn bump_nodes(nodes: &mut NodeCount<'_>) -> Result<(), ConvertError> { + nodes.part += 1; + if nodes.part > limits::MAX_XML_NODES { return Err(ConvertError::ResourceLimit { limit: "max_xml_nodes", detail: format!("part exceeds {} xml nodes", limits::MAX_XML_NODES), }); } + *nodes.document += 1; + if *nodes.document > limits::MAX_DOCUMENT_XML_NODES { + return Err(ConvertError::ResourceLimit { + limit: "max_document_xml_nodes", + detail: format!("document exceeds {} parsed xml nodes", limits::MAX_DOCUMENT_XML_NODES), + }); + } Ok(()) } diff --git a/tests/fixtures/malformed/selfref--errors.epub b/tests/fixtures/malformed/selfref--errors.epub new file mode 100644 index 0000000..10fc045 Binary files /dev/null and b/tests/fixtures/malformed/selfref--errors.epub differ diff --git a/tests/gen_fixtures.py b/tests/gen_fixtures.py index 21552bf..1774a27 100644 --- a/tests/gen_fixtures.py +++ b/tests/gen_fixtures.py @@ -1739,6 +1739,27 @@ def malformed(): # structurally unusable -> the labelled raw-recovery path must run. raw = ppt.read_bytes().replace(b"\xf5\x0f", b"\xf5\xff") (m / "brokenpersist--recovers.ppt").write_bytes(raw) + # Every reading-order entry resolves to the package document itself, so + # the part read once per entry is the one that grows with the entry + # count. Parsing it per entry is quadratic; the package's parsed-tree + # cache is what keeps this linear. No entry yields a chapter, so the book + # itself is unreadable. + n = 6400 + items = "".join( + f'' + for i in range(n)) + refs = "".join(f'' for i in range(n)) + write_zip(m / "selfref--errors.epub", [ + ("META-INF/container.xml", + '' + '' + ''), + ("c.opf", + '' + '' + f'{items}{refs}'), + ], mimetype_first="application/epub+zip") def abuse(): diff --git a/tests/snapshots/snapshots__malformed__selfref--errors.epub.snap b/tests/snapshots/snapshots__malformed__selfref--errors.epub.snap new file mode 100644 index 0000000..8a3eb67 --- /dev/null +++ b/tests/snapshots/snapshots__malformed__selfref--errors.epub.snap @@ -0,0 +1,5 @@ +--- +source: tests/snapshots.rs +expression: output +--- +ERROR: malformed document: no chapter in the book could be read