From e927729f25dff145e980f2ea1d40f888e5df9ca0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 01:40:55 +0000 Subject: [PATCH] SVG: delegate entity expansion bounds, sniffing, and SVGZ to craft-svg --- Cargo.lock | 102 +++++++++++++++++++++++++++---------- crates/eps/src/ps.rs | 3 -- crates/svg/Cargo.toml | 1 + crates/svg/src/entities.rs | 86 ++----------------------------- crates/svg/src/lib.rs | 83 +++++++----------------------- 5 files changed, 97 insertions(+), 178 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7af992cadb..00fe4ca454 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1079,6 +1079,21 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "craft-svg" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=6d620e971e820b517b11a99cad040ebac670e1e2#6d620e971e820b517b11a99cad040ebac670e1e2" +dependencies = [ + "craft-xml", + "flate2", + "kurbo 0.13.1", + "roxmltree 0.21.2", + "thiserror 2.0.21", + "usvg 0.48.1", + "vectorcraft-geom 0.18.0", + "vectorcraft-pathops 0.18.0", +] + [[package]] name = "craft-ui" version = "0.1.0" @@ -1168,6 +1183,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "craft-xml" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=6d620e971e820b517b11a99cad040ebac670e1e2#6d620e971e820b517b11a99cad040ebac670e1e2" +dependencies = [ + "quick-xml 0.41.0", + "thiserror 2.0.21", +] + [[package]] name = "crc32fast" version = "1.5.3" @@ -6408,7 +6432,7 @@ dependencies = [ "serde_json", "vectorcraft-color", "vectorcraft-doc", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", ] [[package]] @@ -6423,8 +6447,8 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", - "vectorcraft-pathops", + "vectorcraft-geom 0.24.1", + "vectorcraft-pathops 0.24.1", "vectorcraft-text", ] @@ -6441,7 +6465,7 @@ dependencies = [ "vectorcraft-doc", "vectorcraft-engine", "vectorcraft-format", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-mcp", "vectorcraft-render", "vectorcraft-svg", @@ -6478,7 +6502,7 @@ dependencies = [ "thiserror 2.0.21", "tiff", "vectorcraft-color", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "zune-jpeg 0.5.15", ] @@ -6493,8 +6517,8 @@ dependencies = [ "svgtypes 0.16.1", "vectorcraft-color", "vectorcraft-doc", - "vectorcraft-geom", - "vectorcraft-pathops", + "vectorcraft-geom 0.24.1", + "vectorcraft-pathops 0.24.1", "vectorcraft-plugins", "vectorcraft-text", "vectorcraft-three-d", @@ -6538,10 +6562,10 @@ dependencies = [ "vectorcraft-doc", "vectorcraft-eps", "vectorcraft-format", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-js", "vectorcraft-metafile", - "vectorcraft-pathops", + "vectorcraft-pathops 0.24.1", "vectorcraft-pdf", "vectorcraft-plugins", "vectorcraft-render", @@ -6576,9 +6600,9 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-metafile", - "vectorcraft-pathops", + "vectorcraft-pathops 0.24.1", "vectorcraft-pdf", "vectorcraft-render", "vectorcraft-testkit", @@ -6597,10 +6621,20 @@ dependencies = [ "serde_json", "thiserror 2.0.21", "vectorcraft-doc", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-testkit", ] +[[package]] +name = "vectorcraft-geom" +version = "0.18.0" +source = "git+https://github.com/storytold/vectorcraft.git?rev=6b3a6b8788ebcb420d69449199cbdf323888d27e#6b3a6b8788ebcb420d69449199cbdf323888d27e" +dependencies = [ + "kurbo 0.13.1", + "serde", + "thiserror 2.0.21", +] + [[package]] name = "vectorcraft-geom" version = "0.24.1" @@ -6635,7 +6669,7 @@ dependencies = [ "vectorcraft-doc", "vectorcraft-engine", "vectorcraft-format", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-render", "vectorcraft-svg", "vectorcraft-testkit", @@ -6652,10 +6686,21 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-text", ] +[[package]] +name = "vectorcraft-pathops" +version = "0.18.0" +source = "git+https://github.com/storytold/vectorcraft.git?rev=6b3a6b8788ebcb420d69449199cbdf323888d27e#6b3a6b8788ebcb420d69449199cbdf323888d27e" +dependencies = [ + "kurbo 0.13.1", + "linesweeper", + "thiserror 2.0.21", + "vectorcraft-geom 0.18.0", +] + [[package]] name = "vectorcraft-pathops" version = "0.24.1" @@ -6664,7 +6709,7 @@ dependencies = [ "linesweeper", "proptest", "thiserror 2.0.21", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-testkit", ] @@ -6697,7 +6742,7 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-testkit", "vectorcraft-text", "weezl 0.2.1", @@ -6716,7 +6761,7 @@ dependencies = [ "thiserror 2.0.21", "vectorcraft-color", "vectorcraft-doc", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "wasmi", "wat", "web-time", @@ -6741,7 +6786,7 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-plugins", "vectorcraft-testkit", "vectorcraft-text", @@ -6765,6 +6810,7 @@ name = "vectorcraft-svg" version = "0.24.1" dependencies = [ "craft-docpath", + "craft-svg", "flate2", "image", "kurbo 0.13.1", @@ -6781,8 +6827,8 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", - "vectorcraft-pathops", + "vectorcraft-geom 0.24.1", + "vectorcraft-pathops 0.24.1", "vectorcraft-testkit", "vectorcraft-text", ] @@ -6800,7 +6846,7 @@ dependencies = [ "vectorcraft-doc", "vectorcraft-engine", "vectorcraft-format", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-render", "vectorcraft-svg", ] @@ -6820,7 +6866,7 @@ dependencies = [ "unicode-script", "unicode-segmentation", "vectorcraft-doc", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vello_cpu", "windows-registry", "write-fonts", @@ -6830,8 +6876,8 @@ dependencies = [ name = "vectorcraft-three-d" version = "0.24.1" dependencies = [ - "vectorcraft-geom", - "vectorcraft-pathops", + "vectorcraft-geom 0.24.1", + "vectorcraft-pathops 0.24.1", ] [[package]] @@ -6846,8 +6892,8 @@ dependencies = [ "vectorcraft-color", "vectorcraft-doc", "vectorcraft-effects", - "vectorcraft-geom", - "vectorcraft-pathops", + "vectorcraft-geom 0.24.1", + "vectorcraft-pathops 0.24.1", "vectorcraft-text", ] @@ -6860,7 +6906,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.21", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", ] [[package]] @@ -6889,7 +6935,7 @@ dependencies = [ "vectorcraft-engine", "vectorcraft-eps", "vectorcraft-format", - "vectorcraft-geom", + "vectorcraft-geom 0.24.1", "vectorcraft-pdf", "vectorcraft-plugins", "vectorcraft-render", diff --git a/crates/eps/src/ps.rs b/crates/eps/src/ps.rs index d3d0dfcf4e..2a10b8a86c 100644 --- a/crates/eps/src/ps.rs +++ b/crates/eps/src/ps.rs @@ -21,9 +21,6 @@ VCdict begin end "; -/// Longest line of encoded data (DSC readers take up to 255 characters). -const LINE: usize = 76; - /// `v` as PostScript writes it: at most four decimals, no trailing zeros (0 when not finite). pub(crate) fn num(v: f64) -> String { let mut s = String::new(); diff --git a/crates/svg/Cargo.toml b/crates/svg/Cargo.toml index 7c506d2bb3..7708d23c06 100644 --- a/crates/svg/Cargo.toml +++ b/crates/svg/Cargo.toml @@ -9,6 +9,7 @@ description = "VectorCraft SVG import and export" workspace = true [dependencies] +craft-svg = { workspace = true } # Which paths stored in documents may be opened. craft-docpath = { workspace = true } vectorcraft-geom = { workspace = true } diff --git a/crates/svg/src/entities.rs b/crates/svg/src/entities.rs index d1eb4cfef7..e80622ae42 100644 --- a/crates/svg/src/entities.rs +++ b/crates/svg/src/entities.rs @@ -2,90 +2,10 @@ //! replaces every `&name;` with the entity's text as it reads, with no total limit: a short file //! that repeats a large entity in an attribute allocates far more than it holds. -use std::collections::HashMap; - -/// The most text the references to entities may expand to in all (bytes). -const MAX_EXPANDED: usize = 16 << 20; - -/// How deep entities that refer to entities are followed (the reader's own limit is lower). -const MAX_NESTING: u32 = 16; - -/// The longest entity name a reference is looked for with ([`references`]). -const MAX_NAME: usize = 256; - -/// The internal general entities declared in `svg`: name → every value declared for it. Text that -/// only looks like a declaration (in a comment, or in another entity's value) is kept too: the -/// largest value counts, so it can only make the estimate larger. -fn declared(svg: &str) -> HashMap<&str, Vec<&str>> { - let mut map: HashMap<&str, Vec<&str>> = HashMap::new(); - for (at, _) in svg.match_indices(" impl Iterator { - text.match_indices('&').filter_map(|(at, _)| { - let rest = text.get(at + 1..)?; - // Look no further than the longest name: a file of `&`s without `;` would be read quadratically. - let end = rest.bytes().take(MAX_NAME + 1).position(|b| b == b';')?; - let name = rest.get(..end)?; - (!name.is_empty() && name.len() <= MAX_NAME && !name.starts_with('#') && !name.contains(char::is_whitespace)).then_some(name) - }) -} - -/// What entity `name` expands to (bytes), references in its value followed. -fn expanded<'a>(name: &'a str, entities: &HashMap<&'a str, Vec<&'a str>>, memo: &mut HashMap<&'a str, usize>, depth: u32) -> usize { - if let Some(n) = memo.get(name) { - return *n; - } - let Some(values) = entities.get(name) else { return 0 }; - if depth >= MAX_NESTING { - return usize::MAX; - } - let mut most = 0usize; - for value in values { - let mut total = value.len(); - for inner in references(value) { - total = total.saturating_add(expanded(inner, entities, memo, depth + 1)); - } - most = most.max(total); - } - memo.insert(name, most); - most -} - -/// Whether the references to the entities `svg` declares expand to more than [`MAX_EXPANDED`] +/// Whether the references to the entities `svg` declares expand to more than the limit /// bytes in all. False for a file that declares none. pub(crate) fn expansion_too_large(svg: &str) -> bool { - let entities = declared(svg); - if entities.is_empty() { - return false; - } - // A reference to a longer name isn't looked for: refuse such a declaration instead. - if entities.keys().any(|name| name.len() > MAX_NAME) { - return true; - } - let mut memo = HashMap::new(); - let mut total = 0usize; - for name in references(svg) { - total = total.saturating_add(expanded(name, &entities, &mut memo, 0)); - if total > MAX_EXPANDED { - return true; - } - } - false + craft_svg::guard::entities_expand_past(svg, craft_svg::guard::MAX_ENTITY_EXPANSION) } #[cfg(test)] @@ -118,7 +38,7 @@ mod tests { assert!(expansion_too_large(&commented)); let in_value = format!("\">]>"); assert!(expansion_too_large(&in_value)); - let long = "n".repeat(MAX_NAME + 1); + let long = "n".repeat(257); let long_name = format!("]>"); assert!(expansion_too_large(&long_name)); } diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 0c698eebc7..23cc07d3c1 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -458,21 +458,17 @@ pub(crate) fn body_hash(parts: &[&str]) -> String { /// Does `bytes` start like gzip data (an SVGZ file)? pub fn is_svgz(bytes: &[u8]) -> bool { - bytes.starts_with(&[0x1f, 0x8b]) + craft_svg::sniff::is_svgz(bytes) } /// Compress SVG text as SVGZ (gzip). pub fn compress(svg: &str) -> Vec { - compress_bytes(svg.as_bytes()) + craft_svg::svgz::compress(svg.as_bytes()) } /// Compress an SVG file (in any encoding, see [`Output::bytes`]) as SVGZ (gzip). pub fn compress_bytes(svg: &[u8]) -> Vec { - use std::io::Write as _; - let mut gz = flate2::write::GzEncoder::new(Vec::with_capacity(svg.len() / 4), flate2::Compression::default()); - // Writing to a Vec cannot fail. - let _ = gz.write_all(svg); - gz.finish().unwrap_or_default() + craft_svg::svgz::compress(svg) } /// Most bytes an SVGZ file may unpack to (a guard against decompression bombs). @@ -487,29 +483,26 @@ pub fn text_of(bytes: &[u8]) -> Result, SvgError> { /// [`text_of`], SVGZ bytes unpacking within what `unpacked`, the bytes SVGZ data unpacked to so /// far, leaves of [`MAX_SVGZ`]. What they unpack to is added to it, whether or not it decodes. pub(crate) fn text_within<'a>(bytes: &'a [u8], unpacked: &std::sync::atomic::AtomicU64) -> Result, SvgError> { - use std::io::Read as _; use std::sync::atomic::Ordering; if !is_svgz(bytes) { - return match decode(bytes)? { - Decoded::Utf8(skip) => std::str::from_utf8(bytes.get(skip..).unwrap_or_default()).map(Into::into).map_err(|_| not_utf8()), - Decoded::Text(t) => Ok(t.into()), - }; + return craft_svg::svgz::decode(bytes).map_err(|_| not_utf8()); } - let most = MAX_SVGZ.saturating_sub(unpacked.load(Ordering::Relaxed)); - let mut raw = Vec::new(); - let read = flate2::read::GzDecoder::new(bytes).take(most.saturating_add(1)).read_to_end(&mut raw); - // Read before a failure is kept: it was unpacked too. - unpacked.fetch_add(raw.len() as u64, Ordering::Relaxed); - read.map_err(|e| SvgError::Parse(format!("not a valid SVGZ file: {e}")))?; - if raw.len() as u64 > most { - return Err(SvgError::TooBig(format!("the SVGZ file unpacks to more than {} MB", most >> 20))); - } - match decode(&raw)? { - Decoded::Utf8(skip) => { - raw.drain(..skip); - String::from_utf8(raw).map(Into::into).map_err(|_| not_utf8()) + let most = usize::try_from(MAX_SVGZ.saturating_sub(unpacked.load(Ordering::Relaxed))).unwrap_or(usize::MAX); + match craft_svg::svgz::inflate(bytes, most) { + Ok(raw) => { + unpacked.fetch_add(raw.len() as u64, Ordering::Relaxed); + craft_svg::svgz::decode(&raw) + .map(|t| std::borrow::Cow::Owned(t.into_owned())) + .map_err(|_| not_utf8()) } - Decoded::Text(t) => Ok(t.into()), + Err(craft_svg::svgz::Error::TooLarge(_)) => { + Err(SvgError::TooBig(format!("the SVGZ file unpacks to more than {} MB", most >> 20))) + } + Err(craft_svg::svgz::Error::Corrupt(e)) => { + Err(SvgError::Parse(format!("not a valid SVGZ file: {e}"))) + } + Err(craft_svg::svgz::Error::Encoding) => Err(not_utf8()), + Err(e) => Err(SvgError::Parse(e.to_string())), } } @@ -517,44 +510,6 @@ fn not_utf8() -> SvgError { SvgError::Parse("SVG is not UTF-8, UTF-16 or ISO 8859-1".into()) } -/// How SVG bytes are read. -enum Decoded { - /// UTF-8 after this many bytes (a byte order mark). - Utf8(usize), - /// Text decoded from another encoding. - Text(String), -} - -/// The encoding of SVG bytes: a byte order mark (UTF-8 or UTF-16), else the XML declaration's -/// (ISO 8859-1 by its names; anything else is read as UTF-8). -fn decode(bytes: &[u8]) -> Result { - let utf16 = |rest: &[u8], unit: fn([u8; 2]) -> u16| { - let (units, odd) = rest.as_chunks::<2>(); - if !odd.is_empty() { - return Err(not_utf8()); - } - char::decode_utf16(units.iter().map(|u| unit(*u))).collect::>().map(Decoded::Text).map_err(|_| not_utf8()) - }; - match bytes { - [0xef, 0xbb, 0xbf, ..] => Ok(Decoded::Utf8(3)), - [0xfe, 0xff, rest @ ..] => utf16(rest, u16::from_be_bytes), - [0xff, 0xfe, rest @ ..] => utf16(rest, u16::from_le_bytes), - _ if declared_latin1(bytes) => Ok(Decoded::Text(bytes.iter().map(|b| char::from(*b)).collect())), - _ => Ok(Decoded::Utf8(0)), - } -} - -/// Does the XML declaration at the start of `bytes` name ISO 8859-1? -fn declared_latin1(bytes: &[u8]) -> bool { - let head = bytes.get(..bytes.len().min(200)).unwrap_or_default(); - let Some(decl) = head.strip_prefix(b"').next()) else { return false }; - let decl = String::from_utf8_lossy(decl).to_ascii_lowercase(); - let Some(at) = decl.find("encoding") else { return false }; - let value = decl.get(at + "encoding".len()..).unwrap_or("").trim_start().trim_start_matches('=').trim_start(); - let name = value.trim_start_matches(['"', '\'']).split(['"', '\'', ' ', '?']).next().unwrap_or(""); - matches!(name, "iso-8859-1" | "iso8859-1" | "iso_8859-1" | "latin1" | "latin-1" | "l1") -} - /// Import an SVG document. pub fn import(svg: &str) -> Result { import_with_report(svg).map(|(d, _)| d)