Merge pull request #3844 from storytold/switch-craft-libs-xls-dv

xlsx: .xls data validation from craft-xls

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/tests.rs

@@ -32,6 +32,7 @@ mod sheet_data_pin;
 mod sheet_names;
 mod smooth_roundtrip;
 mod trendline_roundtrip;
+mod xls_dv_tests;
 mod xls_style_tests;
 mod xls_tests;
 mod xlsb_tests;

crates/xlsx/src/tests/xls_dv_tests.rs

@@ -0,0 +1,355 @@
+//! `.xls` data validation: DVal and Dv records ([MS-XLS] 2.4.96, 2.4.95). Every record is
+//! assembled here from the spec's structures; no Excel-produced file is used. The expected
+//! rules are what the `.xlsx` reader gives for the same rule.
+
+use gridcraft_core::{CellRef, RangeRef};
+use gridcraft_model::{CfOperator, ErrorStyle, Validation, ValidationKind, Workbook};
+
+use super::xls_tests::{cat, compound, rec, u16s};
+use crate::{ReadReport, read_xls};
+
+fn bof(dt: u16) -> Vec<u8> {
+    rec(0x0809, &cat(&[&u16s(&[0x0600, dt, 0x0DBB, 0x07CC]), &[0; 8]]))
+}
+
+fn eof() -> Vec<u8> {
+    rec(0x000A, &[])
+}
+
+/// A workbook of one worksheet, "Sheet1", holding `records`, with defined names (Lbl records,
+/// each a name and its formula) in the globals.
+fn workbook(names: &[(&str, &[u8])], records: &[Vec<u8>]) -> (Workbook, ReadReport) {
+    let lbl = |name: &str, rgce: &[u8]| {
+        rec(0x0018, &cat(&[&u16s(&[0]), &[0, name.len() as u8], &u16s(&[rgce.len() as u16, 0, 0]), &[0; 4], &[0], name.as_bytes(), rgce]))
+    };
+    let lbls: Vec<u8> = names.iter().flat_map(|(n, f)| lbl(n, f)).collect();
+    let globals = |pos: u32| cat(&[&bof(0x0005), &rec(0x0085, &cat(&[&pos.to_le_bytes(), &[0, 0, 6, 0], b"Sheet1"])), &lbls, &eof()]);
+    let body: Vec<u8> = records.concat();
+    let sheet = cat(&[&bof(0x0010), &body, &eof()]);
+    let stream = cat(&[&globals(globals(0).len() as u32), &sheet]);
+    read_xls(&compound("Workbook", &stream)).unwrap()
+}
+
+/// DVal: flags, the input window's position, no drop-down object (-1), then `count` rules.
+fn dval(count: u32) -> Vec<u8> {
+    rec(0x01B2, &cat(&[&[0, 0], &[0; 8], &(-1i32).to_le_bytes(), &count.to_le_bytes()]))
+}
+
+// Dv flag bits ([MS-XLS] 2.4.95).
+const STR_LOOKUP: u32 = 1 << 7;
+const ALLOW_BLANK: u32 = 1 << 8;
+const SUPPRESS_COMBO: u32 = 1 << 9;
+const SHOW_INPUT: u32 = 1 << 18;
+const SHOW_ERROR: u32 = 1 << 19;
+/// What Excel sets on a new rule: blanks allowed, input message and error alert shown.
+const DEFAULTS: u32 = ALLOW_BLANK | SHOW_INPUT | SHOW_ERROR;
+
+/// Dv flags: rule type, alert style, operator, plus `bits`.
+fn flags(kind: u32, alert: u32, op: u32, bits: u32) -> u32 {
+    kind | (alert << 4) | (op << 20) | bits
+}
+
+/// XLUnicodeString with compressed characters; "" is stored as Excel stores an empty
+/// message, a single NUL character.
+fn msg(s: &str) -> Vec<u8> {
+    let s = if s.is_empty() { "\0" } else { s };
+    cat(&[&(s.len() as u16).to_le_bytes(), &[0], s.as_bytes()])
+}
+
+/// XLUnicodeString with UTF-16 characters.
+fn wide(s: &str) -> Vec<u8> {
+    let units: Vec<u16> = s.encode_utf16().collect();
+    cat(&[&(units.len() as u16).to_le_bytes(), &[1], &u16s(&units)])
+}
+
+/// DVParsedFormula: size, 2 unused bytes (Excel leaves stray data there), the tokens.
+fn formula(rgce: &[u8]) -> Vec<u8> {
+    cat(&[&(rgce.len() as u16).to_le_bytes(), &[0x6E, 0x74], rgce])
+}
+
+/// SqRefU: a count, then each Ref8U as rows (first, last) and columns (first, last).
+fn sqref(ranges: &[[u16; 4]]) -> Vec<u8> {
+    let list: Vec<u8> = ranges.iter().flat_map(|r| u16s(r)).collect();
+    cat(&[&(ranges.len() as u16).to_le_bytes(), &list])
+}
+
+/// A Dv record with its four messages (prompt title, error title, prompt, error).
+fn dv(flags: u32, msgs: [Vec<u8>; 4], f1: &[u8], f2: &[u8], ranges: &[[u16; 4]]) -> Vec<u8> {
+    let [pt, et, p, e] = msgs;
+    rec(0x01BE, &cat(&[&flags.to_le_bytes(), &pt, &et, &p, &e, &formula(f1), &formula(f2), &sqref(ranges)]))
+}
+
+fn no_msgs() -> [Vec<u8>; 4] {
+    [msg(""), msg(""), msg(""), msg("")]
+}
+
+/// A Dv with Excel's default flags and no messages.
+fn rule(kind: u32, alert: u32, op: u32, f1: &[u8], f2: &[u8], ranges: &[[u16; 4]]) -> Vec<u8> {
+    dv(flags(kind, alert, op, DEFAULTS), no_msgs(), f1, f2, ranges)
+}
+
+fn range(a1: &str) -> RangeRef {
+    RangeRef::parse(a1).unwrap()
+}
+
+/// The `.xlsx` reader's result for a rule with Excel's default flags and no messages.
+fn expected(ranges: &[&str], kind: ValidationKind, op: CfOperator, f1: &str, f2: Option<&str>) -> Validation {
+    Validation { ranges: ranges.iter().map(|r| range(r)).collect(), kind, op, f1: f1.into(), f2: f2.map(Into::into), ..Validation::default() }
+}
+
+fn hex(s: &str) -> Vec<u8> {
+    (0..s.len()).step_by(2).map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()).collect()
+}
+
+const SKIPPED: &str = "Some data validation rules in .xls files could not be read and were skipped.";
+const IGNORED: &str = "Some data validation records in .xls files were ignored, as Excel ignores them: they have no DVal record before them, or come after the number of rules it gives.";
+
+/// Each kind of rule, as Excel writes it, gives what the `.xlsx` reader gives for the same rule.
+#[test]
+fn reads_every_kind_of_rule() {
+    use CfOperator::*;
+    use ValidationKind::*;
+    let rules = [
+        // Whole number between 1 and 10, with all four messages and the stop alert.
+        dv(
+            flags(1, 0, 0, DEFAULTS),
+            [msg("Whole"), msg("Oops"), msg("Enter 1 to 10"), msg("Not 1..10")],
+            &hex("1e0100"),
+            &hex("1e0a00"),
+            &[[1, 5, 1, 1]],
+        ),
+        // Decimal >= 0.5, warning alert.
+        rule(2, 1, 6, &hex("1f000000000000e03f"), &[], &[[1, 5, 2, 2]]),
+        // Literal list (NUL-separated items), information alert.
+        dv(flags(3, 2, 0, DEFAULTS | STR_LOOKUP), no_msgs(), &hex("170e0052656400477265656e00426c7565"), &[], &[[1, 5, 3, 3]]),
+        // List from an absolute range ($H$2:$H$5).
+        rule(3, 0, 0, &hex("250100040007000700"), &[], &[[1, 5, 4, 4]]),
+        // Date between two serials, time < 0.5, text length <= 5, whole not between 5 and 7.
+        rule(4, 0, 0, &hex("1e37ab"), &hex("1ea4ac"), &[[1, 5, 5, 5]]),
+        rule(5, 0, 5, &hex("1f000000000000e03f"), &[], &[[1, 5, 6, 6]]),
+        rule(6, 0, 7, &hex("1e0500"), &[], &[[1, 5, 8, 8]]),
+        rule(1, 0, 1, &hex("1e0500"), &hex("1e0700"), &[[1, 5, 9, 9]]),
+        // Custom formula K2>A2 on K2:K6 (relative PtgRefN offsets 0 and -10 columns).
+        rule(7, 0, 0, &hex("4c000000c04c0000f6c00d"), &[], &[[1, 5, 10, 10]]),
+        // Any value, with an input message only.
+        dv(flags(0, 0, 0, DEFAULTS), [msg("Note"), msg(""), msg("Anything goes"), msg("")], &[], &[], &[[1, 5, 13, 13]]),
+        // A list from a defined name.
+        rule(3, 0, 0, &hex("2301000000"), &[], &[[1, 5, 16, 16]]),
+        // A dependent list: INDIRECT($S2) on T2:T6.
+        rule(3, 0, 0, &hex("190100004c0000128022019400"), &[], &[[1, 5, 19, 19]]),
+        // Decimal between two absolute cells.
+        rule(2, 0, 0, &hex("4406000700"), &hex("4407000700"), &[[1, 5, 22, 22]]),
+    ];
+    let mut records = vec![dval(rules.len() as u32)];
+    records.extend(rules);
+    let (wb, report) = workbook(&[("Colors", &[0x1E, 1, 0])], &records);
+    let got = &wb.sheets[0].validations;
+    let mut whole = expected(&["B2:B6"], Whole, Between, "1", Some("10"));
+    (whole.input_title, whole.error_title, whole.input_message, whole.error_message) =
+        ("Whole".into(), "Oops".into(), "Enter 1 to 10".into(), "Not 1..10".into());
+    let mut decimal = expected(&["C2:C6"], Decimal, GreaterOrEqual, "0.5", None);
+    decimal.error_style = ErrorStyle::Warning;
+    let mut literal = expected(&["D2:D6"], List, Between, "\"Red,Green,Blue\"", None);
+    literal.error_style = ErrorStyle::Information;
+    let mut any = expected(&["N2:N6"], Any, Between, "", None);
+    (any.input_title, any.input_message) = ("Note".into(), "Anything goes".into());
+    let want = vec![
+        whole,
+        decimal,
+        literal,
+        expected(&["E2:E6"], List, Between, "=$H$2:$H$5", None),
+        expected(&["F2:F6"], Date, Between, "43831", Some("44196")),
+        expected(&["G2:G6"], Time, Less, "0.5", None),
+        expected(&["I2:I6"], TextLength, LessOrEqual, "5", None),
+        expected(&["J2:J6"], Whole, NotBetween, "5", Some("7")),
+        expected(&["K2:K6"], Custom, Between, "K2>A2", None),
+        any,
+        expected(&["Q2:Q6"], List, Between, "=Colors", None),
+        expected(&["T2:T6"], List, Between, "=INDIRECT($S2)", None),
+        expected(&["W2:W6"], Decimal, Between, "$H$7", Some("$H$8")),
+    ];
+    assert_eq!(got.len(), want.len(), "{:?}", report.warnings);
+    for (g, w) in got.iter().zip(&want) {
+        assert_eq!(g, w);
+    }
+    assert!(!report.warnings.iter().any(|w| w == SKIPPED), "{:?}", report.warnings);
+    assert!(report.warnings[0].contains("data validation were read"), "{:?}", report.warnings);
+}
+
+/// The flags that switch features off; an operator stored on a list (where it means nothing);
+/// messages in UTF-16; literal items with quotes and spaces; a whole column.
+#[test]
+fn reads_flags_messages_lists_and_whole_columns() {
+    let off = flags(3, 0, 0, STR_LOOKUP | SUPPRESS_COMBO);
+    let records = [
+        dval(4),
+        dv(off, no_msgs(), &hex("170300610062"), &[], &[[1, 5, 14, 14]]),
+        dv(
+            flags(1, 0, 2, DEFAULTS),
+            [wide("Café"), wide("Ошибка"), wide("日本語のテスト é ü"), wide("Nur 3 ✓")],
+            &hex("1e0300"),
+            &[],
+            &[[1, 5, 20, 20]],
+        ),
+        // typOperator 6 on a list, as found in real files: ignored.
+        dv(flags(3, 0, 6, DEFAULTS | STR_LOOKUP), no_msgs(), &hex("170c00736179202268692200627965"), &[], &[[1, 5, 23, 23]]),
+        // Rows 0-65535: every row of the BIFF8 grid.
+        dv(flags(3, 0, 0, DEFAULTS | STR_LOOKUP), no_msgs(), &hex("170500780079007a"), &[], &[[0, 0xFFFF, 21, 21]]),
+    ];
+    let (wb, report) = workbook(&[], &records);
+    let got = &wb.sheets[0].validations;
+    assert_eq!(got.len(), 4, "{:?}", report.warnings);
+    let mut flags_off = expected(&["O2:O6"], ValidationKind::List, CfOperator::Between, "\"a,b\"", None);
+    (flags_off.allow_blank, flags_off.in_cell_dropdown, flags_off.show_input, flags_off.show_error) = (false, false, false, false);
+    assert_eq!(got[0], flags_off);
+    let mut unicode = expected(&["U2:U6"], ValidationKind::Whole, CfOperator::Equal, "3", None);
+    (unicode.input_title, unicode.error_title, unicode.input_message, unicode.error_message) =
+        ("Café".into(), "Ошибка".into(), "日本語のテスト é ü".into(), "Nur 3 ✓".into());
+    assert_eq!(got[1], unicode);
+    // The item `say "hi"` keeps its quotes, doubled as in `.xlsx`; the operator is ignored.
+    assert_eq!(got[2], expected(&["X2:X6"], ValidationKind::List, CfOperator::Between, "\"say \"\"hi\"\",bye\"", None));
+    let column = &got[3];
+    assert_eq!(column.ranges, vec![RangeRef::cols(21, 21)]);
+    assert!(column.ranges[0].is_full_cols());
+}
+
+/// Relative references are offsets from the top-left cell of all the rule's ranges, whatever
+/// order the ranges are listed in (AF5:AF9 then AG1:AG3 has its formula written for AF1). An
+/// offset Excel couldn't keep inside the grid is stored as PtgRefErr and reads as #REF!.
+#[test]
+fn relative_references_start_from_the_top_left_of_all_ranges() {
+    let records = [
+        dval(3),
+        // =AE1>0 for AF5:AF9 AG1:AG3 (column offset -1 from AF1).
+        rule(7, 0, 0, &hex("4c0000ffc01e00000d"), &[], &[[4, 8, 31, 31], [0, 2, 32, 32]]),
+        // =AB1>0 for AC1:AC3 AA5:AA9 (column offset +1 from AA1).
+        rule(7, 0, 0, &hex("4c000001c01e00000d"), &[], &[[0, 2, 28, 28], [4, 8, 26, 26]]),
+        // PtgRefErr: a reference that fell off the grid.
+        rule(7, 0, 0, &hex("2a0000f4c01e00000d"), &[], &[[4, 8, 12, 12], [0, 2, 11, 11]]),
+    ];
+    let (wb, report) = workbook(&[], &records);
+    let got = &wb.sheets[0].validations;
+    assert_eq!(got.len(), 3, "{:?}", report.warnings);
+    assert_eq!(got[0].ranges, vec![range("AF5:AF9"), range("AG1:AG3")]);
+    assert_eq!(got[0].f1, "AE1>0");
+    assert_eq!(got[1].f1, "AB1>0");
+    assert_eq!(got[2].f1, "#REF!>0");
+}
+
+/// As in Excel, a DVal's count is authoritative: Dv records past it, or with no DVal before
+/// them, aren't read. That's said once, and not as a rule that failed to read.
+#[test]
+fn only_the_dvals_count_of_rules_is_read() {
+    let a = || rule(1, 0, 4, &hex("1e0000"), &[], &[[0, 0, 0, 0]]);
+    let b = || rule(1, 0, 4, &hex("1e0000"), &[], &[[1, 1, 0, 0]]);
+    let (wb, report) = workbook(&[], &[dval(1), a(), b(), b()]);
+    assert_eq!(wb.sheets[0].validations.iter().map(|v| v.ranges.clone()).collect::<Vec<_>>(), vec![vec![range("A1")]]);
+    assert_eq!(report.warnings.iter().filter(|w| *w == IGNORED).count(), 1, "{:?}", report.warnings);
+    assert!(!report.warnings.iter().any(|w| w == SKIPPED), "{:?}", report.warnings);
+    let (wb, report) = workbook(&[], &[a(), b()]);
+    assert!(wb.sheets[0].validations.is_empty());
+    assert!(report.warnings.iter().any(|w| w == IGNORED), "{:?}", report.warnings);
+    assert!(!report.warnings.iter().any(|w| w == SKIPPED), "{:?}", report.warnings);
+}
+
+/// Malformed rules never fail the import. Where Excel opens such a file, the result is what
+/// Excel shows; where Excel refuses the whole file, that one rule is skipped with a warning.
+#[test]
+fn malformed_rules_are_repaired_or_skipped() {
+    let ok = |row: u16| rule(1, 0, 4, &hex("1e0000"), &[], &[[row, row, 0, 0]]);
+    let cases: Vec<(&str, Vec<u8>, Option<&str>)> = vec![
+        // Excel opens an unknown type as "any value".
+        ("unknown type", rule(9, 0, 0, &hex("1e0100"), &[], &[[1, 5, 1, 1]]), Some("B2:B6")),
+        // Excel refuses an unknown alert style, or an unknown operator on a rule that uses one.
+        ("unknown alert", rule(1, 5, 0, &hex("1e0100"), &hex("1e0a00"), &[[1, 5, 1, 1]]), None),
+        ("unknown operator", rule(1, 0, 9, &hex("1e0100"), &[], &[[1, 5, 1, 1]]), None),
+        // Excel drops a reversed range, and clamps columns past IV.
+        ("reversed range", rule(1, 0, 4, &hex("1e0000"), &[], &[[5, 1, 2, 2]]), None),
+        ("past column IV", rule(1, 0, 4, &hex("1e0000"), &[], &[[1, 5, 3, 0x100]]), Some("D2:IV6")),
+        // Excel refuses a range count or a formula size that runs past the record.
+        (
+            "too many ranges",
+            rec(
+                0x01BE,
+                &cat(&[
+                    &flags(1, 0, 4, DEFAULTS).to_le_bytes(),
+                    &no_msgs().concat(),
+                    &formula(&hex("1e0000")),
+                    &formula(&[]),
+                    &u16s(&[2, 1, 5, 4, 4]),
+                ]),
+            ),
+            None,
+        ),
+        (
+            "formula past the record",
+            rec(0x01BE, &cat(&[&flags(1, 0, 4, DEFAULTS).to_le_bytes(), &no_msgs().concat(), &u16s(&[200, 0]), &hex("1e0000")])),
+            None,
+        ),
+        ("truncated message", rec(0x01BE, &cat(&[&flags(1, 0, 4, DEFAULTS).to_le_bytes(), &u16s(&[40]), &[0], b"short"])), None),
+        ("empty record", rec(0x01BE, &[]), None),
+        // A formula GridCraft can't decode (an unknown token).
+        ("unknown token", rule(7, 0, 0, &[0xFF], &[], &[[1, 5, 1, 1]]), None),
+        // No ranges at all.
+        ("no ranges", rule(1, 0, 4, &hex("1e0000"), &[], &[]), None),
+    ];
+    for (what, bad, want) in cases {
+        let (wb, report) = workbook(&[], &[dval(3), ok(10), bad, ok(11)]);
+        let got = &wb.sheets[0].validations;
+        let ranges: Vec<RangeRef> = got.iter().flat_map(|v| v.ranges.clone()).collect();
+        let mut expect = vec![range("A11")];
+        expect.extend(want.map(range));
+        expect.push(range("A12"));
+        assert_eq!(ranges, expect, "{what}: {:?}", report.warnings);
+        assert_eq!(report.warnings.iter().any(|w| w == SKIPPED), want.is_none(), "{what}: {:?}", report.warnings);
+        if what == "unknown type" {
+            assert_eq!((got[1].kind, got[1].f1.as_str(), got[1].f2.as_deref()), (ValidationKind::Any, "", None));
+        }
+    }
+}
+
+/// Hostile counts and sizes never panic or allocate from the file's numbers.
+#[test]
+fn hostile_validation_records_never_panic() {
+    let f = flags(1, 0, 4, DEFAULTS).to_le_bytes();
+    let big = [
+        rec(0x01B2, &[0xFF; 18]),
+        rec(0x01BE, &cat(&[&f, &u16s(&[0xFFFF]), &[1]])),
+        rec(0x01BE, &cat(&[&f, &no_msgs().concat(), &u16s(&[0xFFFF, 0xFFFF])])),
+        rec(0x01BE, &cat(&[&f, &no_msgs().concat(), &formula(&[]), &formula(&[]), &u16s(&[0xFFFF])])),
+        // A literal list whose PtgStr claims 255 UTF-16 characters and holds none.
+        rec(
+            0x01BE,
+            &cat(&[
+                &flags(3, 0, 0, DEFAULTS | STR_LOOKUP).to_le_bytes(),
+                &no_msgs().concat(),
+                &formula(&[0x17, 0xFF, 1]),
+                &formula(&[]),
+                &sqref(&[[0, 0, 0, 0]]),
+            ]),
+        ),
+        // Every flag bit set: type 15, alert 7, operator 15.
+        rec(0x01BE, &cat(&[&[0xFF; 4], &no_msgs().concat(), &formula(&[]), &formula(&[]), &sqref(&[[0, 0, 0, 0]])])),
+    ];
+    let (wb, _) = workbook(&[], &big);
+    assert!(wb.sheets[0].validations.is_empty());
+    let mut many = vec![dval(u32::MAX)];
+    many.extend((0..200u16).map(|r| rule(1, 0, 4, &hex("1e0000"), &[], &[[r, r, 0, 0]])));
+    let (wb, _) = workbook(&[], &many);
+    assert_eq!(wb.sheets[0].validations.len(), 200);
+    assert_eq!(wb.sheets[0].validations[199].ranges, vec![RangeRef::cell(CellRef::new(199, 0))]);
+}
+
+/// Rules with no messages cost no text, so a crafted file could otherwise keep millions of them:
+/// the workbook keeps 65,536 and says it skipped the rest.
+#[test]
+fn rules_past_the_workbook_limit_are_skipped() {
+    let one = rule(0, 0, 0, &[], &[], &[[0, 0, 0, 0]]);
+    let mut records = vec![dval(65_537)];
+    records.extend(std::iter::repeat_n(one, 65_537));
+    let (wb, report) = workbook(&[], &records);
+    assert_eq!(wb.sheets[0].validations.len(), 65_536);
+    // craft-xls counts the rules past its cap with the ones it could not read.
+    assert_eq!(report.warnings.iter().filter(|w| *w == SKIPPED).count(), 1, "{:?}", report.warnings);
+    assert!(!report.warnings.iter().any(|w| w == IGNORED), "{:?}", report.warnings);
+}

