From 8ea413f054d3b37d004bf8156fd13271ffa1844e Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 11:19:42 -0800 Subject: [PATCH] SVG import: measure what an SVG expands to before the style cascade reads it The depth check and the prepass read each element's declarations through a cascade that keeps a copy of them per element, so one 100 kB style sheet rule on 16,000 groups still took 1.5 GB before the SVG was refused. The SVG and each SVG image it shows are now measured before that cascade runs. Copies nested past the depth cap, and loops of references, are found as too deep: an SVG is refused with the depth error, and an SVG image is left out with the depth warning, as before. --- crates/svg/src/import.rs | 9 +++-- crates/svg/src/import/budget.rs | 62 +++++++++++++++++++++++++++------ crates/svg/src/import/css.rs | 2 +- crates/svg/src/import/files.rs | 21 ++++++----- 4 files changed, 71 insertions(+), 23 deletions(-) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index b583f3dc03..c17f689922 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -137,10 +137,14 @@ pub(crate) fn import_finding( return Err(too_deep()); } let xml = parse(svg)?; + // Before the style cascade of `depth::height` and the prepass, which keeps the declarations of + // each element it reads; measured again below on the text usvg reads when that differs. + let styles = budget::check(&xml, budget::MAX_COPIED_STYLES, depth::MAX_ELEMENT_DEPTH)?; let Some(height) = depth::height(&xml, depth::MAX_ELEMENT_DEPTH) else { return Err(too_deep()); }; let (svg, found) = prepass(svg, &scan.markers, &xml, opts)?; + let rewritten = matches!(svg, Cow::Owned(_)); // The prepass's edits are in the text: the XML is read again only when it made any. let xml = match &svg { Cow::Borrowed(_) => xml, @@ -157,8 +161,9 @@ pub(crate) fn import_finding( let (src, slots) = text::prepare(svg, &xml, text_xml.as_ref(), units.dpi, &mut warnings); // Before usvg copies what ``s show and the style values of each copy: the text usvg reads. let styles = match &src { - Cow::Borrowed(_) => budget::check(&xml, budget::MAX_COPIED_STYLES)?, - Cow::Owned(prepared) => budget::check(&parse(prepared)?, budget::MAX_COPIED_STYLES)?, + Cow::Borrowed(_) if !rewritten => styles, + Cow::Borrowed(_) => budget::check(&xml, budget::MAX_COPIED_STYLES, usize::MAX)?, + Cow::Owned(prepared) => budget::check(&parse(prepared)?, budget::MAX_COPIED_STYLES, usize::MAX)?, }; let files = found.files; files.copied_styles(styles); diff --git a/crates/svg/src/import/budget.rs b/crates/svg/src/import/budget.rs index c2ff01a02f..ec4de17eca 100644 --- a/crates/svg/src/import/budget.rs +++ b/crates/svg/src/import/budget.rs @@ -27,7 +27,7 @@ use std::collections::{HashMap, HashSet}; use usvg::roxmltree; use super::{MAX_NODES, SVG_NS, XNode}; -use crate::SvgError; +use crate::{MAX_ELEMENT_DEPTH, SvgError}; /// Most elements usvg may make of an SVG: [`MAX_NODES`], and below the 1,000,000 elements past /// which usvg stops (its count includes a root of its own). @@ -287,12 +287,14 @@ impl<'a, 'i> Measure<'a, 'i> { } /// The bytes of style values usvg would copy into the elements of `xml`, when that is no more -/// than `bytes_left` and it makes no more than [`MAX_SVG_ELEMENTS`] elements of it. -pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64) -> Result { - match measure(xml, MAX_SVG_ELEMENTS as u64, bytes_left) { +/// than `bytes_left` and it makes no more than [`MAX_SVG_ELEMENTS`] elements of it, nested no +/// deeper than `depth` (counted as `depth::height` counts, which finds at least as much). +pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64, depth: usize) -> Result { + match measure(xml, MAX_SVG_ELEMENTS as u64, bytes_left, depth) { Ok(cost) => Ok(cost.bytes), Err(Past::Elements) => Err(SvgError::TooLarge(MAX_SVG_ELEMENTS)), Err(Past::Bytes) => Err(SvgError::StylesTooLarge((MAX_COPIED_STYLES >> 20) as usize)), + Err(Past::Depth) => Err(SvgError::TooDeep(MAX_ELEMENT_DEPTH)), } } @@ -301,11 +303,13 @@ pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64) -> Result Result { +/// What usvg makes of `xml`, up to `elements` elements and `bytes` bytes of copied styles, its +/// elements and the copies ``s show nested up to `depth` deep. Without recursion; each +/// element is measured once. +fn measure(xml: &roxmltree::Document, elements: u64, bytes: u64, depth: usize) -> Result { let mut m = Measure::new(xml); let mut costs: HashMap = HashMap::new(); // The elements entered and not yet left: one entered again is a loop. @@ -326,7 +330,11 @@ fn measure(xml: &roxmltree::Document, elements: u64, bytes: u64) -> Result depth { + return Err(Past::Depth); } let (parts, bare) = match svg_name(n) { Some("text") => (vec![], None), @@ -376,12 +384,12 @@ mod tests { /// What [`measure`] makes of `text` with no limits, as (elements, bytes). fn cost(text: &str) -> (u64, u64) { let xml = roxmltree::Document::parse(text).unwrap(); - let c = measure(&xml, u64::MAX, u64::MAX).unwrap_or_else(|p| panic!("{p:?}")); + let c = measure(&xml, u64::MAX, u64::MAX, usize::MAX).unwrap_or_else(|p| panic!("{p:?}")); (c.elements, c.bytes) } fn past(text: &str, elements: u64, bytes: u64) -> Option { - measure(&roxmltree::Document::parse(text).unwrap(), elements, bytes).err() + measure(&roxmltree::Document::parse(text).unwrap(), elements, bytes, usize::MAX).err() } /// Groups `g0` to `g{levels}`: `g0` a rectangle styled `style`, each other two ``s of the @@ -445,13 +453,30 @@ mod tests { #[test] fn a_loop_usvg_copies_without_end_is_refused_as_usvg_refuses_it() { let looped = svg(""); - assert_eq!(past(&looped, u64::MAX, u64::MAX), Some(Past::Elements)); + assert_eq!(past(&looped, u64::MAX, u64::MAX), Some(Past::Depth)); assert!(usvg::Tree::from_str(&looped, &usvg::Options::default()).is_err()); // Without the loop, usvg reads it. let open = svg(""); assert!(past(&open, u64::MAX, u64::MAX).is_none() && usvg::Tree::from_str(&open, &usvg::Options::default()).is_ok()); } + /// A chain of ``s nested past the depth is too deep, as `depth::height` finds it, before + /// what the chain makes is added up; one within the depth `depth::height` gives is not. + #[test] + fn copies_nested_past_the_depth_are_too_deep() { + let links: String = (0..3000).map(|i| format!("", i + 1)).collect(); + let deep = svg(&format!("{links}")); + let xml = roxmltree::Document::parse(&deep).unwrap(); + assert_eq!(measure(&xml, u64::MAX, u64::MAX, MAX_ELEMENT_DEPTH).err(), Some(Past::Depth)); + assert!(super::super::depth::height(&xml, MAX_ELEMENT_DEPTH).is_none()); + assert!(matches!(check(&xml, MAX_COPIED_STYLES, MAX_ELEMENT_DEPTH), Err(SvgError::TooDeep(_)))); + for text in [chain(5, ""), svg("")] { + let xml = roxmltree::Document::parse(&text).unwrap(); + let height = super::super::depth::height(&xml, usize::MAX).unwrap_or(0); + assert!(measure(&xml, u64::MAX, u64::MAX, height).is_ok(), "{height} {text}"); + } + } + #[test] fn text_counts_its_runs_and_what_trefs_show() { let (n, bytes) = cost(&svg("abcdxyz")); @@ -459,4 +484,19 @@ mod tests { assert_eq!(n, 1 + 1 + 1 + 2 + 1 + 1 + 1); assert_eq!(bytes, (16 + 2) + (16 + 1) + (16 + 4)); } + + /// A long style sheet rule that matches many elements is refused before the import's own style + /// cascade (which keeps the declarations of each element) reads them, in an SVG and in an SVG + /// image it shows. + #[test] + fn a_long_rule_on_many_elements_is_refused_before_the_cascade_reads_it() { + let groups = svg(&format!("{}", "a".repeat(100_000), "".repeat(1000))); + let image = svg(&format!(r#""#, crate::base64_encode(groups.as_bytes()))); + for text in [groups, image] { + let before = super::super::css::selector_tests(); + let err = super::super::import(&text, &crate::ImportOptions::default(), MAX_NODES).err(); + assert!(matches!(err, Some(SvgError::StylesTooLarge(64))), "{err:?}"); + assert_eq!(super::super::css::selector_tests(), before); + } + } } diff --git a/crates/svg/src/import/css.rs b/crates/svg/src/import/css.rs index fd864a547e..7ddfb1bbd5 100644 --- a/crates/svg/src/import/css.rs +++ b/crates/svg/src/import/css.rs @@ -129,7 +129,7 @@ thread_local! { /// [`SELECTOR_TESTS`] of this thread. #[cfg(test)] -fn selector_tests() -> u64 { +pub(super) fn selector_tests() -> u64 { SELECTOR_TESTS.get() } diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index 429c39c70f..98c0e4aed4 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -587,22 +587,25 @@ impl Files { return leave_out(&self.too_deep, true); } let xml = super::parse(text).ok()?; - let Some(height) = depth::height(&xml, left) else { return leave_out(&self.too_deep, true) }; - // usvg's parser would overflow on it, as on the SVG holding it (`import`). - super::convolve_matrices_ok(&xml).ok()?; - // What usvg would make of it, measured as the SVG holding it is (`import`), its copied - // styles counted with theirs while usvg reads it, and kept only when it decodes: past the - // limits, the whole SVG is refused. + // What usvg would make of it, measured as the SVG holding it is (`import`), before the + // style cascade of `depth::height`; its copied styles count with theirs while usvg reads + // it, and only when it decodes. Too deep, it is left out; past the other limits, the whole + // SVG is refused. let spent = self.styles.load(Ordering::Relaxed); - match budget::check(&xml, budget::MAX_COPIED_STYLES.saturating_sub(spent)) { - Ok(bytes) => self.styles.store(spent.saturating_add(bytes), Ordering::Relaxed), + let bytes = match budget::check(&xml, budget::MAX_COPIED_STYLES.saturating_sub(spent), left) { + Ok(bytes) => bytes, + Err(SvgError::TooDeep(_)) => return leave_out(&self.too_deep, true), Err(e) => { if let Ok(mut refused) = self.refused.lock() { refused.get_or_insert(e); } return None; } - } + }; + let Some(height) = depth::height(&xml, left) else { return leave_out(&self.too_deep, true) }; + // usvg's parser would overflow on it, as on the SVG holding it (`import`). + super::convolve_matrices_ok(&xml).ok()?; + self.styles.store(spent.saturating_add(bytes), Ordering::Relaxed); self.depth.store(used + height, Ordering::Relaxed); let within = self.within.lock().ok().map(|mut w| w.push(file)).is_some(); // usvg's `Tree::from_data_nested`, except that hrefs to files go to the resolver.