From 6ff564e7504322b754305ae4399d17d9634c53f4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 14:39:06 -0500 Subject: [PATCH] SVG: bound tessellation, kept geometry, attributes, entity expansion and re-rendering (#2986) * SVG: bound tessellation, kept geometry, attributes, entity expansion and re-rendering Circles and rounded rects flattened at a fixed 0.01 tolerance, so a huge radius made unbounded curves; the tolerance now grows with the radius as arcs already did. Every path a copies and every gradient reference's stops were parsed and kept without a document-wide limit; both now share a budget of 8 million items. An element with thousands of attributes made the XML parser's duplicate check quadratic, and repeated entity references could expand attribute text to gigabytes below the node limit; a preflight refuses more than 1,024 attributes per element and entity references that expand past 64 MiB. Nested knockout and non-isolated groups re-rendered their children several times per level; node draws are now held to 64 per node of the tree. * address review: entity_bytes takes the largest declaration of each name * address review: drop tessellation-tolerance, attribute-count and entity-size refusals; keep retained and draw budgets, warn when the draw budget is hit --- Cargo.lock | 1 + crates/svg/Cargo.toml | 1 + crates/svg/src/lib.rs | 27 +++++++++++++++++++++++++++ crates/svg/src/raster.rs | 32 +++++++++++++++++++++++++++++++- crates/svg/src/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 991b06154..002edc8d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1939,6 +1939,7 @@ dependencies = [ "effectcraft-path", "effectcraft-raster", "kurbo", + "log", "rayon", "roxmltree", ] diff --git a/crates/svg/Cargo.toml b/crates/svg/Cargo.toml index 0e1af7719..03cacaa7b 100644 --- a/crates/svg/Cargo.toml +++ b/crates/svg/Cargo.toml @@ -16,3 +16,4 @@ effectcraft-path = { workspace = true } kurbo = { workspace = true } roxmltree = "0.21" rayon = { workspace = true } +log = { workspace = true } diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 95c00a91a..c31617a92 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -603,8 +603,15 @@ struct Parser<'a, 'i> { /// The `` targets being expanded, outermost first: a reference back into one is a /// cycle and is skipped. using: Vec, + /// Path elements and gradient stops kept so far, `` copies and repeated gradient + /// references included; past [`MAX_RETAINED`] the import errs. + retained: std::cell::Cell, } +/// The most path elements and gradient stops an import keeps: a complex path reused many times, +/// or a gradient with many stops referenced by many shapes, is copied for each use. +const MAX_RETAINED: usize = 8_000_000; + fn is_svg_ns(n: &roxmltree::Node) -> bool { n.is_element() } @@ -628,6 +635,13 @@ impl<'a, 'i> Parser<'a, 'i> { out } + /// Charge `n` kept path elements or gradient stops; false once [`MAX_RETAINED`] is passed. + fn charge_retained(&self, n: usize) -> bool { + let r = self.retained.get().saturating_add(n); + self.retained.set(r); + r <= MAX_RETAINED + } + /// Charge `n` units of CSS work; false once the budget is spent. fn charge_css(&self, n: usize) -> bool { let w = self.css_work.get().saturating_add(n); @@ -875,6 +889,10 @@ impl<'a, 'i> Parser<'a, 'i> { // Stops: from the first element in the chain that has any. let mut stops = vec![]; if let Some(src) = chain.iter().find(|e| e.children().any(|c| c.is_element() && c.tag_name().name() == "stop")) { + // Charged before the stops are read: every reference reads and keeps them again. + if !self.charge_retained(src.children().filter(|c| c.is_element()).count()) { + return None; + } let mut last = 0.0f64; for s in src.children().filter(|c| c.is_element() && c.tag_name().name() == "stop") { let d = self.declared(s); @@ -1026,6 +1044,11 @@ impl<'a, 'i> Parser<'a, 'i> { return; } let Some(geom) = self.geometry(n, &st) else { return }; + if let Geom::Path(p) = &geom + && !self.charge_retained(p.elements().len()) + { + return; + } let fill = if matches!(tag, "line") { None } else { st.fill.as_ref().and_then(|p| self.resolve_paint(p, &st)) }; let (fill, fill_alpha) = match fill { Some((p, a)) => (Some(p), a), @@ -1304,6 +1327,7 @@ pub fn parse(bytes: &[u8]) -> Result { work: 0, budget, using: vec![], + retained: std::cell::Cell::new(0), }; let st = Style::default(); let pct_or = |name: &str, axis: char, def: f64| -> f64 { @@ -1344,6 +1368,9 @@ pub fn parse(bytes: &[u8]) -> Result { // Never a silently partial drawing. return Err(Error::Xml(format!("its references expand to more than {} elements", p.budget))); } + if p.retained.get() > MAX_RETAINED { + return Err(Error::Xml(format!("its paths and gradients, with their copies, hold more than {MAX_RETAINED} elements"))); + } if p.css_work.get() > MAX_CSS_WORK { // Never a silently unstyled drawing. return Err(Error::Xml(format!("matching its