pauljd/effectcraft
1
0
Fork 0
mirror of https://github.com/storytold/effectcraft.git synced 2026-11-12 12:22:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

address review: drop tessellation-tolerance, attribute-count and entity-size refusals; keep retained and draw budgets, warn when the draw budget is hit

Browse Source
This commit is contained in:
Dylan M. Taylor
2026-11-11 05:32:06 +00:00
parent 8a3ccd46ba
commit 774cdc7211
5 changed files with 9 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+1
View File
@@ -1891,6 +1891,7 @@ dependencies = [
"effectcraft-path",
"effectcraft-raster",
"kurbo",
"log",
"rayon",
"roxmltree",
]
crates/svg/Cargo.toml
+1
View File
@@ -16,3 +16,4 @@ effectcraft-path = { workspace = true }
kurbo = { workspace = true }
roxmltree = "0.21"
rayon = { workspace = true }
log = { workspace = true }
crates/svg/src/lib.rs
+4 -88
View File
@@ -147,27 +147,17 @@ impl Geom {
Geom::Rect { x, y, w, h, rx, ry } => {
let r = kurbo::Rect::new(*x, *y, x + w, y + h);
if *rx > 0.0 || *ry > 0.0 {
if (rx - ry).abs() < 1e-9 {
kurbo::RoundedRect::from_rect(r, *rx).to_path(tolerance(*rx))
} else {
elliptic_round_rect(*x, *y, *w, *h, *rx, *ry)
}
if (rx - ry).abs() < 1e-9 { kurbo::RoundedRect::from_rect(r, *rx).to_path(0.01) } else { elliptic_round_rect(*x, *y, *w, *h, *rx, *ry) }
} else {
r.to_path(0.01)
}
}
Geom::Ellipse { cx, cy, rx, ry } => kurbo::Ellipse::new((*cx, *cy), (*rx, *ry), 0.0).to_path(tolerance(rx.max(*ry))),
Geom::Ellipse { cx, cy, rx, ry } => kurbo::Ellipse::new((*cx, *cy), (*rx, *ry), 0.0).to_path(0.01),
Geom::Path(p) => p.clone(),
}
}
}
/// The flattening tolerance for a curve of radius `r`: 0.01, growing with the radius so a huge
/// circle is not split into an unbounded number of curves (as `pathdata` does for arcs).
fn tolerance(r: f64) -> f64 {
0.01_f64.max(r.abs() * 1e-4)
}
fn elliptic_round_rect(x: f64, y: f64, w: f64, h: f64, rx: f64, ry: f64) -> BezPath {
const K: f64 = 0.552_284_749_830_793_6;
let (kx, ky) = (rx * K, ry * K);
@@ -1247,23 +1237,11 @@ const MAX_XML_DEPTH: usize = 256;
/// more than half as many (#1236).
const MAX_EXPANDED_ELEMENTS: usize = 500_000;
/// The most attributes one element may have: the XML parser checks each attribute against all
/// earlier ones on its element, so a tag with very many takes quadratic time.
const MAX_XML_ATTRIBUTES: usize = 1024;
/// The deepest element nesting of `text` (a quick scan that skips comments, CDATA, processing
/// instructions, declarations and quoted attribute values).
#[cfg(test)]
fn xml_depth(text: &str) -> usize {
xml_shape(text).0
}
/// The deepest element nesting of `text` and the most attributes (`=` outside quotes) of one
/// of its start tags.
fn xml_shape(text: &str) -> (usize, usize) {
let b = text.as_bytes();
let (mut i, mut depth, mut max) = (0usize, 0usize, 0usize);
let mut attrs_max = 0usize;
let skip_to = |from: usize, end: &[u8]| b.get(from..).and_then(|r| r.windows(end.len()).position(|w| w == end)).map_or(b.len(), |p| from + p + end.len());
while i < b.len() {
if b[i] != b'<' {
@@ -1286,19 +1264,16 @@ fn xml_shape(text: &str) -> (usize, usize) {
// A start tag: find its end outside quotes; `/>` closes it at once.
let mut j = i + 1;
let mut quote = None;
let mut attrs = 0usize;
while let Some(&c) = b.get(j) {
match quote {
Some(q) if c == q => quote = None,
Some(_) => {}
None if c == b'"' || c == b'\'' => quote = Some(c),
None if c == b'>' => break,
None if c == b'=' => attrs += 1,
None => {}
}
j += 1;
}
attrs_max = attrs_max.max(attrs);
if b.get(j.wrapping_sub(1)) != Some(&b'/') {
depth += 1;
max = max.max(depth);
@@ -1306,59 +1281,7 @@ fn xml_shape(text: &str) -> (usize, usize) {
i = j + 1;
}
}
(max, attrs_max)
}
/// The most bytes the internal entity references of a document may expand to.
const MAX_ENTITY_BYTES: usize = 64 << 20;
/// What the general entity references (`&name;`) of `text` expand to, in bytes, counting the
/// internal entities its DTD declares (`<!ENTITY name "value">`) and the references inside
/// them. The XML parser bounds the nodes entities make but not the text, so a few repeated
/// references to nested entities could otherwise expand attribute values to gigabytes.
fn entity_bytes(text: &str) -> usize {
use std::collections::HashMap;
// Every declaration of a name is kept, comments and quoted text included: the parser uses
// only one of them, and the largest bounds whichever it is.
let mut decls: HashMap<&str, Vec<&str>> = HashMap::new();
let mut rest = text;
while let Some(p) = rest.find("<!ENTITY") {
rest = &rest[p + 8..];
let t = rest.trim_start();
// Parameter entities (`<!ENTITY % name …>`) are not referenced with `&`.
if t.starts_with('%') {
continue;
}
let name_end = t.find(|c: char| c.is_whitespace()).unwrap_or(t.len());
let (name, after) = t.split_at(name_end);
let after = after.trim_start();
let Some(q) = after.chars().next().filter(|c| *c == '"' || *c == '\'') else { continue };
if let Some(end) = after[1..].find(q) {
decls.entry(name).or_default().push(&after[1..1 + end]);
}
}
if decls.is_empty() {
return 0;
}
fn refs(s: &str) -> impl Iterator<Item = &str> {
s.split('&').skip(1).filter_map(|r| r.find(';').map(|e| &r[..e]))
}
fn size<'t>(name: &'t str, decls: &HashMap<&'t str, Vec<&'t str>>, memo: &mut HashMap<&'t str, usize>, depth: usize) -> usize {
if let Some(&n) = memo.get(name) {
return n;
}
let Some(values) = decls.get(name) else { return 0 };
// Nested past any real use (or a cycle, which the parser refuses): too large.
let n = if depth > 32 {
usize::MAX
} else {
values.iter().map(|value| refs(value).fold(value.len(), |n, r| n.saturating_add(size(r, decls, memo, depth + 1)))).max().unwrap_or(0)
};
memo.insert(name, n);
n
}
let mut memo = HashMap::new();
refs(text).fold(0usize, |n, r| n.saturating_add(size(r, &decls, &mut memo, 0)))
max
}
/// Nodes a document may expand to: a few kilobytes of nested internal entities
@@ -1369,16 +1292,9 @@ const MAX_XML_NODES: u32 = 4_000_000;
pub fn parse(bytes: &[u8]) -> Result<Doc, Error> {
let text = String::from_utf8_lossy(bytes);
let text = text.trim_start_matches('\u{feff}');
let (depth, attributes) = xml_shape(text);
if depth > MAX_XML_DEPTH {
if xml_depth(text) > MAX_XML_DEPTH {
return Err(Error::Xml(format!("elements nested more than {MAX_XML_DEPTH} levels deep")));
}
if attributes > MAX_XML_ATTRIBUTES {
return Err(Error::Xml(format!("an element has more than {MAX_XML_ATTRIBUTES} attributes")));
}
if entity_bytes(text) > MAX_ENTITY_BYTES {
return Err(Error::Xml(format!("its entity references expand to more than {} MiB", MAX_ENTITY_BYTES >> 20)));
}
// DTDs stay allowed (Illustrator's SVGs declare entities), but expansion is bounded (#2129).
let opts = roxmltree::ParsingOptions { allow_dtd: true, nodes_limit: MAX_XML_NODES, ..Default::default() };
let xml = roxmltree::Document::parse_with_options(text, opts).map_err(|e| Error::Xml(e.to_string()))?;
crates/svg/src/raster.rs
+3
View File
@@ -421,6 +421,9 @@ fn set_sat(c: [f32; 3], s: f32) -> [f32; 3] {
fn draw_node(n: &Node, m: Affine, opacity: f64, dst: &mut Image, budget: &mut SurfaceBudget) {
let Some(left) = budget.draws_left.checked_sub(1) else { return };
budget.draws_left = left;
if left == 0 {
log::warn!("svg: draw budget reached, the rest of the drawing is skipped");
}
match n {
Node::Group(g) => draw_group(g, m, opacity, dst, budget),
Node::Shape(s) => {
crates/svg/src/tests.rs
-37
View File
@@ -447,43 +447,6 @@ fn at_rule_blocks_are_skipped_by_brace_depth() {
assert_eq!(sheet.rules[0].classes, vec!["e".to_string()]);
}
#[test]
fn a_huge_circle_is_flattened_into_a_bounded_number_of_curves() {
let g = Geom::Ellipse { cx: 0.0, cy: 0.0, rx: 1e15, ry: 1e15 };
assert!(g.to_path().elements().len() < 1000, "{}", g.to_path().elements().len());
let small = Geom::Ellipse { cx: 0.0, cy: 0.0, rx: 10.0, ry: 10.0 }.to_path();
assert!(!small.elements().is_empty());
}
#[test]
fn an_element_with_too_many_attributes_is_refused() {
let attrs: String = (0..2000).map(|i| format!(" a{i}=\"1\"")).collect();
let svg = format!("<svg xmlns=\"http://www.w3.org/2000/svg\"><rect{attrs} width=\"1\" height=\"1\"/></svg>");
assert!(matches!(parse(svg.as_bytes()), Err(Error::Xml(e)) if e.contains("attributes")));
}
#[test]
fn entity_references_that_expand_past_the_limit_are_refused() {
// Each level repeats the one below ten times: 10^7 bytes of `x` per `&e;`, referenced 100 times.
let mut dtd = format!("<!ENTITY e0 \"{}\">", "x".repeat(10));
for i in 1..7 {
dtd.push_str(&format!("<!ENTITY e{i} \"{}\">", format!("&e{};", i - 1).repeat(10)));
}
let svg = format!("<!DOCTYPE svg [{dtd}]><svg xmlns=\"http://www.w3.org/2000/svg\"><g id=\"{}\"/></svg>", "&e6;".repeat(100));
assert!(matches!(parse(svg.as_bytes()), Err(Error::Xml(e)) if e.contains("entity")));
// Illustrator-style entities stay accepted.
let ok = "<!DOCTYPE svg [<!ENTITY ns_svg \"http://www.w3.org/2000/svg\">]><svg xmlns=\"&ns_svg;\"><rect width=\"1\" height=\"1\"/></svg>";
assert!(parse(ok.as_bytes()).is_ok());
}
#[test]
fn a_decoy_entity_declaration_does_not_hide_the_real_one() {
// A commented-out `b` comes first; the parser uses the later `b = &e;` (1 MiB each, 100 times).
let dtd = format!("<!-- <!ENTITY b \"x\"> --><!ENTITY e \"{}\"><!ENTITY b \"&e;\">", "x".repeat(1 << 20));
let svg = format!("<!DOCTYPE svg [{dtd}]><svg xmlns=\"http://www.w3.org/2000/svg\"><g id=\"{}\"/></svg>", "&b;".repeat(100));
assert!(matches!(parse(svg.as_bytes()), Err(Error::Xml(e)) if e.contains("entity")));
}
#[test]
fn gradient_stops_copied_past_the_budget_are_refused() {
// 4,000 stops referenced by 2,100 shapes would keep 8.4 million stops.