craft-xls: comments, conditional formats, pictures and the AutoFilter range (port of gridcraft#3457) (#1005)

* Add craft-sheet: the cell positions, values and tables the spreadsheet formats share

craft-xls carried its own CellRef, Range, Value, CellError, Visibility, sheet-name
quoting, built-in number format table and indexed palette. craft-xlsx and craft-ods
need the same ones, so they move to a crate with no dependencies and craft-xls
re-exports them: an application gets one Value and one CellRef whichever file it read.

Added on the way: A1 parsing (CellRef::parse, Range::parse), Range::count and a1,
CellError::parse and the newer error values SpreadsheetML stores as text,
builtin_format_id for writers.

* craft-xls: read data validation (DVal and Dv records)

Each Dv after a DVal becomes a DataValidation in SpreadsheetML's terms: type,
operator, formulas as A1 text from the top left cell of all the rule's ranges (a
literal list quoted, items separated by commas), the four messages, alert style
and flags. Exactly the DVal's count of rules is read, as Excel does; a rule Excel
would refuse is skipped with a warning, and the rest are kept. A sheet keeps at
most 65,536 rules (the count was not capped on its own).

Ported from storytold/gridcraft#3636.

* craft-xls: read comments, conditional formats, pictures and the AutoFilter range

- Notes with their TxO text, author and visibility.
- CondFmt and CF records (cell value is, formula is), formulas from the bounding
  range's top left cell, and the DXFN as a DifferentialFormat whose unset parts are
  None.
- The drawing group's BLIP store (PNG, JPEG, EMF, WMF with its placeable header,
  DIB as BMP) and each sheet's picture shapes with their client anchors.
- The AutoFilter range from AutoFilterInfo and _FilterDatabase.

From the review of the source PR: picture bytes are shared by the shapes that show
them, pictures, comments and rules are capped per sheet, inflated metafiles share
one allowance, damaged object and CF records are skipped with a warning instead of
failing the read, and CF formulas are relative to the CondFmt's bounding range.
ValidationOperator is now Operator, shared with conditional formats.

Ported from storytold/gridcraft#3457.

---------

Co-authored-by: kiwi4boy <kiwi4boy@gmail.com>
Co-authored-by: BrianDo35 <haison.db35@icloud.com>

Cargo.lock

@@ -2027,6 +2027,7 @@ version = "0.1.0"
 dependencies = [
  "craft-cfb",
  "craft-sheet",
+ "miniz_oxide 0.9.1",
  "thiserror 2.0.21",
 ]
 

crates/xls/Cargo.toml

@@ -13,4 +13,9 @@ workspace = true
 [dependencies]
 craft-cfb = { workspace = true }
 craft-sheet = { workspace = true }
+# zlib-wrapped metafiles in the drawing group.
+miniz_oxide = "0.9"
 thiserror = { workspace = true }
+
+[dev-dependencies]
+miniz_oxide = "0.9"

crates/xls/README.md

@@ -34,7 +34,22 @@ No third-party implementation was consulted.
   and flags, in SpreadsheetML's terms. Exactly the DVal's count of rules is read, as Excel does;
   a rule Excel would refuse is skipped with a warning, and at most 65,536 are kept per sheet.
 
-Not read: charts, pictures, comments, conditional formats, hyperlinks,
+- Cell comments: each Note's cell, author and visibility, with its text object's text (Obj,
+  TxO and its Continue records).
+- Conditional formats: CondFmt ranges with their "cell value is" and "formula is" CF rules,
+  formulas relative to the bounding range, and the DXFN as a `DifferentialFormat` whose unset
+  parts are `None`.
+- Pictures: the drawing group's BLIP store and each sheet's picture shapes with their client
+  anchors (cells and fractions): PNG, JPEG, EMF and WMF (inflated, a WMF given its placeable
+  header back), and DIB as BMP. Shapes that show one picture share its bytes.
+- The AutoFilter range (AutoFilterInfo with the `_FilterDatabase` name); not its criteria.
+
+Limits for these: 256 MiB of drawing records for the workbook and for each sheet, 4096 stored
+pictures, 256 MiB of inflated metafiles in all (64 MiB each), 65,536 pictures, comments and
+conditional formats per sheet. A damaged comment, object or conditional-format record is
+skipped with a warning.
+
+Not read: charts, other shapes, newer conditional formats (CF12), hyperlinks,
 PivotTables, macros. Each is named once in `Read::warnings`. Encrypted workbooks
 (`Error::Encrypted`) and BIFF5 and older (`Error::Biff5`) are refused. There is no writer
 (`craft_xls::testing` builds small fixtures for tests).

crates/xls/src/cond.rs

@@ -0,0 +1,148 @@
+//! Conditional formatting: a CondFmt record names the ranges ([MS-XLS] 2.4.56), and each CF
+//! record after it is one rule (2.4.42): "cell value is" with an operator and one or two
+//! formulas, or a formula that is true. Its look is a DXFN (2.5.81), where every attribute it
+//! leaves unset ("ninch") is left as the cell has it.
+
+use craft_sheet::{CellRef, Range, builtin_format};
+
+use crate::Error;
+use crate::model::{BorderLine, DifferentialFormat, DxfFill, Operator, Underline};
+use crate::records::Cursor;
+use crate::styles::{border_style, color, pattern};
+
+/// Ranges one CondFmt record lists at most.
+pub(crate) const MAX_CF_RANGES: usize = 4096;
+/// Conditional formats kept per sheet; more are skipped with a warning.
+pub(crate) const MAX_RULES: usize = 65_536;
+
+/// A CondFmt record: the top left cell of its bounding range (what the rules' relative
+/// references are offsets from) and its ranges.
+pub(crate) fn condfmt(c: &mut Cursor<'_, '_>) -> Result<(CellRef, Vec<Range>), Error> {
+    // Rule count, then flags and an id.
+    c.skip(4)?;
+    // refBound: rows (first, last), columns (first, last).
+    let (r0, _, c0, _) = (c.u16()?, c.u16()?, c.u16()?, c.u16()?);
+    let n = usize::from(c.u16()?).min(MAX_CF_RANGES).min(c.remaining() / 8);
+    let mut out = Vec::with_capacity(n);
+    for _ in 0..n {
+        let (r0, r1, c0, c1) = (c.u16()?, c.u16()?, c.u16()?, c.u16()?);
+        out.push(Range::new(CellRef::new(u32::from(r0), u32::from(c0)), CellRef::new(u32::from(r1), u32::from(c1))));
+    }
+    Ok((CellRef::new(u32::from(r0), u32::from(c0)), out))
+}
+
+/// One CF record, before its formulas are decoded.
+pub(crate) struct Cf {
+    /// 1: the cell value compared; 2: a formula.
+    pub kind: u8,
+    pub operator: Option<Operator>,
+    pub format: DifferentialFormat,
+    pub rgce1: Vec<u8>,
+    pub rgce2: Vec<u8>,
+}
+
+/// Reads a CF record: its kind, operator, look and the two formulas' tokens. `num_fmt` gives the
+/// code of a number format by id.
+pub(crate) fn cf(c: &mut Cursor<'_, '_>, palette: &[u32; 64], num_fmt: impl Fn(u16) -> Option<String>) -> Result<Cf, Error> {
+    let kind = c.u8()?;
+    let operator = match c.u8()? {
+        1 => Some(Operator::Between),
+        2 => Some(Operator::NotBetween),
+        3 => Some(Operator::Equal),
+        4 => Some(Operator::NotEqual),
+        5 => Some(Operator::Greater),
+        6 => Some(Operator::Less),
+        7 => Some(Operator::GreaterOrEqual),
+        8 => Some(Operator::LessOrEqual),
+        _ => None,
+    };
+    let (cce1, cce2) = (usize::from(c.u16()?), usize::from(c.u16()?));
+    let format = dxfn(c, palette, num_fmt)?;
+    let rest = c.rest();
+    let rgce1 = rest.get(..cce1).unwrap_or(&[]).to_vec();
+    let rgce2 = rest.get(cce1..cce1.saturating_add(cce2)).unwrap_or(&[]).to_vec();
+    Ok(Cf { kind, operator, format, rgce1, rgce2 })
+}
+
+fn bit(flags: u32, n: u32) -> bool {
+    flags & (1 << n) != 0
+}
+
+/// A DXFN: which blocks follow (number format, font, alignment, border, pattern, protection)
+/// and, per attribute, whether it is left unset.
+fn dxfn(c: &mut Cursor<'_, '_>, palette: &[u32; 64], num_fmt: impl Fn(u16) -> Option<String>) -> Result<DifferentialFormat, Error> {
+    let flags = c.u32()?;
+    let flags2 = c.u16()?;
+    let mut d = DifferentialFormat::default();
+    if bit(flags, 25) {
+        if flags2 & 1 != 0 {
+            // DXFNumUsr: its size, then the format code.
+            c.u16()?;
+            d.number_format = Some(c.string()?);
+        } else {
+            c.u8()?;
+            let id = u16::from(c.u8()?);
+            d.number_format = num_fmt(id).or_else(|| builtin_format(u32::from(id)).map(str::to_string));
+        }
+    }
+    if bit(flags, 26) {
+        // DXFFntD, 118 bytes: name (64), height, options (italic 0x02, strikeout 0x80), weight,
+        // script, underline, colour, then which of those are unset. As in a SpreadsheetML
+        // differential font, the face and size stay the cell's.
+        let mut b = [0u8; 118];
+        for slot in b.iter_mut() {
+            *slot = c.u8()?;
+        }
+        let u32_at = |at: usize| b.get(at..at + 4).and_then(|x| <[u8; 4]>::try_from(x).ok()).map_or(u32::MAX, u32::from_le_bytes);
+        let u16_at = |at: usize| b.get(at..at + 2).and_then(|x| <[u8; 2]>::try_from(x).ok()).map_or(0, u16::from_le_bytes);
+        let (options, weight, underline, icv) = (u32_at(68), u16_at(72), b.get(76).copied().unwrap_or(0), u32_at(80));
+        let (style_unset, underline_unset, weight_unset) = (u32_at(88), u32_at(96) != 0, u32_at(100) != 0);
+        if !weight_unset {
+            d.bold = Some(weight >= 600);
+        }
+        if style_unset & 0x02 == 0 {
+            d.italic = Some(options & 0x02 != 0);
+        }
+        if style_unset & 0x80 == 0 {
+            d.strike = Some(options & 0x80 != 0);
+        }
+        if !underline_unset {
+            d.underline = Some(match underline {
+                1 => Underline::Single,
+                2 => Underline::Double,
+                0x21 => Underline::SingleAccounting,
+                0x22 => Underline::DoubleAccounting,
+                _ => Underline::None,
+            });
+        }
+        // 0xFFFFFFFF: the colour is unset.
+        if let Ok(icv) = u16::try_from(icv) {
+            d.font_color = Some(color(icv, palette));
+        }
+    }
+    if bit(flags, 27) {
+        c.skip(8)?;
+    }
+    if bit(flags, 28) {
+        let (a, b) = (c.u32()?, c.u32()?);
+        let line = |unset: bool, dg: u32, icv: u32| {
+            (!unset).then(|| BorderLine { style: border_style(dg & 0x0F), color: color((icv & 0x7F) as u16, palette) })
+        };
+        d.left = line(bit(flags, 10), a, a >> 16);
+        d.right = line(bit(flags, 11), a >> 4, a >> 23);
+        d.top = line(bit(flags, 12), a >> 8, b);
+        d.bottom = line(bit(flags, 13), a >> 12, b >> 7);
+    }
+    if bit(flags, 29) {
+        let (p1, p2) = (c.u16()?, c.u16()?);
+        d.fill = Some(DxfFill {
+            pattern: (!bit(flags, 16)).then(|| pattern((p1 >> 10) & 0x3F)),
+            fg: (!bit(flags, 17)).then(|| color(p2 & 0x7F, palette)),
+            bg: (!bit(flags, 18)).then(|| color((p2 >> 7) & 0x7F, palette)),
+        });
+    }
+    if bit(flags, 30) {
+        c.skip(2)?;
+    }
+    Ok(d)
+}

