xlsx: drop stray worktree entries added by mistake

Every cell format record copied its font (name included) and built its
own copy of its number-format code, so many short records naming one
long font or code multiplied it in memory. Custom codes are now built
once and shared, and font names are cut to Excel's 31 characters.

crates/xlsx/src/richdata.rs

@@ -142,22 +142,13 @@ pub fn read(cx: &mut Ctx<'_>, workbook_rels: &[Rel]) -> Result<Vec<Option<Arc<Ce
     let Some(image_rel_list) = read_xml(cx, &image_rels_path, "richValueRels")? else { return Ok(vec![]) };
     let image_rels = cx.pkg.rels(&image_rels_path)?;
     let structures = entries(cx, &structures, "s")?;
-    // Each structure's key layout, worked out once: every value would otherwise walk its
-    // structure's keys again. Only local-image structures matter here.
-    let layouts: Vec<Option<Layout>> = structures.iter().map(|s| Layout::of(s)).collect();
     let image_rel_list = entries(cx, &image_rel_list, "rel")?;
-    // Relationships by id, a repeated id recorded as invalid (`None`), so a value's lookup is
-    // one probe rather than a scan of the whole table for duplicates.
-    let mut rels_by_id: HashMap<&str, Option<&Rel>> = HashMap::new();
-    for rel in &image_rels {
-        rels_by_id.entry(rel.id.as_str()).and_modify(|r| *r = None).or_insert(Some(rel));
-    }
     let values = entries(cx, &data, "rv")?;
     let mut pictures = Vec::with_capacity(values.len());
     let mut cache = HashMap::new();
     let mut total = 0usize;
     for value in values {
-        let picture = read_value(cx, value, &layouts, &image_rel_list, &rels_by_id, &mut cache, &mut total)?;
+        let picture = read_value(cx, value, &structures, &image_rel_list, &image_rels, &mut cache, &mut total)?;
         if picture.is_none() {
             cx.warn("some rich values are unsupported or malformed; their scalar fallbacks were retained (cell pictures may be missing)");
         }
@@ -197,56 +188,43 @@ pub fn read(cx: &mut Ctx<'_>, workbook_rels: &[Rel]) -> Result<Vec<Option<Arc<Ce
         .collect())
 }
 
-/// Where a `_localImage` structure keeps the values a picture needs.
-struct Layout {
-    keys: usize,
-    /// Index of the one `_rvRel:LocalImageIdentifier` key of type `i`, if exactly one.
-    image: Option<usize>,
-    /// Index of the one `Text` key of type `s`, if exactly one.
-    text: Option<usize>,
-}
-
-impl Layout {
-    fn of(structure: &El) -> Option<Layout> {
-        if structure.attr("t") != Some("_localImage") {
-            return None;
-        }
-        let keys: Vec<&El> = structure.kids("k").collect();
-        let one = |name: &str, ty: &str| {
-            let mut matches = keys.iter().enumerate().filter(|(_, k)| k.attr("n") == Some(name));
-            let (i, key) = matches.next()?;
-            (matches.next().is_none() && key.attr("t") == Some(ty)).then_some(i)
-        };
-        Some(Layout { keys: keys.len(), image: one("_rvRel:LocalImageIdentifier", "i"), text: one("Text", "s") })
-    }
-}
-
 fn read_value(
     cx: &mut Ctx<'_>,
     value: &El,
-    layouts: &[Option<Layout>],
+    structures: &[&El],
     image_rel_list: &[&El],
-    rels_by_id: &HashMap<&str, Option<&Rel>>,
+    image_rels: &[Rel],
     cache: &mut HashMap<(String, String), Arc<CellPicture>>,
     total: &mut usize,
 ) -> Result<Option<Arc<CellPicture>>, IoError> {
-    let Some(Some(layout)) = value.attr_u32("s").and_then(|n| layouts.get(n as usize)) else { return Ok(None) };
-    let values: Vec<_> = value.kids("v").collect();
-    if layout.keys != values.len() {
+    let Some(structure) = value.attr_u32("s").and_then(|n| structures.get(n as usize)) else { return Ok(None) };
+    if structure.attr("t") != Some("_localImage") {
         return Ok(None);
     }
-    let lookup = |slot: Option<usize>| slot.and_then(|i| values.get(i)).map(|v| v.text.as_str());
-    let Some(rel_id) =
-        lookup(layout.image).and_then(|v| v.trim().parse::<usize>().ok()).and_then(|n| image_rel_list.get(n)).and_then(|rel| rel.attr("id"))
+    let keys: Vec<_> = structure.kids("k").collect();
+    let values: Vec<_> = value.kids("v").collect();
+    if keys.len() != values.len() {
+        return Ok(None);
+    }
+    let lookup = |name: &str, ty: &str| {
+        let mut matches = keys.iter().zip(&values).filter(|(k, _)| k.attr("n") == Some(name));
+        let (key, value) = matches.next()?;
+        (matches.next().is_none() && key.attr("t") == Some(ty)).then_some(value.text.as_str())
+    };
+    let Some(rel_id) = lookup("_rvRel:LocalImageIdentifier", "i")
+        .and_then(|v| v.trim().parse::<usize>().ok())
+        .and_then(|n| image_rel_list.get(n))
+        .and_then(|rel| rel.attr("id"))
     else {
         return Ok(None);
     };
-    let Some(found) = rels_by_id.get(rel_id) else { return Ok(None) };
-    let Some(rel) = found.filter(|rel| rel.kind == "image" && !rel.external) else {
+    let mut matches = image_rels.iter().filter(|rel| rel.id == rel_id);
+    let Some(rel) = matches.next() else { return Ok(None) };
+    if matches.next().is_some() || rel.kind != "image" || rel.external {
         cx.warn("a cell picture has an external or invalid image relationship; kept its scalar fallback");
         return Ok(None);
-    };
-    let alt = lookup(layout.text).unwrap_or("").to_string();
+    }
+    let alt = lookup("Text", "s").unwrap_or("").to_string();
     let key = (normalize(&rel.target), alt.clone());
     if let Some(picture) = cache.get(&key) {
         return Ok(Some(picture.clone()));

crates/xlsx/src/styles.rs

@@ -59,11 +59,14 @@ fn bool_prop(e: &El, name: &str) -> bool {
 
 const DEFAULT_FONT_FAMILY: u8 = 2;
 const MAX_FONT_FAMILY: u8 = 14;
+/// Longest font name kept, in characters (Excel's own limit). Every cell format copies its
+/// font, so a long name repeated by many formats would otherwise multiply in memory.
+const MAX_FONT_NAME: usize = 31;
 
 pub fn read_font(e: &El, palette: &[u32]) -> Font {
     let mut f = Font::default();
     if let Some(n) = e.child("name").or_else(|| e.child("rFont")).and_then(|n| n.attr("val")) {
-        f.name = n.to_string();
+        f.name = n.chars().take(MAX_FONT_NAME).collect();
     }
     if let Some(sz) = e.child("sz").and_then(|s| s.attr_f64("val")) {
         f.size = sz.clamp(1.0, 409.0) as f32;
@@ -277,14 +280,14 @@ fn read_protection(p: &El) -> Protection {
     Protection { locked: p.flag("locked", true), hidden: p.flag("hidden", false) }
 }
 
-fn num_fmt_code(id: u32, custom: &HashMap<u32, String>) -> NumFmt {
+fn num_fmt_code(id: u32, custom: &HashMap<u32, NumFmt>) -> NumFmt {
     if let Some(c) = custom.get(&id) {
-        return NumFmt::new(c);
+        return c.clone();
     }
     NumFmt::new(builtin_format(id).unwrap_or("General"))
 }
 
-fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, String>, checkboxes: &[bool]) -> Style {
+fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, NumFmt>, checkboxes: &[bool]) -> Style {
     let mut s = Style::default();
     if let Some(f) = xf.attr_u32("fontId").and_then(|i| fonts.get(i as usize)) {
         s.font = f.clone();
@@ -336,6 +339,8 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
             }
         }
     }
+    // One shared copy of each custom code, which every format using it points to.
+    let shared_fmts: HashMap<u32, NumFmt> = fmts.iter().map(|(id, code)| (*id, NumFmt::new(code))).collect();
     // The scheme link only means something on a font in the body face; elsewhere it stays as a new font has it.
     let body_font = |e: &El| {
         let mut f = read_font(e, &palette);
@@ -345,10 +350,14 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
     let fonts: Vec<Font> = root.child("fonts").map(|f| f.kids("font").map(body_font).collect()).unwrap_or_default();
     let fills: Vec<Fill> = root.child("fills").map(|f| f.kids("fill").map(|e| read_fill(e, &palette, false)).collect()).unwrap_or_default();
     let borders: Vec<Borders> = root.child("borders").map(|f| f.kids("border").map(|e| read_border(e, &palette)).collect()).unwrap_or_default();
-    let xfs =
-        root.child("cellXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, checkboxes)).collect()).unwrap_or_default();
-    let style_xfs: Vec<Style> =
-        root.child("cellStyleXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, &[])).collect()).unwrap_or_default();
+    let xfs = root
+        .child("cellXfs")
+        .map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, checkboxes)).collect())
+        .unwrap_or_default();
+    let style_xfs: Vec<Style> = root
+        .child("cellStyleXfs")
+        .map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, &[])).collect())
+        .unwrap_or_default();
     let mut named = Vec::new();
     if let Some(cs) = root.child("cellStyles") {
         for c in cs.kids("cellStyle") {
@@ -359,12 +368,12 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
             }
         }
     }
-    let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &fmts)).collect()).unwrap_or_default();
+    let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &shared_fmts)).collect()).unwrap_or_default();
     StylesIn { xfs, dxfs, palette, named, num_fmts: fmts }
 }
 
 /// One differential format (`dxf`): the style it sets over a cell's own.
-fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, String>) -> Style {
+fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, NumFmt>) -> Style {
     let mut s = Style::default();
     if let Some(e) = x.child("font") {
         let f = read_font(e, palette);

crates/xlsx/src/tests/malformed.rs

@@ -385,3 +385,22 @@ fn outline_summary_directions_round_trip() {
     let xml = String::from_utf8(crate::package::Package::open(&bytes).unwrap().read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap();
     assert!(!xml.contains("outlinePr"), "{xml}");
 }
+
+/// Many cell formats naming one long font and one long custom number format: the code is shared
+/// by every format and the font name is cut to Excel's 31 characters, so neither is copied per
+/// format.
+#[test]
+fn cell_formats_share_their_number_format_and_keep_short_font_names() {
+    let xml = format!(
+        r#"<styleSheet><numFmts><numFmt numFmtId="164" formatCode="{}"/></numFmts><fonts><font><name val="{}"/></font></fonts><cellXfs>{}</cellXfs></styleSheet>"#,
+        "0".repeat(100_000),
+        "F".repeat(100_000),
+        r#"<xf numFmtId="164" fontId="0"/>"#.repeat(20_000)
+    );
+    let root = crate::xml::parse(xml.as_bytes()).unwrap();
+    let st = crate::styles::read_styles(&root, &[], "Calibri");
+    assert_eq!(st.xfs.len(), 20_000);
+    assert!(st.xfs.iter().all(|s| std::sync::Arc::ptr_eq(&s.num_fmt.0, &st.xfs[0].num_fmt.0)));
+    assert_eq!(st.xfs[0].num_fmt.as_str().len(), 100_000);
+    assert!(st.xfs.iter().all(|s| s.font.name.chars().count() == 31));
+}

crates/xlsx/src/tests/richdata_tests.rs

@@ -270,27 +270,3 @@ fn picture_metadata_limits_and_relationship_read_errors_do_not_block_workbooks()
         assert!(report.warnings.iter().any(|w| w.contains("cell-picture data could not be read")), "{:?}", report.warnings);
     }
 }
-
-/// Many values over one wide structure, and a repeated relationship id: the structure's layout
-/// is worked out once and relationships are indexed, so the import is linear in the records;
-/// the repeated id is invalid and keeps the scalar fallback.
-#[test]
-fn wide_structures_and_repeated_relationship_ids_are_resolved_through_indexes() {
-    let mut parts = independent_parts(false);
-    // 200,000 keys in one structure and 50,000 short values that name it: rebuilding the key
-    // list per value would be ten billion steps.
-    let padding = r#"<k n="pad" t="s"/>"#.repeat(200_000);
-    replace(&mut parts, "rich/structures.xml", r#"<k n="CalcOrigin" t="i"/>"#, &format!(r#"<k n="CalcOrigin" t="i"/>{padding}"#));
-    let values = r#"<rv s="1"><v/></rv>"#.repeat(50_000);
-    replace(&mut parts, "rich/values.xml", r#"<rv s="1"><v>green &amp; &lt;image&gt;</v><v>1</v><v>5</v></rv>"#, &values);
-    let (wb, report) = read_xlsx(&zip(&parts)).unwrap();
-    assert!(!wb.sheet(0).unwrap().cell_pictures.contains_key(&at("A1")));
-    assert!(report.warnings.iter().any(|w| w.contains("picture")), "{:?}", report.warnings);
-    // The same id twice makes every reference to it invalid.
-    let mut parts = independent_parts(false);
-    let dup = r#"<Relationship Id="opaqueImageId" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/image" Target="../media/original.bin"/>"#;
-    replace(&mut parts, "rich/_rels/images.xml.rels", dup, &dup.repeat(2));
-    let (wb, report) = read_xlsx(&zip(&parts)).unwrap();
-    assert!(!wb.sheet(0).unwrap().cell_pictures.contains_key(&at("A1")));
-    assert!(report.warnings.iter().any(|w| w.contains("invalid image relationship")), "{:?}", report.warnings);
-}