Merge remote-tracking branch 'origin/switch-craft-libs-offcrypto' into codex/reviewfix-3843

craft-cfb refuses a header without the 0xFFFE mark at offset 28
([MS-CFB] 2.2), so craft-xls's sniff called the fixture LegacyBinary.
The old hand-written parser never read that field, which is why the test
only broke with the switch. The fixture is a real workbook otherwise:
sniff now says Xls and read_xls parses it.

Cargo.lock

@@ -935,12 +935,12 @@ dependencies = [
 [[package]]
 name = "craft-fonts-build"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 
 [[package]]
 name = "craft-i18n"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 dependencies = [
  "fixed_decimal",
  "icu_decimal",
@@ -977,10 +977,21 @@ dependencies = [
  "zeroize",
 ]
 
+[[package]]
+name = "craft-opc"
+version = "0.1.0"
+source = "git+https://github.com/storytold/craft-libs?rev=bb57d422d0bc842cbde430955825553ab7b976f4#bb57d422d0bc842cbde430955825553ab7b976f4"
+dependencies = [
+ "craft-xml",
+ "craft-zip",
+ "log",
+ "thiserror 2.0.21",
+]
+
 [[package]]
 name = "craft-print"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 dependencies = [
  "craft-winprint-gdi",
  "log",
@@ -999,7 +1010,7 @@ source = "git+https://github.com/storytold/craft-libs?rev=bb57d422d0bc842cbde430
 [[package]]
 name = "craft-ui"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 dependencies = [
  "craft-fonts-build",
  "craft-i18n",
@@ -1046,7 +1057,7 @@ dependencies = [
 [[package]]
 name = "craft-web-api"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 dependencies = [
  "js-sys",
  "serde_json",
@@ -1057,7 +1068,7 @@ dependencies = [
 [[package]]
 name = "craft-web-host"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=119415f46f601e20b9bc6b1b4d6979ceda86d88e#119415f46f601e20b9bc6b1b4d6979ceda86d88e"
+source = "git+https://github.com/storytold/craft-libs?rev=aa84415bae3b765fef6b7ef42404f66ad1cff9d9#aa84415bae3b765fef6b7ef42404f66ad1cff9d9"
 dependencies = [
  "wasm-bindgen",
  "web-sys",
@@ -1104,6 +1115,16 @@ dependencies = [
  "thiserror 2.0.21",
 ]
 
+[[package]]
+name = "craft-zip"
+version = "0.1.0"
+source = "git+https://github.com/storytold/craft-libs?rev=bb57d422d0bc842cbde430955825553ab7b976f4#bb57d422d0bc842cbde430955825553ab7b976f4"
+dependencies = [
+ "flate2",
+ "thiserror 2.0.21",
+ "zip 9.1.0",
+]
+
 [[package]]
 name = "crc32fast"
 version = "1.5.2"
@@ -2326,6 +2347,7 @@ dependencies = [
  "craft-cfb",
  "craft-csv",
  "craft-offcrypto",
+ "craft-opc",
  "craft-xls",
  "gridcraft-core",
  "gridcraft-formula",

Cargo.toml

@@ -34,6 +34,8 @@ craft-update = { git = "https://github.com/storytold/craft-libs", rev = "bb57d42
 # Compound files ([MS-CFB]) and password-encrypted OOXML packages ([MS-OFFCRYPTO]).
 craft-cfb = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
 craft-offcrypto = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
+# OPC zip packages (parts, relationships, the zip writer) under `crates/xlsx/src/package.rs`.
+craft-opc = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
 # Delimited text (CSV, TSV) and SYLK; `crates/xlsx/src/csv.rs` and `crates/engine/src/io/sylk.rs`
 # turn them into workbooks.
 craft-csv = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }

apps/gridcraft-web/Cargo.toml

@@ -18,8 +18,8 @@ gridcraft-engine = { workspace = true }
 gridcraft-ui-egui = { workspace = true }
 # The host messages (storytold/craft-libs): one contract and one bridge for the web builds.
 # The page command API (storytold/craft-libs#413): window.gridcraft over the control channel.
-craft-web-api = { git = "https://github.com/storytold/craft-libs", rev = "119415f46f601e20b9bc6b1b4d6979ceda86d88e", version = "0.1.0" }
-craft-web-host = { git = "https://github.com/storytold/craft-libs", rev = "119415f46f601e20b9bc6b1b4d6979ceda86d88e", version = "0.1.0" }
+craft-web-api = { git = "https://github.com/storytold/craft-libs", rev = "aa84415bae3b765fef6b7ef42404f66ad1cff9d9", version = "0.1.0" }
+craft-web-host = { git = "https://github.com/storytold/craft-libs", rev = "aa84415bae3b765fef6b7ef42404f66ad1cff9d9", version = "0.1.0" }
 egui = { workspace = true }
 log = { workspace = true }
 serde_json = { workspace = true }

apps/gridcraft/Cargo.toml

@@ -35,7 +35,7 @@ winit = { version = "0.30", default-features = false, features = ["x11", "waylan
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 # Same craft-libs revision as the workspace's craft-update and craft-window (see the root
 # Cargo.toml).
-craft-print = { git = "https://github.com/storytold/craft-libs", rev = "119415f46f601e20b9bc6b1b4d6979ceda86d88e" }
+craft-print = { git = "https://github.com/storytold/craft-libs", rev = "aa84415bae3b765fef6b7ef42404f66ad1cff9d9" }
 craft-update = { workspace = true }
 rfd = { workspace = true }
 arboard = { workspace = true, features = ["image-data"] }

crates/ui-egui/Cargo.toml

@@ -11,7 +11,7 @@ workspace = true
 [dependencies]
 # The shared capture slot (`craft_ui::capture`) comes from the one craft-libs revision every
 # craft-libs dependency uses (see the root Cargo.toml).
-craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "119415f46f601e20b9bc6b1b4d6979ceda86d88e" }
+craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "aa84415bae3b765fef6b7ef42404f66ad1cff9d9" }
 gridcraft-engine = { workspace = true }
 gridcraft-chart = { workspace = true }
 # `monochrome_emoji_fonts` restores the Noto Emoji and emoji-icon faces that 0.37 moved behind

crates/xlsx/Cargo.toml

@@ -21,6 +21,7 @@ gridcraft-model = { workspace = true }
 craft-cfb = { workspace = true }
 craft-csv = { workspace = true }
 craft-offcrypto = { workspace = true }
+craft-opc = { workspace = true }
 craft-xls = { workspace = true }
 thiserror = { workspace = true }
 quick-xml = { workspace = true }

crates/xlsx/src/ods.rs

@@ -280,7 +280,7 @@ impl Import {
                 Event::GeneralRef(r) => {
                     let ch = match r.resolve_char_ref().map_err(xml_error)? {
                         Some(c) => c,
-                        None => match r.as_ref() {
+                        None => match &*r {
                             b"amp" => '&',
                             b"lt" => '<',
                             b"gt" => '>',

crates/xlsx/src/package.rs

@@ -1,12 +1,12 @@
-//! OPC package access: reading zip parts with decompression limits, relationships and part-name
-//! resolution; writing a zip package deterministically.
-
-use std::collections::HashMap;
-use std::io::{Cursor, Read, Write};
+//! OPC package access through `craft-opc`: zip parts read against GridCraft's limits,
+//! relationships with GridCraft's `Rel`, part-name resolution, and a deterministic zip writer.
+//! Parts are parsed with this crate's XML reader.
 
 use crate::IoError;
 use crate::xml::{self, El};
 
+pub use craft_opc::{normalize, resolve, split_dir};
+
 /// Largest decompressed part accepted.
 pub const MAX_PART: u64 = 512 * 1024 * 1024;
 /// Largest total decompressed size read from one package.
@@ -14,11 +14,10 @@ pub const MAX_TOTAL: u64 = 1024 * 1024 * 1024;
 /// Most entries accepted in the zip directory.
 pub const MAX_ENTRIES: usize = 200_000;
 
+const LIMITS: craft_opc::Limits = craft_opc::Limits::new(MAX_ENTRIES, MAX_PART, MAX_TOTAL);
+
 pub struct Package<'a> {
-    zip: zip::ZipArchive<Cursor<&'a [u8]>>,
-    /// Lower-case normalized part name → zip index.
-    names: HashMap<String, usize>,
-    total: u64,
+    opc: craft_opc::LazyPackage<'a>,
 }
 
 /// A relationship.
@@ -33,88 +32,49 @@ pub struct Rel {
     pub external: bool,
 }
 
-pub fn normalize(name: &str) -> String {
-    name.replace('\\', "/").trim_start_matches('/').to_ascii_lowercase()
+/// GridCraft's wording of a package error.
+fn error(e: craft_opc::Error) -> IoError {
+    use craft_opc::Error as E;
+    match e {
+        E::Zip(m) | E::Write(m) => IoError::Zip(m),
+        E::TooManyEntries { declared, .. } => IoError::TooLarge(format!("{declared} zip entries")),
+        E::PartTooLarge { part, limit } => IoError::TooLarge(format!("{part} expands to more than {limit} bytes")),
+        E::PackageTooLarge { .. } => IoError::TooLarge("package expands to more than 1 GB".into()),
+        E::Xml { part, error } => IoError::Xml(format!("{part}: {error}")),
+        e if e.is_limit() => IoError::TooLarge(e.to_string()),
+        e => IoError::Zip(e.to_string()),
+    }
 }
 
 impl<'a> Package<'a> {
     pub fn open(bytes: &'a [u8]) -> Result<Package<'a>, IoError> {
-        let zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| IoError::Zip(e.to_string()))?;
-        if zip.len() > MAX_ENTRIES {
-            return Err(IoError::TooLarge(format!("{} zip entries", zip.len())));
-        }
-        let mut names = HashMap::new();
-        for i in 0..zip.len() {
-            if let Some(n) = zip.name_for_index(i) {
-                names.entry(normalize(n)).or_insert(i);
-            }
-        }
-        Ok(Package { zip, names, total: 0 })
+        Ok(Package { opc: craft_opc::LazyPackage::open(bytes, &LIMITS).map_err(error)? })
     }
 
     /// Every part's (normalized) name, sorted.
     #[cfg(test)]
     pub fn names(&self) -> Vec<String> {
-        let mut v: Vec<String> = self.names.keys().cloned().collect();
-        v.sort();
-        v
+        self.opc.names().into_iter().map(str::to_string).collect()
     }
 
     pub fn has(&self, part: &str) -> bool {
-        self.names.contains_key(&normalize(part))
+        self.opc.has(part)
     }
 
     /// Reads a part's bytes; `Ok(None)` when it doesn't exist.
     pub fn read(&mut self, part: &str) -> Result<Option<Vec<u8>>, IoError> {
-        self.read_limited(part, MAX_PART)
+        self.opc.read(part).map_err(error)
     }
 
     /// Reads a part the reader can do without (the caller warns and goes on when it fails). A read
-    /// that fails partway is charged this part's whole allowance (`maximum`, and the byte that
-    /// detects an overrun) instead of ending the package's: the decompression work stays bounded,
-    /// and the parts read after it still open.
+    /// that fails partway is charged this part's whole allowance instead of ending the package's.
     pub fn read_optional_limited(&mut self, part: &str, maximum: u64) -> Result<Option<Vec<u8>>, IoError> {
-        let before = self.total;
-        let read = self.read_limited(part, maximum);
-        if let Err(IoError::Zip(_)) = &read {
-            self.total = before.saturating_add(maximum.min(MAX_PART).saturating_add(1)).min(MAX_TOTAL);
-        }
-        read
+        self.opc.read_optional_limited(part, maximum).map_err(error)
     }
 
     /// Reads a small metadata part without allowing it to expand to the general part limit.
     pub fn read_limited(&mut self, part: &str, maximum: u64) -> Result<Option<Vec<u8>>, IoError> {
-        let maximum = maximum.min(MAX_PART);
-        let Some(&idx) = self.names.get(&normalize(part)) else {
-            return Ok(None);
-        };
-        let mut f = self.zip.by_index(idx).map_err(|e| IoError::Zip(format!("{part}: {e}")))?;
-        if f.size() > maximum {
-            return Err(IoError::TooLarge(format!("{part} expands to {} bytes", f.size())));
-        }
-        let remaining = MAX_TOTAL.saturating_sub(self.total);
-        let limit = maximum.min(remaining);
-        let mut out = Vec::with_capacity(f.size().min(16 * 1024 * 1024) as usize);
-        let read = (&mut f).take(limit + 1).read_to_end(&mut out).map_err(|e| {
-            // A read that fails partway has already spent decompression work and buffered data
-            // the package budget never sees: count the allowance as spent, so a loop over many
-            // broken parts can't decompress it over and over.
-            self.total = MAX_TOTAL;
-            IoError::Zip(format!("{part}: {e}"))
-        });
-        if out.len() as u64 > limit {
-            // The allowance was spent getting here; make the exhaustion terminal instead of
-            // letting later reads pay the same decompression cost again.
-            self.total = MAX_TOTAL;
-            return Err(IoError::TooLarge(if remaining < maximum {
-                "package expands to more than 1 GB".into()
-            } else {
-                format!("{part} expands to more than {maximum} bytes")
-            }));
-        }
-        read?;
-        self.total += out.len() as u64;
-        Ok(Some(out))
+        self.opc.read_limited(part, maximum).map_err(error)
     }
 
     pub fn read_xml(&mut self, part: &str) -> Result<Option<El>, IoError> {
@@ -127,25 +87,13 @@ impl<'a> Package<'a> {
     /// Relationships of a part (`xl/workbook.xml` → `xl/_rels/workbook.xml.rels`). Missing or
     /// broken rels parts give an empty list.
     pub fn rels(&mut self, part: &str) -> Result<Vec<Rel>, IoError> {
-        let (dir, file) = split_dir(part);
-        let rels_name = if dir.is_empty() { format!("_rels/{file}.rels") } else { format!("{dir}/_rels/{file}.rels") };
-        let root = match self.read(&rels_name)? {
-            Some(b) => match xml::parse(&b) {
-                Ok(r) => r,
-                Err(_) => return Ok(vec![]),
-            },
-            None => return Ok(vec![]),
-        };
-        Ok(root
-            .kids("Relationship")
-            .filter_map(|r| {
-                let id = r.attr("Id")?.to_string();
-                let ty = r.attr("Type").unwrap_or("");
-                let kind = ty.rsplit('/').next().unwrap_or("").to_string();
-                let target_raw = r.attr("Target")?;
-                let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("External"));
-                let target = if external { target_raw.to_string() } else { resolve(dir, target_raw) };
-                Some(Rel { id, kind, target, external })
+        let rels = self.opc.rels(part).map_err(error)?;
+        Ok(rels
+            .list
+            .into_iter()
+            .map(|r| {
+                let kind = r.kind().to_string();
+                Rel { id: r.id, kind, target: r.target, external: r.external }
             })
             .collect())
     }
@@ -153,11 +101,7 @@ impl<'a> Package<'a> {
 
 /// The first part whose declared unpacked size is over [`MAX_PART`], with that size.
 pub fn oversized_part(bytes: &[u8]) -> Option<(String, u64)> {
-    let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).ok()?;
-    (0..zip.len().min(MAX_ENTRIES)).find_map(|i| {
-        let f = zip.by_index_raw(i).ok()?;
-        (f.size() > MAX_PART).then(|| (f.name().to_string(), f.size()))
-    })
+    craft_opc::declared_oversized(bytes, &LIMITS)
 }
 
 fn prefix(part: &str, e: IoError) -> IoError {
@@ -167,54 +111,22 @@ fn prefix(part: &str, e: IoError) -> IoError {
     }
 }
 
-/// `xl/worksheets/sheet1.xml` → (`xl/worksheets`, `sheet1.xml`).
-pub fn split_dir(part: &str) -> (&str, &str) {
-    let p = part.trim_start_matches('/');
-    match p.rfind('/') {
-        Some(i) => (p.get(..i).unwrap_or(""), p.get(i + 1..).unwrap_or("")),
-        None => ("", p),
-    }
-}
-
-/// Resolves a relationship target against the source part's directory.
-pub fn resolve(dir: &str, target: &str) -> String {
-    let target = target.replace('\\', "/");
-    let target = target.split('#').next().unwrap_or("").to_string();
-    let mut segs: Vec<&str> = if target.starts_with('/') { vec![] } else { dir.split('/').filter(|s| !s.is_empty()).collect() };
-    for s in target.split('/') {
-        match s {
-            "" | "." => {}
-            ".." => {
-                segs.pop();
-            }
-            s => segs.push(s),
-        }
-    }
-    segs.join("/")
-}
-
 // ---------------------------------------------------------------- writing
 
+/// A zip package being written: every entry deflated, with one fixed timestamp.
 pub struct ZipOut {
-    zip: zip::ZipWriter<Cursor<Vec<u8>>>,
+    zip: craft_opc::ZipOut,
 }
 
 impl ZipOut {
     pub fn new() -> ZipOut {
-        ZipOut { zip: zip::ZipWriter::new(Cursor::new(Vec::new())) }
+        ZipOut { zip: craft_opc::ZipOut::new() }
     }
     pub fn add(&mut self, name: &str, data: &[u8]) -> Result<(), IoError> {
-        let opts = zip::write::SimpleFileOptions::default()
-            .compression_method(zip::CompressionMethod::Deflated)
-            .compression_level(Some(6))
-            .last_modified_time(zip::DateTime::default())
-            .large_file(data.len() as u64 >= u32::MAX as u64);
-        self.zip.start_file(name, opts).map_err(|e| IoError::Zip(e.to_string()))?;
-        self.zip.write_all(data).map_err(|e| IoError::Zip(e.to_string()))?;
-        Ok(())
+        self.zip.add(name, data, craft_opc::Compression::Deflate).map_err(error)
     }
     pub fn finish(self) -> Result<Vec<u8>, IoError> {
-        self.zip.finish().map(|c| c.into_inner()).map_err(|e| IoError::Zip(e.to_string()))
+        self.zip.finish().map_err(error)
     }
 }
 
@@ -232,43 +144,4 @@ mod tests {
         assert_eq!(split_dir("xl/workbook.xml"), ("xl", "workbook.xml"));
         assert_eq!(split_dir("x.xml"), ("", "x.xml"));
     }
-
-    /// An over-budget read must spend the package's allowance once and for all: loops over many
-    /// broken parts (a sheet's optional VML relationships, for one) otherwise re-decompress the
-    /// remaining allowance again and again.
-    #[test]
-    fn a_failed_part_ends_the_package_budget() {
-        let z = crate::tests::make_zip(&[("a.xml", b"0123456789"), ("b.xml", b"hello")]);
-        let mut pkg = Package::open(&z).unwrap();
-        assert!(pkg.read("a.xml").unwrap().is_some());
-        // Nothing left to spend within the per-part limit either.
-        pkg.total = MAX_TOTAL - 1;
-        assert!(pkg.read("b.xml").is_err());
-        assert_eq!(pkg.total, MAX_TOTAL, "the failed read is charged");
-        // Oversized repeated attempts stay cheap errors now: no allowance is left to pay for.
-        assert!(matches!(pkg.read("b.xml"), Err(IoError::TooLarge(_))));
-    }
-
-    /// A part the reader can do without, damaged inside an intact archive, costs its own
-    /// allowance: the parts after it are still read.
-    #[test]
-    fn a_damaged_optional_part_costs_only_its_allowance() {
-        use std::io::Write as _;
-        let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
-        let stored = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
-        w.start_file("a.xml", stored).unwrap();
-        w.write_all(b"<a>damaged</a>").unwrap();
-        w.start_file("b.xml", zip::write::SimpleFileOptions::default()).unwrap();
-        w.write_all(b"hello").unwrap();
-        let mut z = w.finish().unwrap().into_inner();
-        let at = z.windows(7).position(|x| x == b"damaged").unwrap();
-        z[at] ^= 0x20;
-        let mut pkg = Package::open(&z).unwrap();
-        assert!(matches!(pkg.read_optional_limited("a.xml", 1024), Err(IoError::Zip(_))));
-        assert_eq!(pkg.total, 1025, "the failed read is charged its allowance");
-        assert_eq!(pkg.read("b.xml").unwrap().as_deref(), Some(&b"hello"[..]));
-        // An ordinary read of it still ends the package's budget.
-        assert!(pkg.read("a.xml").is_err());
-        assert_eq!(pkg.total, MAX_TOTAL);
-    }
 }

crates/xlsx/src/xls.rs

@@ -126,6 +126,10 @@ pub(crate) fn read_xls_budgeted(bytes: &[u8], limits: x::Limits, functions: Func
         let sheet = sheet_of(s, &styles, &mut unparsed);
         wb.sheets.push(Arc::new(sheet));
     }
+    // craft-xls reads the rules; GridCraft does not take them from .xls yet.
+    if book.sheets.iter().any(|s| !s.validations.is_empty()) {
+        report.warn("Data validation in .xls files is not imported yet.");
+    }
     if unparsed {
         report.warn("Some formulas use syntax GridCraft can't parse yet; their saved results were kept as values.");
     }