crates/xls/src/drawing.rs

@@ -0,0 +1,336 @@
+//! Pictures, from the Office Drawing records ([MS-ODRAW]): the workbook's
+//! MsoDrawingGroup holds the pictures (a BStore of FBSE entries, each with its BLIP), and each
+//! sheet's MsoDrawing records hold its shapes; a shape whose properties name a BLIP (`pib`,
+//! 0x0104) and that has a client anchor (cells and fractions of them) is a picture.
+
+use std::sync::Arc;
+
+use crate::model::ClientAnchor;
+
+/// Pictures read from one workbook at most.
+const MAX_BLIPS: usize = 4096;
+/// Bytes inflated from all of a workbook's metafiles together.
+const MAX_METAFILES: usize = 256 << 20;
+/// Nesting of drawing containers followed at most.
+const MAX_DEPTH: usize = 16;
+
+/// One stored picture: its bytes (shared by every shape that shows it) and MIME type, or `None`
+/// for a format that is not read (PICT, TIFF) or a damaged one, so that `pib` indexes still
+/// line up.
+pub(crate) type Blip = Option<(Arc<Vec<u8>>, &'static str)>;
+
+/// An Office Drawing record header: version/instance, type, length.
+fn header(b: &[u8], at: usize) -> Option<(u16, u16, usize)> {
+    let h = b.get(at..at.checked_add(8)?)?;
+    let vi = u16::from_le_bytes([*h.first()?, *h.get(1)?]);
+    let ty = u16::from_le_bytes([*h.get(2)?, *h.get(3)?]);
+    let len = u32::from_le_bytes([*h.get(4)?, *h.get(5)?, *h.get(6)?, *h.get(7)?]);
+    Some((vi, ty, usize::try_from(len).ok()?))
+}
+
+/// The records directly inside `b` (a container's body): (version/instance, type, body).
+fn children(b: &[u8]) -> Vec<(u16, u16, &[u8])> {
+    let mut out = Vec::new();
+    let mut at = 0usize;
+    while let Some((vi, ty, len)) = header(b, at) {
+        let start = at + 8;
+        // A record claiming more than is there keeps what is there: the last one in a buffer
+        // is often cut where its sheet's drawing records end.
+        let end = start.saturating_add(len).min(b.len());
+        out.push((vi, ty, b.get(start..end).unwrap_or(&[])));
+        at = end;
+    }
+    out
+}
+
+/// The pictures of the workbook's MsoDrawingGroup, in BStore order (`pib` 1 is the first).
+pub(crate) fn blip_store(dgg: &[u8]) -> Vec<Blip> {
+    let mut out = Vec::new();
+    let mut inflate_left = MAX_METAFILES;
+    for (_, ty, body) in children(dgg) {
+        if ty != 0xF000 {
+            continue;
+        }
+        for (_, ty, store) in children(body) {
+            if ty != 0xF001 {
+                continue;
+            }
+            for (_, ty, fbse) in children(store) {
+                if ty == 0xF007 && out.len() < MAX_BLIPS {
+                    out.push(fbse_blip(fbse, &mut inflate_left).map(|(b, m)| (Arc::new(b), m)));
+                }
+            }
+        }
+    }
+    out
+}
+
+/// A picture's bytes and MIME type.
+type Bytes = Option<(Vec<u8>, &'static str)>;
+
+/// The picture of one FBSE: 36 bytes of description, its name, then the BLIP record.
+/// `inflate_left` is what the workbook's metafiles may still inflate to.
+fn fbse_blip(fbse: &[u8], inflate_left: &mut usize) -> Bytes {
+    let name_len = usize::from(*fbse.get(33)?);
+    let rest = fbse.get(36usize.checked_add(name_len)?..)?;
+    let (vi, ty, len) = header(rest, 0)?;
+    let data = rest.get(8..8usize.checked_add(len)?.min(rest.len()))?;
+    let instance = vi >> 4;
+    // EMF and WMF: one or two identifiers, a metafile header, then the (usually zlib) bytes.
+    match (ty, instance) {
+        (0xF01A, 0x3D4) => return metafile(data, 1, false, inflate_left),
+        (0xF01A, 0x3D5) => return metafile(data, 2, false, inflate_left),
+        (0xF01B, 0x216) => return metafile(data, 1, true, inflate_left),
+        (0xF01B, 0x217) => return metafile(data, 2, true, inflate_left),
+        // DIB: identifiers, a tag byte, then a BITMAPINFOHEADER and the pixels.
+        (0xF01F, 0x7A8) => return dib_as_bmp(data.get(17..)?),
+        (0xF01F, 0x7A9) => return dib_as_bmp(data.get(33..)?),
+        _ => {}
+    }
+    // PNG and JPEG keep one or two 16-byte identifiers and a tag byte before the bytes.
+    let (uids, mime) = match (ty, instance) {
+        (0xF01E, 0x6E0) => (1, "image/png"),
+        (0xF01E, 0x6E1) => (2, "image/png"),
+        (0xF01D | 0xF02A, 0x46A | 0x6E2) => (1, "image/jpeg"),
+        (0xF01D | 0xF02A, 0x46B | 0x6E3) => (2, "image/jpeg"),
+        _ => return None,
+    };
+    let bytes = data.get(16 * uids + 1..)?;
+    (!bytes.is_empty()).then(|| (bytes.to_vec(), mime))
+}
+
+/// Largest metafile inflated from one BLIP.
+const MAX_METAFILE: usize = 64 << 20;
+
+/// An EMF or WMF BLIP's file: after `uids` identifiers comes the 34-byte OfficeArtMetafileHeader
+/// (uncompressed size, bounds, size in EMUs, saved size, compression 0 = DEFLATE in a zlib wrapper
+/// or 0xFE = none, filter), then the saved bytes. A WMF is stored without its placeable header;
+/// one is put back from the bounds so the picture keeps its size.
+fn metafile(data: &[u8], uids: usize, wmf: bool, inflate_left: &mut usize) -> Bytes {
+    let h = data.get(16 * uids..16 * uids + 34)?;
+    let u32_at = |at: usize| Some(u32::from_le_bytes(h.get(at..at + 4)?.try_into().ok()?));
+    let i32_at = |at: usize| Some(i32::from_le_bytes(h.get(at..at + 4)?.try_into().ok()?));
+    let size = usize::try_from(u32_at(0)?).ok()?;
+    let saved = usize::try_from(u32_at(28)?).ok()?;
+    let body = data.get(16 * uids + 34..)?;
+    let body = body.get(..saved.min(body.len()))?;
+    let bytes = match *h.get(32)? {
+        0x00 => {
+            let out = miniz_oxide::inflate::decompress_to_vec_zlib_with_limit(body, size.min(MAX_METAFILE).min(*inflate_left)).ok()?;
+            *inflate_left = inflate_left.saturating_sub(out.len());
+            out
+        }
+        0xFE => body.to_vec(),
+        _ => return None,
+    };
+    if bytes.is_empty() {
+        return None;
+    }
+    if !wmf {
+        return Some((bytes, "image/x-emf"));
+    }
+    // Bounds in the metafile's units, size in EMUs (914400 an inch): its units per inch.
+    let (l, t, r, b) = (i32_at(4)?, i32_at(8)?, i32_at(12)?, i32_at(16)?);
+    let width_emu = f64::from(i32_at(20)?);
+    let units = f64::from(r.saturating_sub(l));
+    let inch = if width_emu > 0.0 && units > 0.0 { (units * 914_400.0 / width_emu).round().clamp(1.0, 65_535.0) as u16 } else { 1440 };
+    let clamp16 = |v: i32| v.clamp(i32::from(i16::MIN), i32::from(i16::MAX)) as i16;
+    let mut out = Vec::with_capacity(22 + bytes.len());
+    out.extend_from_slice(&0x9AC6_CDD7u32.to_le_bytes());
+    out.extend_from_slice(&0u16.to_le_bytes());
+    for v in [l, t, r, b] {
+        out.extend_from_slice(&clamp16(v).to_le_bytes());
+    }
+    out.extend_from_slice(&inch.to_le_bytes());
+    out.extend_from_slice(&0u32.to_le_bytes());
+    // Checksum: the XOR of the ten words before it.
+    let sum = out.chunks(2).fold(0u16, |x, w| x ^ u16::from_le_bytes([w[0], *w.get(1).unwrap_or(&0)]));
+    out.extend_from_slice(&sum.to_le_bytes());
+    out.extend_from_slice(&bytes);
+    Some((out, "image/x-wmf"))
+}
+
+/// A packed DIB (header, palette, pixels) as a BMP file: the 14-byte file header in front, its
+/// pixel offset counted from the info header's size and palette.
+fn dib_as_bmp(dib: &[u8]) -> Bytes {
+    let u32_at = |at: usize| Some(u32::from_le_bytes(dib.get(at..at + 4)?.try_into().ok()?));
+    let header = u32_at(0)?;
+    if !(12..=124).contains(&header) || dib.len() <= header as usize {
+        return None;
+    }
+    let palette = if header == 12 {
+        0
+    } else {
+        let bits = u16::from_le_bytes(dib.get(14..16)?.try_into().ok()?);
+        let used = u32_at(32)?;
+        let colors = if used > 0 {
+            used.min(256)
+        } else if bits <= 8 {
+            1 << bits
+        } else {
+            0
+        };
+        // BI_BITFIELDS with a 40-byte header keeps its three masks after it.
+        let masks = if header == 40 && u32_at(16)? == 3 { 12 } else { 0 };
+        colors * 4 + masks
+    };
+    let offset = 14 + header + palette;
+    let total = u32::try_from(dib.len()).ok()?.checked_add(14)?;
+    let mut out = Vec::with_capacity(total as usize);
+    out.extend_from_slice(b"BM");
+    out.extend_from_slice(&total.to_le_bytes());
+    out.extend_from_slice(&0u32.to_le_bytes());
+    out.extend_from_slice(&offset.to_le_bytes());
+    out.extend_from_slice(dib);
+    Some((out, "image/bmp"))
+}
+
+/// A picture shape: its BLIP (1-based) and client anchor.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) struct PictureShape {
+    pub pib: u32,
+    pub anchor: ClientAnchor,
+}
+
+/// The picture shapes in a sheet's drawing (its MsoDrawing records joined).
+pub(crate) fn picture_shapes(dg: &[u8]) -> Vec<PictureShape> {
+    let mut out = Vec::new();
+    walk(dg, 0, &mut out);
+    out
+}
+
+fn walk(b: &[u8], depth: usize, out: &mut Vec<PictureShape>) {
+    if depth > MAX_DEPTH {
+        return;
+    }
+    for (vi, ty, body) in children(b) {
+        if ty == 0xF004 {
+            if let Some(p) = shape(body) {
+                out.push(p);
+            }
+        } else if vi & 0x0F == 0x0F {
+            walk(body, depth + 1, out);
+        }
+    }
+}
+
+/// A shape container's BLIP and anchor, when it has both.
+fn shape(sp: &[u8]) -> Option<PictureShape> {
+    let (mut pib, mut anchor) = (None, None);
+    for (vi, ty, body) in children(sp) {
+        match ty {
+            // Properties: `instance` of them, six bytes each (id with flags, value).
+            0xF00B => {
+                for p in body.chunks(6).take(usize::from(vi >> 4)) {
+                    if let [a, b, c, d, e, f] = *p
+                        && u16::from_le_bytes([a, b]) & 0x3FFF == 0x0104
+                    {
+                        pib = Some(u32::from_le_bytes([c, d, e, f]));
+                    }
+                }
+            }
+            // Client anchor: flags, then eight 16-bit values.
+            0xF010 => {
+                let mut v = [0u16; 8];
+                for (i, slot) in v.iter_mut().enumerate() {
+                    let at = 2 + i * 2;
+                    *slot = u16::from_le_bytes([*body.get(at)?, *body.get(at + 1)?]);
+                }
+                let [col1, dx1, row1, dy1, col2, dx2, row2, dy2] = v;
+                anchor = Some(ClientAnchor { col1, dx1, row1, dy1, col2, dx2, row2, dy2 });
+            }
+            _ => {}
+        }
+    }
+    Some(PictureShape { pib: pib.filter(|p| *p > 0)?, anchor: anchor? })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn rec(vi: u16, ty: u16, body: &[u8]) -> Vec<u8> {
+        [&vi.to_le_bytes()[..], &ty.to_le_bytes(), &(body.len() as u32).to_le_bytes(), body].concat()
+    }
+
+    #[test]
+    fn pictures_and_their_anchors_are_found_and_hostile_drawings_are_harmless() {
+        let png = [0x89, b'P', b'N', b'G'];
+        let blip = rec(0x6E00, 0xF01E, &[&[0u8; 17][..], &png].concat());
+        let fbse = rec(0x0062, 0xF007, &[&[0u8; 36][..], &blip].concat());
+        let wmf = rec(0x0062, 0xF007, &[&[0u8; 36][..], &rec(0x2160, 0xF01B, &[0; 60])].concat());
+        let dgg = rec(0x000F, 0xF000, &rec(0x002F, 0xF001, &[fbse, wmf].concat()));
+        let store = blip_store(&dgg);
+        assert_eq!(store.len(), 2);
+        assert_eq!(store[0], Some((Arc::new(png.to_vec()), "image/png")));
+        assert_eq!(store[1], None, "a damaged metafile keeps its slot");
+        // A shape with pib 1 and an anchor from B2 (half way across) to D5.
+        let opt = rec(0x0013, 0xF00B, &[&0x4104u16.to_le_bytes()[..], &1u32.to_le_bytes()].concat());
+        let anchor_body: Vec<u8> = [0u16, 1, 512, 1, 0, 3, 0, 4, 0].iter().flat_map(|v| v.to_le_bytes()).collect();
+        let sp = rec(0x000F, 0xF004, &[opt, rec(0x0000, 0xF010, &anchor_body)].concat());
+        let dg = rec(0x000F, 0xF002, &rec(0x000F, 0xF003, &sp));
+        let anchor = ClientAnchor { col1: 1, dx1: 512, row1: 1, dy1: 0, col2: 3, dx2: 0, row2: 4, dy2: 0 };
+        assert_eq!(picture_shapes(&dg), vec![PictureShape { pib: 1, anchor }]);
+        // Truncated or nonsense drawings give nothing, never a panic.
+        for bad in [&[][..], &[0xFF; 8][..], &[0; 70][..]] {
+            assert_eq!(metafile(bad, 1, false, &mut usize::MAX.clone()), None);
+            let _ = metafile(bad, 2, true, &mut usize::MAX.clone());
+            let _ = dib_as_bmp(bad);
+        }
+        for bad in [&dg[..dg.len() - 5], &[0xFF; 40][..], &[][..], &dgg[..20]] {
+            let _ = picture_shapes(bad);
+            let _ = blip_store(bad);
+        }
+    }
+
+    #[test]
+    fn metafiles_share_one_inflate_allowance() {
+        // A zlib metafile of 1000 zero bytes, as an EMF BLIP with one identifier.
+        let raw = vec![0u8; 1000];
+        let z = miniz_oxide::deflate::compress_to_vec_zlib(&raw, 6);
+        let mut header = Vec::new();
+        for v in [1000u32, 0, 0, 10, 10, 0, 0, z.len() as u32] {
+            header.extend_from_slice(&v.to_le_bytes());
+        }
+        header.extend_from_slice(&[0x00, 0xFE]);
+        let data = [&[0u8; 16][..], &header, &z].concat();
+        let mut left = 1500;
+        assert_eq!(metafile(&data, 1, false, &mut left).map(|(b, _)| b.len()), Some(1000));
+        assert_eq!(left, 500);
+        // The second would inflate past what is left: refused, not cut.
+        assert_eq!(metafile(&data, 1, false, &mut left), None);
+    }
+
+    #[test]
+    fn a_wmf_gets_its_placeable_header_back_and_a_dib_becomes_a_bmp() {
+        // Bounds 0,0–2000×1000 units over 2 × 1 inches (EMUs): 1000 units an inch; stored as is.
+        let wmf = b"WMF records".to_vec();
+        let mut header = Vec::new();
+        for v in [wmf.len() as u32, 0, 0, 2000, 1000, 2 * 914_400, 914_400, wmf.len() as u32] {
+            header.extend_from_slice(&v.to_le_bytes());
+        }
+        header.extend_from_slice(&[0xFE, 0xFE]);
+        let data = [&[0u8; 16][..], &header, &wmf].concat();
+        let (bytes, mime) = metafile(&data, 1, true, &mut usize::MAX.clone()).expect("wmf");
+        assert_eq!(mime, "image/x-wmf");
+        assert_eq!(&bytes[..4], &0x9AC6_CDD7u32.to_le_bytes());
+        assert_eq!(u16::from_le_bytes([bytes[14], bytes[15]]), 1000, "units per inch");
+        let sum = bytes[..20].chunks(2).fold(0u16, |x, w| x ^ u16::from_le_bytes([w[0], w[1]]));
+        assert_eq!(u16::from_le_bytes([bytes[20], bytes[21]]), sum);
+        assert_eq!(&bytes[22..], &wmf[..]);
+        // A 1 × 1, 24-bit DIB: the pixels start right after the file and info headers.
+        let mut dib = Vec::new();
+        for v in [40u32, 1, 1] {
+            dib.extend_from_slice(&v.to_le_bytes());
+        }
+        dib.extend_from_slice(&1u16.to_le_bytes());
+        dib.extend_from_slice(&24u16.to_le_bytes());
+        dib.extend_from_slice(&[0; 24]);
+        dib.extend_from_slice(&[0, 0, 255, 0]);
+        let (bmp, mime) = dib_as_bmp(&dib).expect("bmp");
+        assert_eq!(mime, "image/bmp");
+        assert_eq!(&bmp[..2], b"BM");
+        assert_eq!(u32::from_le_bytes(bmp[10..14].try_into().unwrap()), 54);
+        assert_eq!(bmp.len(), 14 + dib.len());
+    }
+}

crates/xls/src/dv.rs

@@ -11,7 +11,7 @@
 use craft_sheet::{CellRef, MAX_COLS, MAX_ROWS, Range};
 
 use crate::formula::{Links, Mode, decode};
-use crate::model::{DataValidation, ErrorStyle, ValidationOperator, ValidationType};
+use crate::model::{DataValidation, ErrorStyle, Operator, ValidationType};
 use crate::records::Cursor;
 use crate::{Error, expr};
 
@@ -137,18 +137,18 @@ pub(crate) fn validation(dv: Dv, links: &Links) -> Option<DataValidation> {
     let compares = !matches!(kind, ValidationType::Any | ValidationType::List | ValidationType::Custom);
     let operator = if compares {
         match (f >> 20) & 0xF {
-            0 => ValidationOperator::Between,
-            1 => ValidationOperator::NotBetween,
-            2 => ValidationOperator::Equal,
-            3 => ValidationOperator::NotEqual,
-            4 => ValidationOperator::Greater,
-            5 => ValidationOperator::Less,
-            6 => ValidationOperator::GreaterOrEqual,
-            7 => ValidationOperator::LessOrEqual,
+            0 => Operator::Between,
+            1 => Operator::NotBetween,
+            2 => Operator::Equal,
+            3 => Operator::NotEqual,
+            4 => Operator::Greater,
+            5 => Operator::Less,
+            6 => Operator::GreaterOrEqual,
+            7 => Operator::LessOrEqual,
             _ => return None,
         }
     } else {
-        ValidationOperator::Between
+        Operator::Between
     };
     let text = |rgce: &[u8]| decode(rgce, &[], base, Mode::Cell, links).ok().map(|e| expr::print(&e));
     let formula1 = match kind {
@@ -160,11 +160,8 @@ pub(crate) fn validation(dv: Dv, links: &Links) -> Option<DataValidation> {
         },
         _ => text(&dv.rgce1)?,
     };
-    let formula2 = if compares && matches!(operator, ValidationOperator::Between | ValidationOperator::NotBetween) && !dv.rgce2.is_empty() {
-        Some(text(&dv.rgce2)?)
-    } else {
-        None
-    };
+    let formula2 =
+        if compares && matches!(operator, Operator::Between | Operator::NotBetween) && !dv.rgce2.is_empty() { Some(text(&dv.rgce2)?) } else { None };
     Some(DataValidation {
         ranges: dv.ranges,
         kind,

crates/xls/src/lib.rs

@@ -8,9 +8,11 @@
 //! (number formats, fonts, fills, borders, alignment, protection, with palette colours
 //! resolved), user-defined cell styles, merged cells, column and row records, sheet display
 //! flags, calculation settings, defined names, print areas and print titles, data validation
-//! rules, and the 1904 date flag. Widths and heights stay in the file's units.
+//! rules, cell comments, conditional formats ("cell value is" and "formula is"), pictures with
+//! their cell anchors, the AutoFilter range, and the 1904 date flag. Widths, heights and anchors
+//! stay in the file's units.
 //!
-//! Everything else (charts, pictures, comments, conditional formats, hyperlinks,
+//! Everything else (charts, other shapes, newer conditional formats, hyperlinks,
 //! PivotTables, macros) is skipped and named in [`Read::warnings`]. Encrypted workbooks and
 //! versions before BIFF8 are refused ([`Error::Encrypted`], [`Error::Biff5`]). A formula that
 //! cannot be put back into text (links to other workbooks, macro commands, unknown tokens)
@@ -30,11 +32,14 @@
 //! # }
 //! ```
 
+mod cond;
+mod drawing;
 mod dv;
 mod expr;
 mod formula;
 mod ftab;
 mod model;
+mod notes;
 mod read;
 mod records;
 mod styles;
@@ -43,9 +48,9 @@ pub mod testing;
 
 pub use craft_sheet::{DEFAULT_PALETTE, quote_sheet, sheet_needs_quotes};
 pub use model::{
-    Alignment, BorderLine, BorderStyle, Borders, Calc, CalcMode, Cell, CellError, CellFormat, CellRef, Color, Column, DataValidation, DefinedName,
-    ErrorStyle, Fill, Font, Formula, HAlign, Pattern, Range, Row, Script, Sheet, Underline, VAlign, ValidationOperator, ValidationType, Value, View,
-    Visibility, Workbook,
+    Alignment, BorderLine, BorderStyle, Borders, Calc, CalcMode, Cell, CellError, CellFormat, CellRef, ClientAnchor, Color, Column, ConditionRule,
+    ConditionalFormat, DataValidation, DefinedName, DifferentialFormat, DxfFill, ErrorStyle, Fill, Font, Formula, HAlign, Note, Operator, Pattern,
+    Picture, Range, Row, Script, Sheet, Underline, VAlign, ValidationType, Value, View, Visibility, Workbook,
 };
 pub use read::{is_xls, read, read_with};
 
@@ -87,12 +92,23 @@ pub enum Warning {
     MissingSharedString,
     #[error("what-if data tables were not read; their values were kept")]
     DataTable,
-    #[error("cell comments were not read")]
-    Comments,
-    #[error("pictures, charts and shapes were not read")]
-    Drawings,
-    #[error("conditional formatting was not read")]
-    ConditionalFormatting,
+    /// Some comment or drawing object records were shorter than they say and were skipped.
+    #[error("some comment or object records were damaged and skipped")]
+    DamagedObjects,
+    /// Charts embedded in a worksheet are not read.
+    #[error("charts were not read")]
+    Charts,
+    /// Pictures in a format that is not read (PICT, TIFF), damaged ones, ones past the limits,
+    /// and shapes that name a missing picture.
+    #[error("some pictures were damaged, in a format that is not read (PICT, TIFF), or past the limits, and were skipped")]
+    PicturesSkipped,
+    /// Some CF records could not be read: a rule type other than "cell value is" and "formula
+    /// is", a formula that could not be decoded, a damaged record, or more than the reader keeps.
+    #[error("some conditional formatting rules could not be read and were skipped")]
+    ConditionalFormattingSkipped,
+    /// CF12 records (Excel 2007 data bars, colour scales, icon sets) are not read.
+    #[error("newer conditional formats (data bars, colour scales, icon sets) were not read")]
+    ConditionalFormatting12,
     /// Some Dv records could not be read: Excel would refuse them (an unknown alert style or
     /// operator, a formula or range list past the record), no range was left, a formula could not
     /// be decoded, or the sheet had more rules than the reader keeps.

crates/xls/src/model.rs

@@ -2,6 +2,7 @@
 //! the format (widths and heights keep the file's units, formulas are text).
 
 use std::collections::BTreeMap;
+use std::sync::Arc;
 
 pub use craft_sheet::{BorderStyle, CellError, CellRef, HAlign, Pattern, Range, Script, Underline, VAlign, Value, Visibility};
 
@@ -105,6 +106,97 @@ pub struct Sheet {
     pub print_titles: Option<String>,
     /// Data validation rules (Dv records), in file order.
     pub validations: Vec<DataValidation>,
+    /// Cell comments (Note records with their text), in file order.
+    pub notes: Vec<Note>,
+    /// Conditional formats (CF records), in file order: the first has the highest priority.
+    pub conditional_formats: Vec<ConditionalFormat>,
+    /// Pictures placed on the sheet, in drawing order.
+    pub pictures: Vec<Picture>,
+    /// The AutoFilter's range: the sheet's `_FilterDatabase` name, when the sheet has an
+    /// AutoFilterInfo record (the name can outlive the filter). The criteria are not read.
+    pub autofilter: Option<Range>,
+}
+
+/// A cell comment (Note, [MS-XLS] 2.4.179, with the TxO text of its object).
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct Note {
+    pub cell: CellRef,
+    pub author: String,
+    /// Empty when the comment's text object is missing.
+    pub text: String,
+    /// Shown all the time, not only when the cell is pointed at.
+    pub visible: bool,
+}
+
+/// Where a picture sits: its top left and bottom right corners as a cell and a fraction of it
+/// (OfficeArtClientAnchorSheet, [MS-XLS] 2.5.193): `dx` in 1/1024 of the column's width, `dy`
+/// in 1/256 of the row's height.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+pub struct ClientAnchor {
+    pub col1: u16,
+    pub dx1: u16,
+    pub row1: u16,
+    pub dy1: u16,
+    pub col2: u16,
+    pub dx2: u16,
+    pub row2: u16,
+    pub dy2: u16,
+}
+
+/// A picture: a shape that names a stored picture and has a client anchor.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct Picture {
+    pub anchor: ClientAnchor,
+    /// The file: PNG, JPEG, EMF, WMF (with its placeable header put back) or BMP (a DIB with a
+    /// file header in front). Shapes that show one picture share its bytes.
+    pub data: Arc<Vec<u8>>,
+    /// `image/png`, `image/jpeg`, `image/x-emf`, `image/x-wmf` or `image/bmp`.
+    pub mime: &'static str,
+}
+
+/// What a conditional format tests.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum ConditionRule {
+    /// "Cell value is": the cell compared with one formula, or two for `Between` and
+    /// `NotBetween`.
+    CellIs { operator: Operator, formula1: String, formula2: Option<String> },
+    /// A formula that is true.
+    Expression { formula: String },
+}
+
+/// The fill a differential format sets: each part `None` when it leaves the cell's.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+pub struct DxfFill {
+    pub pattern: Option<Pattern>,
+    pub fg: Option<Color>,
+    pub bg: Option<Color>,
+}
+
+/// What a conditional format changes (DXFN, [MS-XLS] 2.5.81): each part `None` when it leaves
+/// the cell's. The font's face and size are never changed.
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub struct DifferentialFormat {
+    pub number_format: Option<String>,
+    pub bold: Option<bool>,
+    pub italic: Option<bool>,
+    pub strike: Option<bool>,
+    pub underline: Option<Underline>,
+    pub font_color: Option<Color>,
+    pub left: Option<BorderLine>,
+    pub right: Option<BorderLine>,
+    pub top: Option<BorderLine>,
+    pub bottom: Option<BorderLine>,
+    pub fill: Option<DxfFill>,
+}
+
+/// A conditional format (CondFmt with one of its CF records).
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct ConditionalFormat {
+    pub ranges: Vec<Range>,
+    /// Formulas are A1 text without `=`, relative references from the top left cell of the
+    /// CondFmt's bounding range.
+    pub rule: ConditionRule,
+    pub format: DifferentialFormat,
 }
 
 /// What a data validation rule allows ([MS-XLS] 2.4.95 `typOperator`'s sibling `valType`).
@@ -122,10 +214,10 @@ pub enum ValidationType {
     Custom,
 }
 
-/// How a rule compares the value with its formulas. Only whole, decimal, date, time and text
-/// length rules have one.
+/// How a value is compared with one or two formulas: in data validation (whole, decimal,
+/// date, time and text length rules) and in "cell value is" conditional formats.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
-pub enum ValidationOperator {
+pub enum Operator {
     #[default]
     Between,
     NotBetween,
@@ -155,7 +247,7 @@ pub struct DataValidation {
     pub ranges: Vec<Range>,
     pub kind: ValidationType,
     /// `Between` for the types without an operator.
-    pub operator: ValidationOperator,
+    pub operator: Operator,
     /// The first formula as A1 text without `=`, relative references from the top left cell of
     /// all the ranges. A literal list is quoted, its items separated by commas and each `"`
     /// doubled (`"Red,Green"`), as SpreadsheetML's `formula1`. Empty for "any value".

crates/xls/src/notes.rs

@@ -0,0 +1,57 @@
+//! Cell comments (notes): the Note record names the cell, its author and the
+//! drawing object that holds the text ([MS-XLS] 2.4.179 Note); the Obj record before it says
+//! the object is a comment ([MS-XLS] 2.4.181 Obj, 2.5.143 FtCmo, `ot` 0x19); the TxO record
+//! that follows holds the text in its Continue records ([MS-XLS] 2.4.329 TxO).
+
+/// Characters in one comment at most.
+pub(crate) const MAX_NOTE_CHARS: usize = 32_767;
+
+/// FtCmo's object type for a comment.
+pub(crate) const OT_NOTE: u16 = 0x19;
+
+/// The text of a TxO record: `cch` characters in the Continue records after its fixed part,
+/// each starting with its own compression flag (0: one byte a character, 1: UTF-16). The
+/// formatting runs after the text are ignored.
+pub(crate) fn txo_text(parts: &[&[u8]], cch: usize) -> String {
+    let cch = cch.min(MAX_NOTE_CHARS);
+    let mut units: Vec<u16> = Vec::with_capacity(cch);
+    for part in parts.iter().skip(1) {
+        if units.len() >= cch {
+            break;
+        }
+        let Some((&flag, rest)) = part.split_first() else { continue };
+        if flag & 1 != 0 {
+            for pair in rest.as_chunks::<2>().0 {
+                if units.len() >= cch {
+                    break;
+                }
+                units.push(u16::from_le_bytes(*pair));
+            }
+        } else {
+            for b in rest {
+                if units.len() >= cch {
+                    break;
+                }
+                units.push(u16::from(*b));
+            }
+        }
+    }
+    String::from_utf16_lossy(&units)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn txo_text_reads_compressed_and_wide_parts_and_stops_at_its_length() {
+        let fixed: &[u8] = &[0; 18];
+        assert_eq!(txo_text(&[fixed, &[0, b'H', b'i']], 2), "Hi");
+        let wide: Vec<u8> = [1u8].into_iter().chain("Đỗ".encode_utf16().flat_map(u16::to_le_bytes)).collect();
+        assert_eq!(txo_text(&[fixed, &wide], 2), "Đỗ");
+        // A text over two Continue records, then the formatting runs (not text).
+        assert_eq!(txo_text(&[fixed, &[0, b'a', b'b'], &[0, b'c'], &[0, 0, 0, 0]], 3), "abc");
+        assert_eq!(txo_text(&[fixed], 5), "", "no text record");
+        assert_eq!(txo_text(&[fixed, &[1, 0x41]], 1), "", "half a UTF-16 unit is dropped");
+    }
+}

crates/xls/src/read.rs

@@ -8,15 +8,24 @@ use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
 use crate::formula::{Links, Mode, SupBook, Unsupported, Xti, cell_error, decode};
-use crate::model::{CalcMode, Cell, CellRef, Column, DefinedName, Formula, Range, Row, Sheet, Value, View, Visibility, Workbook};
+use crate::model::{
+    CalcMode, Cell, CellRef, Column, ConditionRule, ConditionalFormat, DefinedName, Formula, Note, Picture, Range, Row, Sheet, Value, View,
+    Visibility, Workbook,
+};
 use crate::records::{Cursor, Record, Records};
-use crate::{Error, Limits, Read, Warning, styles};
+use crate::{Error, Limits, Read, Warning, cond, drawing, notes, styles};
 
 const MAX_STRINGS: usize = 4_000_000;
 const MAX_SHEETS: usize = 4096;
 const MAX_NAMES: usize = 65_536;
 const MAX_XFS: usize = 65_536;
 const MAX_MERGES: usize = 100_000;
+/// Bytes of drawing records kept for the workbook's drawing group, and for one sheet.
+const MAX_DRAWING: usize = 256 << 20;
+/// Pictures placed on one sheet at most.
+const MAX_PICTURES: usize = 65_536;
+/// Comments kept on one sheet at most.
+const MAX_NOTES: usize = 65_536;
 /// The Workbook stream is read whole; larger ones are refused.
 const MAX_STREAM: u64 = 1 << 30;
 /// Distinct warnings kept.
@@ -160,6 +169,8 @@ struct Globals {
     date1904: bool,
     formats: HashMap<u16, String>,
     styles: styles::StyleRecords,
+    /// The workbook's pictures (the drawing group's BStore), `pib` 1 first.
+    blips: Vec<drawing::Blip>,
     sst: Vec<Arc<str>>,
     links: Links,
     names: Vec<Lbl>,
@@ -210,7 +221,9 @@ pub fn read_with(bytes: &[u8], limits: Limits) -> Result<Read, Error> {
         parsed.push((bs.visibility, st));
     }
     // Every sheet's mandatory text is charged by now; shared formulas take what is left.
+    let mut filtered = Vec::with_capacity(parsed.len());
     for (visibility, st) in parsed {
+        filtered.push(st.filtered);
         let mut sheet = finish_sheet(st, &mut budget, &mut report);
         sheet.visibility = visibility;
         // Every worksheet repeats the workbook's calculation settings; Excel applies the first.
@@ -223,7 +236,7 @@ pub fn read_with(bytes: &[u8], limits: Limits) -> Result<Read, Error> {
         return Err(invalid("the workbook has no worksheets"));
     }
     wb.active_sheet = g.active_tab.and_then(|t| model_index.get(usize::from(t)).copied().flatten());
-    defined_names(&g, &model_index, &mut wb, &mut report);
+    defined_names(&g, &model_index, &filtered, &mut wb, &mut report);
     Ok(Read { workbook: wb, warnings: report.warnings })
 }
 
@@ -248,6 +261,7 @@ fn globals(stream: &[u8], budget: &mut Budget, report: &mut Report) -> Result<Gl
     }
     let mut g = Globals::default();
     let mut offsets = HashSet::new();
+    let mut dgg: Vec<u8> = Vec::new();
     while let Some(rec) = recs.next_record()? {
         budget.record()?;
         let mut c = Cursor::new(&rec.parts);
@@ -299,6 +313,8 @@ fn globals(stream: &[u8], budget: &mut Budget, report: &mut Report) -> Result<Gl
             0x0031 => g.styles.font(&mut c),
             0x0092 => g.styles.palette(&mut c),
             0x0293 => g.styles.style(&mut c),
+            // MsoDrawingGroup: one buffer of drawing records, cut at MAX_DRAWING.
+            0x00EB if dgg.len() < MAX_DRAWING => dgg.extend(c.rest()),
             0x00FC => g.sst = shared_strings(&mut c, budget)?,
             0x01AE => {
                 let _ctab = c.u16()?;
@@ -340,6 +356,7 @@ fn globals(stream: &[u8], budget: &mut Budget, report: &mut Report) -> Result<Gl
     // The same test `defined_names` applies, before any cell formula is decoded.
     let skipped = g.names.iter().map(|l| !l.rgce.is_empty() && decode(&l.rgce, &l.rgcb, CellRef::new(0, 0), Mode::Name, &g.links).is_err()).collect();
     g.links.skipped = skipped;
+    g.blips = drawing::blip_store(&dgg);
     Ok(g)
 }
 
@@ -441,6 +458,20 @@ struct SheetState<'g> {
     /// Dv records still to read: the last DVal's count, less those read since. Excel reads
     /// exactly that many after a DVal, and none without one.
     dv_left: u32,
+    /// The comment object the last Obj record described, waiting for its TxO.
+    note_object: Option<u16>,
+    /// Comment texts by object id.
+    note_texts: HashMap<u16, String>,
+    /// Note records: the cell, its comment object, the author and whether it shows.
+    notes: Vec<(CellRef, u16, String, bool)>,
+    /// The CondFmt whose CF rules follow: its bounding range's top left cell and its ranges.
+    cf: Option<(CellRef, Vec<Range>)>,
+    /// The workbook's colour table, for conditional formats.
+    palette: [u32; 64],
+    /// The sheet's MsoDrawing records, joined (shapes may run across them).
+    drawing: Vec<u8>,
+    /// The sheet has an AutoFilterInfo record: its filter is on.
+    filtered: bool,
 }
 
 impl SheetState<'_> {
@@ -594,6 +625,13 @@ fn worksheet<'g>(
         dropped: 0,
         unsupported: vec![],
         dv_left: 0,
+        note_object: None,
+        note_texts: HashMap::new(),
+        notes: vec![],
+        cf: None,
+        palette: g.styles.color_table(),
+        drawing: vec![],
+        filtered: false,
     };
     let mut depth = 1usize;
     while let Some(rec) = recs.next_record()? {
@@ -628,6 +666,22 @@ fn finish_sheet(mut st: SheetState<'_>, budget: &mut Budget, report: &mut Report
     let pending = std::mem::take(&mut st.pending);
     resolve_pending(&mut st, budget, &arrays, &shared, &pending);
     blank_array_followers(&mut st, &pending);
+    let mut skipped_pictures = false;
+    for shape in drawing::picture_shapes(&std::mem::take(&mut st.drawing)) {
+        let blip = usize::try_from(shape.pib).ok().and_then(|i| i.checked_sub(1)).and_then(|i| st.g.blips.get(i)).cloned().flatten();
+        match blip {
+            Some((data, mime)) if st.sheet.pictures.len() < MAX_PICTURES => st.sheet.pictures.push(Picture { anchor: shape.anchor, data, mime }),
+            _ => skipped_pictures = true,
+        }
+    }
+    if skipped_pictures {
+        report.warn(Warning::PicturesSkipped);
+    }
+    // A Note names its text's object, whose TxO came earlier in the substream.
+    for (cell, object, author, visible) in std::mem::take(&mut st.notes) {
+        let text = st.note_texts.get(&object).cloned().unwrap_or_default();
+        st.sheet.notes.push(Note { cell, author, text, visible });
+    }
     if st.dropped > 0 {
         report.warn(Warning::CellsOutsideGrid(st.dropped));
     }
@@ -799,9 +853,29 @@ fn sheet_record(rec: &Record<'_>, st: &mut SheetState<'_>, budget: &mut Budget,
                 right_to_left: on(0x0040),
             });
         }
-        0x001C => report.warn(Warning::Comments),
-        0x00EC => report.warn(Warning::Drawings),
-        0x01B0 | 0x0879 => report.warn(Warning::ConditionalFormatting),
+        0x005D | 0x01B6 | 0x001C | 0x00EC => match object_record(rec, &mut c, st, budget, report) {
+            Err(Error::Invalid(_)) => {
+                if rec.id == 0x005D {
+                    st.note_object = None;
+                }
+                report.warn(Warning::DamagedObjects);
+            }
+            r => r?,
+        },
+        0x009D => st.filtered = true,
+        0x01B0 => match cond::condfmt(&mut c) {
+            Ok(cf) => st.cf = Some(cf),
+            Err(_) => {
+                st.cf = None;
+                report.warn(Warning::ConditionalFormattingSkipped);
+            }
+        },
+        0x01B1 => {
+            if conditional_format(&mut c, st).is_none() {
+                report.warn(Warning::ConditionalFormattingSkipped);
+            }
+        }
+        0x0879 => report.warn(Warning::ConditionalFormatting12),
         // A short DVal starts no rules.
         0x01B2 => st.dv_left = crate::dv::dval(&mut c).unwrap_or(0),
         0x01BE => {
@@ -829,6 +903,68 @@ fn sheet_record(rec: &Record<'_>, st: &mut SheetState<'_>, budget: &mut Budget,
     Ok(())
 }
 
+/// The drawing-layer records of a sheet: comment objects (Obj, TxO, Note) and the drawing
+/// stream (MsoDrawing) that places pictures.
+fn object_record(rec: &Record<'_>, c: &mut Cursor<'_, '_>, st: &mut SheetState<'_>, budget: &mut Budget, report: &mut Report) -> Result<(), Error> {
+    match rec.id {
+        // Obj: FtCmo first (ft 0x15, cb, object type, id).
+        0x005D => {
+            let ft = c.u16()?;
+            c.u16()?;
+            let ot = c.u16()?;
+            let id = c.u16()?;
+            st.note_object = (ft == 0x15 && ot == notes::OT_NOTE).then_some(id);
+            if ft == 0x15 && ot == 5 {
+                report.warn(Warning::Charts);
+            }
+        }
+        // TxO: the text of the comment object just described.
+        0x01B6 => {
+            if let Some(id) = st.note_object.take() {
+                c.skip(10)?;
+                let cch = usize::from(c.u16()?);
+                let text = notes::txo_text(&rec.parts, cch);
+                budget.text(text.len())?;
+                st.note_texts.insert(id, text);
+            }
+        }
+        // Note: the cell, flags (0x0002: shown), the object id and the author.
+        0x001C => {
+            let (row, col, flags, object) = (c.u16()?, c.u16()?, c.u16()?, c.u16()?);
+            let author = c.string().unwrap_or_default();
+            budget.text(author.len())?;
+            if let Some(cell) = st.pos(row, col)
+                && st.notes.len() < MAX_NOTES
+            {
+                st.notes.push((cell, object, author, flags & 0x0002 != 0));
+            }
+        }
+        0x00EC if st.drawing.len() < MAX_DRAWING => st.drawing.extend(c.rest()),
+        _ => {}
+    }
+    Ok(())
+}
+
+/// One CF record of the current CondFmt; `None` when it is not kept.
+fn conditional_format(c: &mut Cursor<'_, '_>, st: &mut SheetState<'_>) -> Option<()> {
+    let g = st.g;
+    let rule = cond::cf(c, &st.palette, |ifmt| g.formats.get(&ifmt).cloned()).ok()?;
+    let (base, ranges) = st.cf.as_ref()?;
+    if ranges.is_empty() || st.sheet.conditional_formats.len() >= cond::MAX_RULES {
+        return None;
+    }
+    // The formulas are offsets from the top left cell of the CondFmt's bounding range.
+    let formula =
+        |rgce: &[u8]| (!rgce.is_empty()).then(|| decode(rgce, &[], *base, Mode::Cell, &g.links).ok().map(|e| crate::expr::print(&e))).flatten();
+    let rule_kind = match (rule.kind, rule.operator) {
+        (1, Some(operator)) => ConditionRule::CellIs { operator, formula1: formula(&rule.rgce1)?, formula2: formula(&rule.rgce2) },
+        (2, _) => ConditionRule::Expression { formula: formula(&rule.rgce1)? },
+        _ => return None,
+    };
+    st.sheet.conditional_formats.push(ConditionalFormat { ranges: ranges.clone(), rule: rule_kind, format: rule.format });
+    Some(())
+}
+
 /// MergeCells ([MS-XLS] 2.4.168).
 fn merges(c: &mut Cursor<'_, '_>, st: &mut SheetState<'_>) -> Result<(), Error> {
     let n = usize::from(c.u16()?);
@@ -892,7 +1028,7 @@ fn formula_cell(c: &mut Cursor<'_, '_>, st: &mut SheetState<'_>, budget: &mut Bu
     Ok(())
 }
 
-fn defined_names(g: &Globals, model_index: &[Option<usize>], wb: &mut Workbook, report: &mut Report) {
+fn defined_names(g: &Globals, model_index: &[Option<usize>], filtered: &[bool], wb: &mut Workbook, report: &mut Report) {
     let scope_of = |itab: u16| -> Option<Option<usize>> {
         if itab == 0 {
             return Some(None);
@@ -918,6 +1054,19 @@ fn defined_names(g: &Globals, model_index: &[Option<usize>], wb: &mut Workbook,
                     sheet.print_titles = Some(text);
                 }
             }
+            // _FilterDatabase: the AutoFilter's range, on a sheet whose filter is on.
+            Some(13) => {
+                let Some(si) = scope else { continue };
+                if !filtered.get(si).copied().unwrap_or(false) {
+                    continue;
+                }
+                let area = text.rsplit('!').next().unwrap_or("").replace('$', "");
+                if let (Some(range), Some(sheet)) = (Range::parse(&area), wb.sheets.get_mut(si))
+                    && sheet.autofilter.is_none()
+                {
+                    sheet.autofilter = Some(range);
+                }
+            }
             Some(_) => {}
             None if lbl.macro_name => report.warn(Warning::MacroNames),
             None => wb.names.push(DefinedName { name: lbl.name.clone(), scope, formula: text, hidden: lbl.hidden }),

crates/xls/src/styles.rs

@@ -161,7 +161,7 @@ impl StyleRecords {
     }
 
     /// The 64-entry colour table: the built-in eight, then the palette (or the default one).
-    fn color_table(&self) -> [u32; 64] {
+    pub(crate) fn color_table(&self) -> [u32; 64] {
         let mut table = DEFAULT_PALETTE;
         if let Some(p) = &self.palette {
             for (slot, c) in table.iter_mut().skip(8).zip(p.iter()) {
@@ -215,7 +215,7 @@ fn read_font(c: &mut Cursor<'_, '_>) -> Option<(Font, u16)> {
 
 /// A colour index in an XF (IcvXF) or Font (IcvFont): 0-63 from the colour table, the system
 /// and automatic colours as automatic.
-fn color(icv: u16, table: &[u32; 64]) -> Color {
+pub(crate) fn color(icv: u16, table: &[u32; 64]) -> Color {
     match icv {
         ICV_DEFAULT_FG | ICV_DEFAULT_BG | ICV_FONT_AUTO => Color::Auto,
         _ => table.get(usize::from(icv)).map_or(Color::Auto, |c| Color::Rgb(*c)),
@@ -223,7 +223,7 @@ fn color(icv: u16, table: &[u32; 64]) -> Color {
 }
 
 /// A border line style from its BIFF8 code (dgLeft and the like); unknown codes are no border.
-fn border_style(dg: u32) -> BorderStyle {
+pub(crate) fn border_style(dg: u32) -> BorderStyle {
     match dg {
         1 => BorderStyle::Thin,
         2 => BorderStyle::Medium,
@@ -243,7 +243,7 @@ fn border_style(dg: u32) -> BorderStyle {
 }
 
 /// A fill pattern from its BIFF8 code (fls); unknown codes are no pattern.
-fn pattern(fls: u16) -> Pattern {
+pub(crate) fn pattern(fls: u16) -> Pattern {
     match fls {
         0x01 => Pattern::Solid,
         0x02 => Pattern::Gray50,

crates/xls/tests/objects.rs

@@ -0,0 +1,240 @@
+//! Comments, conditional formats, pictures and the AutoFilter range, each record assembled here
+//! from [MS-XLS] and [MS-ODRAW]; no Excel-produced file is used. Ported from
+//! storytold/gridcraft#3457, with tests for the fixes its review asked for.
+
+mod common;
+
+use common::*;
+use craft_xls::{ClientAnchor, Color, ConditionRule, DxfFill, Operator, Pattern, Range, Workbook};
+
+fn book(globals_extra: &[u8], sheet_records: &[u8]) -> (Workbook, Report) {
+    let xf = rec(0x00E0, &cat(&[&u16s(&[0, 0, 0]), &[0; 14]]));
+    let globals = |pos: u32| cat(&[&bof(0x0005), &xf, globals_extra, &rec(0x0085, &cat(&[&pos.to_le_bytes(), &[0, 0, 6, 0], b"Sheet1"])), &eof()]);
+    let sheet = cat(&[&bof(0x0010), sheet_records, &eof()]);
+    let stream = cat(&[&globals(globals(0).len() as u32), &sheet]);
+    read_xls(&compound("Workbook", &stream)).unwrap()
+}
+
+fn wide(s: &str) -> Vec<u8> {
+    s.encode_utf16().flat_map(u16::to_le_bytes).collect()
+}
+
+/// Obj: FtCmo (ft 0x15, cb 0x12, ot 0x19 comment, id, flags, reserved), then FtEnd.
+fn obj(id: u16) -> Vec<u8> {
+    rec(0x005D, &cat(&[&u16s(&[0x15, 0x12, 0x19, id, 0x4011]), &[0; 12], &u16s(&[0, 0])]))
+}
+
+/// TxO with its text and formatting runs in Continue records.
+fn txo(text: &str) -> Vec<u8> {
+    let n = text.encode_utf16().count() as u16;
+    cat(&[
+        &rec(0x01B6, &cat(&[&u16s(&[0x0212, 0]), &[0; 6], &u16s(&[n, 16, 0, 0])])),
+        &rec(0x003C, &cat(&[&[1], &wide(text)])),
+        &rec(0x003C, &[0; 16]),
+    ])
+}
+
+/// Note: row, column, flags (0x0002: shown), object id, author (UTF-16).
+fn note(r: u16, c: u16, flags: u16, id: u16, author: &str) -> Vec<u8> {
+    rec(0x001C, &cat(&[&u16s(&[r, c, flags, id]), &(author.encode_utf16().count() as u16).to_le_bytes(), &[1], &wide(author), &[0]]))
+}
+
+#[test]
+fn reads_cell_comments() {
+    let (wb, report) = book(
+        &[],
+        &cat(&[
+            &rec(0x0203, &cat(&[&cell(1, 2, 0), &1.0f64.to_le_bytes()])),
+            &obj(1),
+            &txo("Đỗ Anh Hải Sơn:\nKiểm tra lại số tài khoản"),
+            &obj(2),
+            &txo("Ghi chú"),
+            &note(1, 2, 0x0002, 1, "Đỗ Anh Hải Sơn"),
+            &note(4, 0, 0, 2, "Kế toán"),
+            // A Note for an object with no text, and one outside the grid, break nothing.
+            &note(5, 0, 0, 9, "x"),
+            &note(0xFFFF, 300, 0, 1, "y"),
+        ]),
+    );
+    let notes = &wb.sheets[0].notes;
+    assert_eq!(notes.len(), 3, "{:?}", report.warnings);
+    let got: Vec<_> = notes.iter().map(|n| (n.cell.a1(), n.author.as_str(), n.text.as_str(), n.visible)).collect();
+    assert_eq!(
+        got,
+        [
+            ("C2".to_string(), "Đỗ Anh Hải Sơn", "Đỗ Anh Hải Sơn:\nKiểm tra lại số tài khoản", true),
+            ("A5".to_string(), "Kế toán", "Ghi chú", false),
+            ("A6".to_string(), "x", "", false),
+        ]
+    );
+}
+
+#[test]
+fn damaged_object_records_are_skipped_not_fatal() {
+    // A short Obj, a short Note and a TxO cut before its count: each is skipped with a warning,
+    // and the comment after them still reads.
+    let (wb, report) = book(
+        &[],
+        &cat(&[&rec(0x005D, &[0x15, 0]), &rec(0x001C, &[1, 0]), &obj(3), &rec(0x01B6, &[0; 4]), &obj(1), &txo("kept"), &note(0, 0, 0, 1, "a")]),
+    );
+    assert_eq!(wb.sheets[0].notes.iter().map(|n| n.text.as_str()).collect::<Vec<_>>(), ["kept"]);
+    assert!(report.warnings.iter().any(|w| w.contains("comment or object records were damaged")), "{:?}", report.warnings);
+}
+
+/// A CF record: kind, operator, formula sizes, DXFN, the formulas.
+fn cf(kind: u8, op: u8, rgce: &[u8], dxfn: &[u8]) -> Vec<u8> {
+    rec(0x01B1, &cat(&[&[kind, op], &u16s(&[rgce.len() as u16, 0]), dxfn, rgce]))
+}
+
+/// CondFmt: rule count, flags, bounding range (rows, columns), then the range list.
+fn condfmt(bound: [u16; 4], ranges: &[[u16; 4]]) -> Vec<u8> {
+    let list: Vec<u8> = ranges.iter().flat_map(|r| u16s(r)).collect();
+    rec(0x01B0, &cat(&[&u16s(&[2, 0]), &u16s(&bound), &(ranges.len() as u16).to_le_bytes(), &list]))
+}
+
+/// `PtgRefN` to the anchor cell (both offsets 0), then `> 5`.
+fn relative() -> Vec<u8> {
+    cat(&[&[0x2C], &u16s(&[0, 0xC000]), &[0x1E], &5u16.to_le_bytes(), &[0x0D]])
+}
+
+#[test]
+fn reads_conditional_formats() {
+    // DXFN: font block (26) and pattern block (29), the foreground colour unset (17).
+    let mut font = vec![0u8; 118];
+    font[64..68].copy_from_slice(&u32::MAX.to_le_bytes());
+    font[72..74].copy_from_slice(&700u16.to_le_bytes());
+    font[80..84].copy_from_slice(&10u32.to_le_bytes());
+    font[88..92].copy_from_slice(&0x82u32.to_le_bytes());
+    font[92..96].copy_from_slice(&1u32.to_le_bytes());
+    font[96..100].copy_from_slice(&1u32.to_le_bytes());
+    let dxfn = cat(&[&((1u32 << 26) | (1 << 29) | (1 << 17)).to_le_bytes(), &0u16.to_le_bytes(), &font, &u16s(&[1 << 10, 64 | (13 << 7)])]);
+    let greater = cat(&[&[0x1E], &100u16.to_le_bytes()]);
+    let (wb, report) = book(
+        &[],
+        &cat(&[
+            &rec(0x0203, &cat(&[&cell(1, 1, 0), &150.0f64.to_le_bytes()])),
+            &condfmt([1, 3, 1, 2], &[[1, 3, 1, 2]]),
+            &cf(1, 5, &greater, &dxfn),
+            &cf(2, 0, &relative(), &dxfn),
+        ]),
+    );
+    let cfs = &wb.sheets[0].conditional_formats;
+    assert_eq!(cfs.len(), 2, "{:?}", report.warnings);
+    assert_eq!(cfs[0].ranges, vec![Range::parse("B2:C4").unwrap()]);
+    assert_eq!(cfs[0].rule, ConditionRule::CellIs { operator: Operator::Greater, formula1: "100".into(), formula2: None });
+    let f = &cfs[0].format;
+    assert_eq!((f.bold, f.italic, f.strike, f.underline), (Some(true), None, None, None));
+    assert_eq!(f.font_color, Some(Color::Rgb(0xFF0000)));
+    assert_eq!(f.fill, Some(DxfFill { pattern: Some(Pattern::Solid), fg: None, bg: Some(Color::Rgb(0xFFFF00)) }));
+    assert_eq!(cfs[1].rule, ConditionRule::Expression { formula: "B2>5".into() });
+    assert!(report.warnings.is_empty(), "{:?}", report.warnings);
+}
+
+#[test]
+fn formulas_are_relative_to_the_bounding_range() {
+    // Ranges D5:D6 then B2:B3: the bounding range starts at B2, and Excel writes the formula
+    // from there, whatever order the ranges are in.
+    let (wb, report) =
+        book(&[], &cat(&[&condfmt([1, 5, 1, 3], &[[4, 5, 3, 3], [1, 2, 1, 1]]), &cf(2, 0, &relative(), &cat(&[&0u32.to_le_bytes(), &[0, 0]]))]));
+    let cfs = &wb.sheets[0].conditional_formats;
+    assert_eq!(cfs.len(), 1, "{:?}", report.warnings);
+    assert_eq!(cfs[0].rule, ConditionRule::Expression { formula: "B2>5".into() });
+}
+
+#[test]
+fn unreadable_conditional_formats_are_skipped() {
+    let plain = cat(&[&0u32.to_le_bytes(), &[0, 0]]);
+    let (wb, report) = book(
+        &[],
+        &cat(&[
+            // A CF with no CondFmt before it, a damaged CondFmt, an unknown rule type, a short CF.
+            &cf(2, 0, &relative(), &plain),
+            &rec(0x01B0, &[1]),
+            &cf(2, 0, &relative(), &plain),
+            &condfmt([0, 0, 0, 0], &[[0, 0, 0, 0]]),
+            &cf(3, 0, &relative(), &plain),
+            &rec(0x01B1, &[1]),
+            &cf(2, 0, &relative(), &plain),
+            &rec(0x0879, &[0; 20]),
+        ]),
+    );
+    assert_eq!(wb.sheets[0].conditional_formats.len(), 1);
+    assert!(report.warnings.iter().any(|w| w.contains("conditional formatting rules could not be read")), "{:?}", report.warnings);
+    assert!(report.warnings.iter().any(|w| w.contains("data bars")), "{:?}", report.warnings);
+}
+
+fn od(vi: u16, ty: u16, body: &[u8]) -> Vec<u8> {
+    cat(&[&vi.to_le_bytes(), &ty.to_le_bytes(), &(body.len() as u32).to_le_bytes(), body])
+}
+
+fn fbse(blip: &[u8]) -> Vec<u8> {
+    od(0x0062, 0xF007, &cat(&[&[0u8; 36], blip]))
+}
+
+fn shape(pib: u32, anchor: [u16; 8]) -> Vec<u8> {
+    let opt = od(0x0013, 0xF00B, &cat(&[&0x4104u16.to_le_bytes(), &pib.to_le_bytes()]));
+    let a: Vec<u8> = std::iter::once(0u16).chain(anchor).flat_map(u16::to_le_bytes).collect();
+    od(0x000F, 0xF004, &cat(&[&opt, &od(0, 0xF010, &a)]))
+}
+
+/// The drawing group (split over a Continue record, as real files do) and one sheet's drawing.
+fn pictures(blips: &[Vec<u8>], shapes: &[u8]) -> (Workbook, Report) {
+    let dgg = od(0x000F, 0xF000, &od(0x002F, 0xF001, &blips.iter().map(|b| fbse(b)).collect::<Vec<_>>().concat()));
+    let (first, second) = dgg.split_at(30);
+    let dg = od(0x000F, 0xF002, &od(0x000F, 0xF003, shapes));
+    // MsoDrawing records are at most 8224 bytes; longer drawings continue in more of them.
+    let drawing: Vec<u8> = dg.chunks(8000).flat_map(|c| rec(0x00EC, c)).collect();
+    book(&cat(&[&rec(0x00EB, first), &rec(0x003C, second)]), &drawing)
+}
+
+#[test]
+fn reads_pictures() {
+    let png = [0x89, b'P', b'N', b'G', 1, 2, 3];
+    let blip = od(0x6E00, 0xF01E, &cat(&[&[0u8; 17], &png]));
+    let emf_bytes = b" EMF the picture's own records".to_vec();
+    let packed = miniz_oxide::deflate::compress_to_vec_zlib(&emf_bytes, 6);
+    let meta_header = cat(&[&(emf_bytes.len() as u32).to_le_bytes(), &[0u8; 24], &(packed.len() as u32).to_le_bytes(), &[0x00, 0xFE]]);
+    let emf = od(0x3D40, 0xF01A, &cat(&[&[0u8; 16], &meta_header, &packed]));
+    let damaged = od(0x3D40, 0xF01A, &[0u8; 60]);
+    let shapes = cat(&[
+        &shape(1, [1, 0, 1, 0, 3, 0, 3, 0]),
+        &shape(2, [5, 0, 5, 0, 6, 0, 6, 0]),
+        &shape(3, [7, 0, 7, 0, 8, 0, 8, 0]),
+        &shape(1, [9, 512, 9, 128, 10, 0, 10, 0]),
+    ]);
+    let (wb, report) = pictures(&[blip, emf, damaged], &shapes);
+    let p = &wb.sheets[0].pictures;
+    assert_eq!(p.len(), 3, "{:?}", report.warnings);
+    assert_eq!((p[0].data.as_slice(), p[0].mime), (&png[..], "image/png"));
+    assert_eq!(p[0].anchor, ClientAnchor { col1: 1, dx1: 0, row1: 1, dy1: 0, col2: 3, dx2: 0, row2: 3, dy2: 0 });
+    assert_eq!((p[1].data.as_slice(), p[1].mime), (&emf_bytes[..], "image/x-emf"));
+    // Two shapes that show one picture share its bytes.
+    assert!(std::sync::Arc::ptr_eq(&p[0].data, &p[2].data));
+    assert_eq!((p[2].anchor.dx1, p[2].anchor.dy1), (512, 128));
+    assert!(report.warnings.iter().any(|w| w.contains("pictures were damaged")), "{:?}", report.warnings);
+}
+
+#[test]
+fn pictures_past_the_cap_are_skipped() {
+    let png = od(0x6E00, 0xF01E, &cat(&[&[0u8; 17], b"png"]));
+    let shapes: Vec<u8> = (0..65_537).flat_map(|_| shape(1, [0; 8])).collect();
+    let (wb, report) = pictures(&[png], &shapes);
+    assert_eq!(wb.sheets[0].pictures.len(), 65_536);
+    assert!(report.warnings.iter().any(|w| w.contains("past the limits")), "{:?}", report.warnings);
+}
+
+#[test]
+fn reads_the_autofilter_range() {
+    let read = |filter_on: bool| {
+        // _FilterDatabase on sheet 1 (hidden, built-in 13) = Sheet1!$A$8:$DH$19 (PtgArea3d).
+        let area = cat(&[&[0x3B], &0u16.to_le_bytes(), &u16s(&[7, 18, 0, 111])]);
+        let lbl = rec(0x0018, &cat(&[&u16s(&[0x0021]), &[0, 1], &u16s(&[area.len() as u16, 0, 1]), &[0; 4], &[0, 13], &area]));
+        let names = cat(&[&rec(0x01AE, &u16s(&[1, 0x0401])), &rec(0x0017, &cat(&[&u16s(&[1]), &u16s(&[0, 0, 0])])), &lbl]);
+        let info = if filter_on { rec(0x009D, &u16s(&[112])) } else { vec![] };
+        book(&names, &cat(&[&rec(0x0203, &cat(&[&cell(7, 0, 0), &1.0f64.to_le_bytes()])), &info])).0
+    };
+    let wb = read(true);
+    assert_eq!(wb.sheets[0].autofilter, Some(Range::parse("A8:DH19").unwrap()));
+    assert!(!wb.names.iter().any(|n| n.name.contains("FilterDatabase")), "a built-in name, not a user one");
+    assert_eq!(read(false).sheets[0].autofilter, None, "the name alone turns no filter on");
+}

crates/xls/tests/validation.rs

@@ -5,7 +5,7 @@
 mod common;
 
 use common::*;
-use craft_xls::{CellRef, DataValidation, ErrorStyle, Range, ValidationOperator, ValidationType, Workbook};
+use craft_xls::{CellRef, DataValidation, ErrorStyle, Operator, Range, ValidationType, Workbook};
 
 /// A workbook of one worksheet, "Sheet1", holding `records`, with defined names (Lbl records,
 /// each a name and its formula) in the globals.
@@ -82,7 +82,7 @@ fn range(a1: &str) -> Range {
 }
 
 /// A rule with Excel's default flags and no messages.
-fn expected(ranges: &[&str], kind: ValidationType, operator: ValidationOperator, f1: &str, f2: Option<&str>) -> DataValidation {
+fn expected(ranges: &[&str], kind: ValidationType, operator: Operator, f1: &str, f2: Option<&str>) -> DataValidation {
     DataValidation {
         ranges: ranges.iter().map(|r| range(r)).collect(),
         kind,
@@ -111,7 +111,7 @@ const IGNORED: &str =
 
 #[test]
 fn reads_every_kind_of_rule() {
-    use ValidationOperator::*;
+    use Operator::*;
     use ValidationType::*;
     let rules = [
         // Whole number between 1 and 10, with all four messages and the stop alert.
@@ -199,15 +199,15 @@ fn reads_flags_messages_lists_and_whole_columns() {
     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"], ValidationType::List, ValidationOperator::Between, "\"a,b\"", None);
+    let mut flags_off = expected(&["O2:O6"], ValidationType::List, Operator::Between, "\"a,b\"", None);
     (flags_off.allow_blank, flags_off.in_cell_dropdown, flags_off.show_prompt, flags_off.show_error) = (false, false, false, false);
     assert_eq!(got[0], flags_off);
-    let mut unicode = expected(&["U2:U6"], ValidationType::Whole, ValidationOperator::Equal, "3", None);
+    let mut unicode = expected(&["U2:U6"], ValidationType::Whole, Operator::Equal, "3", None);
     (unicode.prompt_title, unicode.error_title, unicode.prompt, unicode.error) =
         ("Café".into(), "Ошибка".into(), "日本語のテスト é ü".into(), "Nur 3 ✓".into());
     assert_eq!(got[1], unicode);
     // The item `say "hi"` keeps its quotes, doubled; the operator is ignored.
-    assert_eq!(got[2], expected(&["X2:X6"], ValidationType::List, ValidationOperator::Between, "\"say \"\"hi\"\",bye\"", None));
+    assert_eq!(got[2], expected(&["X2:X6"], ValidationType::List, Operator::Between, "\"say \"\"hi\"\",bye\"", None));
     assert_eq!(got[3].ranges, vec![range("V:V")]);
 }