crates/xlsx/src/tests/xls_tests.rs

@@ -133,18 +133,18 @@ pub(crate) fn compound(name: &str, data: &[u8]) -> Vec<u8> {
     out
 }
 
-fn rec(id: u16, payload: &[u8]) -> Vec<u8> {
+pub(super) fn rec(id: u16, payload: &[u8]) -> Vec<u8> {
     let mut v = id.to_le_bytes().to_vec();
     v.extend_from_slice(&(payload.len() as u16).to_le_bytes());
     v.extend_from_slice(payload);
     v
 }
 
-fn cat(parts: &[&[u8]]) -> Vec<u8> {
+pub(super) fn cat(parts: &[&[u8]]) -> Vec<u8> {
     parts.concat()
 }
 
-fn u16s(v: &[u16]) -> Vec<u8> {
+pub(super) fn u16s(v: &[u16]) -> Vec<u8> {
     v.iter().flat_map(|x| x.to_le_bytes()).collect()
 }
 

crates/xlsx/src/xls.rs

@@ -13,8 +13,9 @@ use craft_xls as x;
 use gridcraft_core::{CellError, CellRef, DateSystem, RangeRef, Value};
 use gridcraft_model::style::Protection;
 use gridcraft_model::{
-    Alignment, BorderLine, BorderStyle, Borders, CalcMode, CalcSettings, Cell, Color, DefinedName, Fill, Font, Formula, HAlign, LineInfo, NumFmt,
-    PatternType, Sheet, Style, StyleId, StyleTable, Underline, VAlign, VertAlign, Visibility, Workbook,
+    Alignment, BorderLine, BorderStyle, Borders, CalcMode, CalcSettings, Cell, CfOperator, Color, DefinedName, ErrorStyle, Fill, Font, Formula,
+    HAlign, LineInfo, NumFmt, PatternType, Sheet, Style, StyleId, StyleTable, Underline, VAlign, Validation, ValidationKind, VertAlign, Visibility,
+    Workbook,
 };
 
 use crate::tables::{col_width_to_px, pt_to_px};
@@ -72,8 +73,8 @@ fn warning(w: &x::Warning) -> String {
         W::Charts => "Charts in .xls files are not imported yet.".into(),
         W::PicturesSkipped => "Some pictures in .xls files were skipped.".into(),
         W::ConditionalFormattingSkipped | W::ConditionalFormatting12 => "Some conditional formatting in .xls files was not imported.".into(),
-        W::DataValidationSkipped => "Some data validation in .xls files could not be read and was skipped.".into(),
-        W::DataValidationIgnored => "Some data validation records in .xls files were ignored, as Excel ignores them.".into(),
+        W::DataValidationSkipped => "Some data validation rules in .xls files could not be read and were skipped.".into(),
+        W::DataValidationIgnored => "Some data validation records in .xls files were ignored, as Excel ignores them: they have no DVal record before them, or come after the number of rules it gives.".into(),
         W::Hyperlinks => "Hyperlinks in .xls files are not imported yet.".into(),
         W::PivotTables => "PivotTables in .xls files are not imported yet; their values were kept.".into(),
         W::CellsOutsideGrid(n) => format!("{n} cells outside the grid were skipped."),
@@ -113,15 +114,15 @@ pub(crate) fn read_xls_budgeted(bytes: &[u8], limits: x::Limits, functions: Func
     }
     let mut unparsed = false;
     for s in &book.sheets {
-        // craft-xls keeps these rules in its read model, but GridCraft's XLS adapter does not
-        // import conditional formatting. Its own warnings cover rules it could not decode; this
-        // reports the additional rules that this adapter drops.
-        if !s.conditional_formats.is_empty() {
-            report.warn(warning(&x::Warning::ConditionalFormattingSkipped));
-        }
-        // Data validation is not yet imported by this adapter.
-        if !s.validations.is_empty() {
-            report.warn(warning(&x::Warning::DataValidationSkipped));
+        // Report rules craft-xls kept that the adapter cannot represent. craft-xls already warns
+        // about records it could not decode.
+        for (dropped, skipped) in [
+            (!s.conditional_formats.is_empty(), x::Warning::ConditionalFormattingSkipped),
+            (s.validations.iter().any(|v| validation_of(v).is_none()), x::Warning::DataValidationSkipped),
+        ] {
+            if dropped {
+                report.warn(warning(&skipped));
+            }
         }
         let sheet = sheet_of(s, &styles, &mut unparsed);
         wb.sheets.push(Arc::new(sheet));
@@ -145,7 +146,7 @@ pub(crate) fn read_xls_budgeted(bytes: &[u8], limits: x::Limits, functions: Func
     // The overview comes first.
     report.warnings.insert(
         0,
-        "Imported from an Excel 97-2003 (.xls) workbook: values, formulas, cell formatting, merged cells, column widths, row heights and defined names were read. Charts, pictures, comments and other features are not imported yet. Save as .xlsx; saving .xls is not supported.".into(),
+        "Imported from an Excel 97-2003 (.xls) workbook: values, formulas, cell formatting, merged cells, column widths, row heights, defined names and data validation were read. Charts, pictures, comments and other features are not imported yet. Save as .xlsx; saving .xls is not supported.".into(),
     );
     Ok((wb, report))
 }
@@ -257,6 +258,7 @@ fn sheet_of(s: &x::Sheet, styles: &[StyleId], unparsed: &mut bool) -> Sheet {
         sheet.show_zeros = v.show_zeros;
         sheet.right_to_left = v.right_to_left;
     }
+    sheet.validations = s.validations.iter().filter_map(validation_of).collect();
     if let Some(text) = &s.print_area {
         sheet.print.print_area = crate::read::parse_print_area(text);
     }
@@ -268,6 +270,59 @@ fn sheet_of(s: &x::Sheet, styles: &[StyleId], unparsed: &mut bool) -> Sheet {
     sheet
 }
 
+/// A data validation rule, as `sheet_read` maps a SpreadsheetML `dataValidation`: a list that
+/// is not a literal refers to cells (`=…`).
+fn validation_of(v: &x::DataValidation) -> Option<Validation> {
+    let ranges: Vec<RangeRef> = v.ranges.iter().map(|r| range(*r)).collect();
+    if ranges.is_empty() {
+        return None;
+    }
+    let kind = match v.kind {
+        x::ValidationType::Whole => ValidationKind::Whole,
+        x::ValidationType::Decimal => ValidationKind::Decimal,
+        x::ValidationType::List => ValidationKind::List,
+        x::ValidationType::Date => ValidationKind::Date,
+        x::ValidationType::Time => ValidationKind::Time,
+        x::ValidationType::TextLength => ValidationKind::TextLength,
+        x::ValidationType::Custom => ValidationKind::Custom,
+        _ => ValidationKind::Any,
+    };
+    let f1 = if kind == ValidationKind::List && !v.formula1.is_empty() && !v.formula1.starts_with('"') {
+        format!("={}", v.formula1)
+    } else {
+        v.formula1.clone()
+    };
+    Some(Validation {
+        ranges,
+        kind,
+        op: match v.operator {
+            x::Operator::NotBetween => CfOperator::NotBetween,
+            x::Operator::Equal => CfOperator::Equal,
+            x::Operator::NotEqual => CfOperator::NotEqual,
+            x::Operator::Greater => CfOperator::Greater,
+            x::Operator::Less => CfOperator::Less,
+            x::Operator::GreaterOrEqual => CfOperator::GreaterOrEqual,
+            x::Operator::LessOrEqual => CfOperator::LessOrEqual,
+            _ => CfOperator::Between,
+        },
+        f1,
+        f2: v.formula2.clone(),
+        allow_blank: v.allow_blank,
+        in_cell_dropdown: v.in_cell_dropdown,
+        input_title: v.prompt_title.clone(),
+        input_message: v.prompt.clone(),
+        show_input: v.show_prompt,
+        error_title: v.error_title.clone(),
+        error_message: v.error.clone(),
+        error_style: match v.error_style {
+            x::ErrorStyle::Warning => ErrorStyle::Warning,
+            x::ErrorStyle::Information => ErrorStyle::Information,
+            _ => ErrorStyle::Stop,
+        },
+        show_error: v.show_error,
+    })
+}
+
 fn color(c: x::Color) -> Color {
     match c {
         x::Color::Rgb(rgb) => Color::Rgb(rgb),

docs/file-format-parity.md

@@ -1,6 +1,6 @@
 # File-format parity
 
-> **Last reviewed:** 2026-11-01 · **Last updated:** 2026-11-12 · **Change:** minor (.xls is read by the shared craft-xls crate: the workbook opens on the sheet it was saved on, and rich-text (RString) cells are read; earlier: form controls: buttons, scroll bars and spin buttons read and written; earlier: a web page's first table opens as a sheet, also when the page is named `.xls`; earlier: `workbookPr filterPrivacy` kept, and honoured on save; earlier: a workbook's R1C1 reference style, `calcPr refMode`, is kept through open and save; earlier: XLSX page margins are written as their decimals, `0.3` rather than `0.30000001192092896`; earlier: Python cells in `.xlsx` as [MS-XLSX] specifies, `_xlfn._xlws.PY(…)` naming code in the Python part, with cached values, and in GridCraft JSON, with limits on open; earlier: equations in xlsx as Office Math with a marker in `descr` so an edit made in Excel reads back, read back only up to the equation source cap, and a brace with a label and a script on its other side keeps both; Symbol font in PDF; earlier: XLSX reads and writes Page Setup's page order, `pageSetup pageOrder`, and PDF export follows it; earlier: chart data tables: native c:dTable import/export; earlier: XLSX workbook-view selection, normalized tab ratios and sheet-edit regressions; fidelity estimates unchanged; earlier: formulas from Google Sheets' XLSX export open as the original formula instead of `IFERROR(__xludf.DUMMYFUNCTION(…),…)` when GridCraft calculates every function in it, and save as it; `file.exportCsv {values: true}` writes raw values for automation; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; CSV and text import detects likely Windows-1256 Arabic when invalid UTF-8 has Arabic byte runs; other invalid UTF-8 remains Windows-1252; HTML export places pictures and charts, as inline SVG, at their anchors; earlier: `file.exportCsv {values: true}` writes raw values for automation; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; CSV and text import detects likely Windows-1256 Arabic when invalid UTF-8 has Arabic byte runs; other invalid UTF-8 remains Windows-1252; HTML export places pictures and charts, as inline SVG, at their anchors; CSV and text open detects each column's date order from values that read only one way, warns when every date is ambiguous, and keeps a column with both kinds as text (#2765)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
+> **Last reviewed:** 2026-11-01 · **Last updated:** 2026-11-12 · **Change:** minor (.xls data validation is read; earlier: .xls is read by the shared craft-xls crate: the workbook opens on the sheet it was saved on, and rich-text (RString) cells are read; earlier: form controls: buttons, scroll bars and spin buttons read and written; earlier: a web page's first table opens as a sheet, also when the page is named `.xls`; earlier: `workbookPr filterPrivacy` kept, and honoured on save; earlier: a workbook's R1C1 reference style, `calcPr refMode`, is kept through open and save; earlier: XLSX page margins are written as their decimals, `0.3` rather than `0.30000001192092896`; earlier: Python cells in `.xlsx` as [MS-XLSX] specifies, `_xlfn._xlws.PY(…)` naming code in the Python part, with cached values, and in GridCraft JSON, with limits on open; earlier: equations in xlsx as Office Math with a marker in `descr` so an edit made in Excel reads back, read back only up to the equation source cap, and a brace with a label and a script on its other side keeps both; Symbol font in PDF; earlier: XLSX reads and writes Page Setup's page order, `pageSetup pageOrder`, and PDF export follows it; earlier: chart data tables: native c:dTable import/export; earlier: XLSX workbook-view selection, normalized tab ratios and sheet-edit regressions; fidelity estimates unchanged; earlier: formulas from Google Sheets' XLSX export open as the original formula instead of `IFERROR(__xludf.DUMMYFUNCTION(…),…)` when GridCraft calculates every function in it, and save as it; `file.exportCsv {values: true}` writes raw values for automation; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; CSV and text import detects likely Windows-1256 Arabic when invalid UTF-8 has Arabic byte runs; other invalid UTF-8 remains Windows-1252; HTML export places pictures and charts, as inline SVG, at their anchors; earlier: `file.exportCsv {values: true}` writes raw values for automation; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; CSV and text import detects likely Windows-1256 Arabic when invalid UTF-8 has Arabic byte runs; other invalid UTF-8 remains Windows-1252; HTML export places pictures and charts, as inline SVG, at their anchors; CSV and text open detects each column's date order from values that read only one way, warns when every date is ambiguous, and keeps a column with both kinds as text (#2765)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
 
 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests.
 Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h**
@@ -92,7 +92,7 @@ past the parser's limit, and a wrapper inside the text. **Saving writes the real
 | `.xlsm` Macro-Enabled Workbook | R/W | yes | yes, macros kept, not run | The VBA project is kept byte-for-byte and written back only into `.xlsm`/`.xltm` (#6); macros never run. Saved with the macro-enabled workbook content type | `roundtrip.rs` (`vba_project_roundtrip`, `package_kind_sets_workbook_content_type`), engine `io.rs` (`macros_are_saved_only_into_xlsm`) |
 | `.xltx` / `.xltm` templates | R/W | yes | partial | Saved with the template (and macro-enabled template) content type, chosen by extension; `.xltm` keeps a VBA project, `.xltx` drops it; "new from template" behaviour missing | `roundtrip.rs` (`package_kind_sets_workbook_content_type`), engine `io.rs` |
 | `.xlsb` Binary Workbook | R/W | **values only** (#101) | no | Names, visibility, numbers, text, booleans, errors, merges, cached formula results; no formulas, formatting, charts, names | `xlsb_tests.rs` |
-| `.xls` Excel 97-2003 (BIFF8) | R/W | **yes, partial** (#121) | no (Save As `.xlsx`) | Clean-room from [MS-CFB] and [MS-XLS]. Read: worksheets and visibility, shared strings, numbers, text, booleans, errors, formulas (text for the common tokens, shared and array formulas, 3-D refs, names, newer functions called by name) with cached results, number formats (dates display as dates), cell formatting (fonts, solid and pattern fills, borders with diagonals, alignment with rotation, indent, wrap and shrink, locked and hidden, the custom colour palette, the Normal font as the workbook default, user-defined cell styles), 1904 date system, merges, column widths, row heights, defined names, print area and titles, calculation settings (mode, iteration with its maximum iterations and change, precision as displayed), sheet display options (right-to-left, gridlines, headings, zero values, formulas; a truncated WINDOW2 is ignored). Every formula except a Ctrl+Shift+Enter array is a legacy one and opens with Excel's implicit `@`, as an `.xlsx` formula without `cm` does (#410); Ctrl+Shift+Enter arrays, one cell or several, stay arrays, and saving as `.xlsx` writes the formulas without `cm`. Not read: exact colours beyond the 56-colour palette (XFExt: theme colours and tints show as the palette colour Excel stored next to them), rich-text runs inside a cell, charts, pictures, comments, CF, validation, hyperlinks, freeze panes, external links (formula kept as its value). Chart and macro sheets skipped with a warning; encrypted (FILEPASS) and BIFF5 (Excel 5.0/95) files refused with a message. Opens as a new unsaved workbook. Unverified on a real-world corpus | `xls_tests.rs`, `xls_style_tests.rs` |
+| `.xls` Excel 97-2003 (BIFF8) | R/W | **yes, partial** (#121) | no (Save As `.xlsx`) | Clean-room from [MS-CFB] and [MS-XLS]. Read: worksheets and visibility, shared strings, numbers, text, booleans, errors, formulas (text for the common tokens, shared and array formulas, 3-D refs, names, newer functions called by name) with cached results, number formats (dates display as dates), cell formatting (fonts, solid and pattern fills, borders with diagonals, alignment with rotation, indent, wrap and shrink, locked and hidden, the custom colour palette, the Normal font as the workbook default, user-defined cell styles), 1904 date system, merges, column widths, row heights, defined names, print area and titles, calculation settings (mode, iteration with its maximum iterations and change, precision as displayed), data validation rules, sheet display options (right-to-left, gridlines, headings, zero values, formulas; a truncated WINDOW2 is ignored). Every formula except a Ctrl+Shift+Enter array is a legacy one and opens with Excel's implicit `@`, as an `.xlsx` formula without `cm` does (#410); Ctrl+Shift+Enter arrays, one cell or several, stay arrays, and saving as `.xlsx` writes the formulas without `cm`. Not read: exact colours beyond the 56-colour palette (XFExt: theme colours and tints show as the palette colour Excel stored next to them), rich-text runs inside a cell, charts, pictures, comments, CF, hyperlinks, freeze panes, external links (formula kept as its value). Chart and macro sheets skipped with a warning; encrypted (FILEPASS) and BIFF5 (Excel 5.0/95) files refused with a message. Opens as a new unsaved workbook. Unverified on a real-world corpus | `xls_tests.rs`, `xls_style_tests.rs`, `xls_dv_tests.rs` |
 | `.xlt` Excel 97-2003 template | R/W | yes (read as `.xls`) | no | | — |
 | `.xlam` / `.xla` add-ins | R/W | no | no | VBA; out of scope | — |
 | `.xml` XML Spreadsheet 2003 | R/W | no | no | Simple to support | — |
@@ -161,6 +161,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here.
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-11-12 | minor | `.xls` files open with their data validation (drop-down lists, input messages, error alerts and the rules), read by craft-xls and mapped as the `.xlsx` reader maps the same rules. A rule Excel would refuse, or one past 65,536 in a workbook, is skipped with a warning (#3634, from #3636) |
 | 2026-11-12 | minor | .xls import reads the file with craft-xls (storytold/craft-libs), the reader that moved there from this repository; GridCraft keeps the mapping onto its model. The workbook now opens on the sheet it was saved on (WINDOW1 `itabCur`), falling back to the first visible sheet as before (from #3551), and cells saved as RString records (text with formatting runs) are read as text instead of skipped |
 | 2026-11-10 | minor | `.xls` import: every formula except a Ctrl+Shift+Enter array is read as a legacy one with Excel's implicit `@` (a title row `=Other!A1:G1` shows its first cell instead of `#SPILL!`), one-cell Ctrl+Shift+Enter arrays stay arrays, and `@` of a 2-D range meets the formula's row and column (#410, PR #1367) |
 | 2026-11-02 | minor | XLSX: buttons, scroll bars and spin buttons read and written (control properties parts, `<controls>`, VML `x:ClientData`); other form controls stay with pass-through; a button's command, fixed input or output cell travels in a hidden defined name and comes back only while the file still assigns the button's macro (or none, for a command) |