From bc770c21cc983e817a233b84dd8591e7e5b6d740 Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Wed, 11 Nov 2026 02:22:43 +0000
Subject: [PATCH] address review: budget guide-name bytes cached across
 coordinate spaces

---
 crates/pptx/src/custgeom.rs | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/crates/pptx/src/custgeom.rs b/crates/pptx/src/custgeom.rs
index 449654c80..0c64fd66e 100644
--- a/crates/pptx/src/custgeom.rs
+++ b/crates/pptx/src/custgeom.rs
@@ -19,6 +19,10 @@ const DEG: f64 = 60_000.0;
 /// further spaces are left out.
 const MAX_SPACES: usize = 16;
 
+/// Guide-name bytes the cached spaces after the first may hold between them: every space copies
+/// the names, which the XML limits bound only once.
+const MAX_SPACE_NAME_BYTES: usize = 4 << 20;
+
 /// Guide variables for one shape.
 pub struct Guides {
     vars: HashMap<String, f64>,
@@ -70,6 +74,10 @@ impl Guides {
         set("7cd8", 18_900_000.0);
         Guides { vars: v }
     }
+    /// Bytes of the guide names held.
+    fn name_bytes(&self) -> usize {
+        self.vars.keys().map(String::len).sum()
+    }
     pub fn get(&self, name: &str) -> f64 {
         let name = name.trim();
         if let Some(v) = self.vars.get(name) {
@@ -156,6 +164,7 @@ pub fn read_cust_geom(cg: &El, w_emu: f64, h_emu: f64) -> Vec<CustomPath> {
     let mut out = vec![];
     let Some(pl) = cg.child("pathLst") else { return out };
     let mut spaces: HashMap<(u64, u64), Guides> = HashMap::new();
+    let mut name_bytes = 0usize;
     // The cap on cached spaces saves guide evaluation: a geometry without guides has none to save.
     let guided = ["avLst", "gdLst"].iter().any(|n| cg.child(n).is_some_and(|l| l.elements().next().is_some()));
     for p in pl.children_named("path").take(1000) {
@@ -172,6 +181,12 @@ pub fn read_cust_geom(cg: &El, w_emu: f64, h_emu: f64) -> Vec<CustomPath> {
             let mut g = Guides::new(gw, gh);
             g.eval_list(cg.child("avLst"));
             g.eval_list(cg.child("gdLst"));
+            // The first space is always kept; later ones only while the cache stays within budget.
+            let bytes = g.name_bytes();
+            if guided && !spaces.is_empty() && name_bytes.saturating_add(bytes) > MAX_SPACE_NAME_BYTES {
+                continue;
+            }
+            name_bytes = name_bytes.saturating_add(bytes);
             spaces.insert(key, g);
         }
         let Some(g) = spaces.get(&key) else { continue };
@@ -501,6 +516,21 @@ mod tests {
         assert!(out[MAX_SPACES].d.starts_with("M 5 1"), "{}", out[MAX_SPACES].d);
     }
 
+    #[test]
+    fn guide_name_bytes_across_spaces_are_budgeted() {
+        let name = "n".repeat(3 << 20);
+        let path = |w: usize| format!(r#"<a:path w="{w}" h="10"><a:moveTo><a:pt x="{name}" y="1"/></a:moveTo></a:path>"#);
+        let x = format!(
+            r#"<a:custGeom xmlns:a="x"><a:gdLst><a:gd name="{name}" fmla="*/ w 1 2"/></a:gdLst><a:pathLst>{}{}{}</a:pathLst></a:custGeom>"#,
+            path(10),
+            path(20),
+            path(10)
+        );
+        let d = parse(x.as_bytes()).unwrap();
+        let out = read_cust_geom(&d.root, 1.0, 1.0);
+        assert_eq!(out.len(), 2, "the second 3 MiB space is over budget and left out; the first space's repeat stays");
+    }
+
     #[test]
     fn guide_free_geometries_are_not_capped_by_space() {
         let paths: String = (1..=40).map(|i| format!(r#"<a:path w="{}" h="10"><a:moveTo><a:pt x="1" y="1"/></a:moveTo></a:path>"#, 10 * i)).collect();