Never crash: cms is panic-free (Rule 9)

- Real crash: a gray ICC profile whose A2B0 tag is stored as "mBA "
  parsed fine, but re-encoding its RGB view (gray_as_rgb, used for gray
  documents' composites) hit panic!("unsupported lutAtoB stage layout").
  The ICC writer now leaves out a LUT its tag type can't store, and
  gray_as_rgb drops the gray LUTs (one input channel) so the RGB view is
  the matrix/TRC model it builds. Regression test
  gray_profile_with_reversed_lut_type_reencodes.
- math::rgb_to_xyz_matrix returns None for dependent primaries.
- Builtin::profile indexes its cache by discriminant (tested); the shipped
  CMYK profile falls back to synth::coated_cmyk() instead of expect.
- Two constant-only expects (Bradford inverse, built-in primaries) use the
  documented narrow allow.
- cms denies unwrap/expect/panic/unimplemented/todo/unreachable.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/cms/src/builtin.rs

@@ -106,8 +106,9 @@ impl Builtin {
     /// The profile (built once per process).
     pub fn profile(self) -> &'static Profile {
         static CELLS: [OnceLock<Profile>; 10] = [const { OnceLock::new() }; 10];
-        let i = Builtin::ALL.iter().position(|b| *b == self).expect("listed");
-        CELLS[i].get_or_init(|| build(self))
+        // `ALL` lists the variants in declaration order (checked by `all_in_declaration_order`),
+        // so the discriminant is the cell index.
+        CELLS[self as usize].get_or_init(|| build(self))
     }
 }
 
@@ -134,7 +135,10 @@ fn d65() -> [f64; 3] {
 /// Colorant matrix adapted to D50 and quantized to s15Fixed16, with each row nudged so that
 /// RGB (1, 1, 1) maps exactly to the stored D50 white (keeps neutrals neutral).
 fn colorants(r: [f64; 2], g: [f64; 2], b: [f64; 2], white: [f64; 3]) -> (Mat3, Option<Mat3>) {
-    let m = math::rgb_to_xyz_matrix(r, g, b, white);
+    // Only called with the constant primaries of the built-in spaces, which are independent
+    // (every built-in profile is built in the tests).
+    #[allow(clippy::expect_used)]
+    let m = math::rgb_to_xyz_matrix(r, g, b, white).expect("built-in primaries are independent");
     let d50 = math::d50_quantized();
     let adapt = if (white[0] - math::D50[0]).abs() < 1e-3 && (white[2] - math::D50[2]).abs() < 1e-3 { None } else { Some(math::bradford(white, math::D50)) };
     let m50 = match &adapt {
@@ -258,6 +262,8 @@ fn build(b: Builtin) -> Profile {
         Builtin::GrayGamma22 => gray(b.description(), Curve::Gamma(2.2)),
         Builtin::SGray => gray(b.description(), srgb_trc()),
         Builtin::LabD50 => lab(),
-        Builtin::CoatedCmyk => Profile::parse(COATED_CMYK_ICC).expect("shipped CMYK profile parses"),
+        // The shipped bytes are `synth::coated_cmyk()` (checked by the `regen` test); rebuild
+        // them rather than crash if they ever fail to parse.
+        Builtin::CoatedCmyk => Profile::parse(COATED_CMYK_ICC).unwrap_or_else(|_| crate::synth::coated_cmyk()),
     }
 }

crates/cms/src/lib.rs

@@ -13,6 +13,7 @@
 //! CIE 15, and Adobe's published black point compensation paper. This crate depends on no
 //! other workspace crate.
 #![forbid(unsafe_code)]
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
 pub mod builtin;
 pub mod clut;

crates/cms/src/math.rs

@@ -59,6 +59,8 @@ const BRADFORD: Mat3 = [[0.8951, 0.2664, -0.1614], [-0.7502, 1.7135, 0.0367], [0
 
 /// Bradford chromatic adaptation matrix from white `src` to white `dst` (both XYZ).
 pub fn bradford(src: [f64; 3], dst: [f64; 3]) -> Mat3 {
+    // `BRADFORD` is a constant with determinant ~1.7 (see `bradford_maps_white`).
+    #[allow(clippy::expect_used)]
     let inv = invert(&BRADFORD).expect("Bradford matrix is invertible");
     let s = apply(&BRADFORD, src);
     let d = apply(&BRADFORD, dst);
@@ -66,12 +68,13 @@ pub fn bradford(src: [f64; 3], dst: [f64; 3]) -> Mat3 {
     mul(&inv, &mul(&scale, &BRADFORD))
 }
 
-/// RGB → XYZ matrix (relative to `white`, Y = 1) for primaries given as chromaticities.
-pub fn rgb_to_xyz_matrix(r: [f64; 2], g: [f64; 2], b: [f64; 2], white: [f64; 3]) -> Mat3 {
+/// RGB → XYZ matrix (relative to `white`, Y = 1) for primaries given as chromaticities, or
+/// `None` when the primaries are not independent.
+pub fn rgb_to_xyz_matrix(r: [f64; 2], g: [f64; 2], b: [f64; 2], white: [f64; 3]) -> Option<Mat3> {
     let (xr, xg, xb) = (xy_to_xyz(r), xy_to_xyz(g), xy_to_xyz(b));
     let m = [[xr[0], xg[0], xb[0]], [xr[1], xg[1], xb[1]], [xr[2], xg[2], xb[2]]];
-    let s = apply(&invert(&m).expect("primaries are independent"), white);
-    [[m[0][0] * s[0], m[0][1] * s[1], m[0][2] * s[2]], [m[1][0] * s[0], m[1][1] * s[1], m[1][2] * s[2]], [m[2][0] * s[0], m[2][1] * s[1], m[2][2] * s[2]]]
+    let s = apply(&invert(&m)?, white);
+    Some([[m[0][0] * s[0], m[0][1] * s[1], m[0][2] * s[2]], [m[1][0] * s[0], m[1][1] * s[1], m[1][2] * s[2]], [m[2][0] * s[0], m[2][1] * s[1], m[2][2] * s[2]]])
 }
 
 const EPS: f64 = 216.0 / 24389.0; // (6/29)^3

crates/cms/src/profile.rs

@@ -241,6 +241,9 @@ impl Profile {
         p.matrix = Some(m);
         p.trc = Some([c.clone(), c.clone(), c]);
         p.gray_trc = None;
+        // The gray LUTs take one input channel; this profile is the matrix/TRC model above.
+        p.a2b = Default::default();
+        p.b2a = Default::default();
         p.bytes = None;
         p.hash = Default::default();
         Some(p)

crates/cms/src/write.rs

@@ -111,10 +111,13 @@ fn lut_parts(stages: &[Stage]) -> Option<LutParts<'_>> {
     }
 }
 
-fn encode_mft(stages: &[Stage], wide: bool) -> Vec<u8> {
-    let (mat, a, clut, b) = lut_parts(stages).expect("lut8/lut16 stages are [matrix?] curves, clut, curves");
-    let g = clut.grid[0];
-    assert!(clut.grid.iter().all(|x| *x == g), "lut8/lut16 need a uniform grid");
+/// `None` unless the stages are `[matrix?] curves, clut, curves` with a uniform grid.
+fn encode_mft(stages: &[Stage], wide: bool) -> Option<Vec<u8>> {
+    let (mat, a, clut, b) = lut_parts(stages)?;
+    let g = *clut.grid.first()?;
+    if !clut.grid.iter().all(|x| *x == g) {
+        return None;
+    }
     let mut o = if wide { b"mft2\0\0\0\0".to_vec() } else { b"mft1\0\0\0\0".to_vec() };
     o.extend_from_slice(&[clut.inputs as u8, clut.outputs as u8, g as u8, 0]);
     let m = mat.unwrap_or_else(|| vec![1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]);
@@ -146,10 +149,11 @@ fn encode_mft(stages: &[Stage], wide: bool) -> Vec<u8> {
             put(&mut o, v);
         }
     }
-    o
+    Some(o)
 }
 
-fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<u8> {
+/// `None` when the stages aren't a layout `lutAtoB`/`lutBtoA` can store in this direction.
+fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Option<Vec<u8>> {
     let mut a_curves: Option<&[Curve]> = None;
     let mut clut: Option<&Clut> = None;
     let mut m_curves: Option<&[Curve]> = None;
@@ -173,7 +177,7 @@ fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<
                 b_curves = Some(b.as_slice());
             }
             [Stage::Curves(b)] => b_curves = Some(b.as_slice()),
-            _ => panic!("unsupported lutAtoB stage layout"),
+            _ => return None,
         }
     } else {
         // B [matrix, M] [CLUT, A]
@@ -193,7 +197,9 @@ fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<
             a_curves = Some(a.as_slice());
             k += 2;
         }
-        assert_eq!(k, v.len(), "unsupported lutBtoA stage layout");
+        if k != v.len() {
+            return None;
+        }
     }
     let mut o = if a2b { b"mAB \0\0\0\0".to_vec() } else { b"mBA \0\0\0\0".to_vec() };
     o.extend_from_slice(&[inputs as u8, outputs as u8, 0, 0]);
@@ -205,7 +211,7 @@ fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<
             pad4(o);
         }
     };
-    let b = b_curves.expect("B curves are required");
+    let b = b_curves?;
     offs[0] = o.len() as u32;
     curves(&mut o, b);
     if let Some((mm, off)) = matrix {
@@ -217,7 +223,7 @@ fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<
             o.extend_from_slice(&s15(*v));
         }
         offs[2] = o.len() as u32;
-        curves(&mut o, m_curves.expect("M curves accompany the matrix"));
+        curves(&mut o, m_curves?);
     }
     if let Some(c) = clut {
         offs[3] = o.len() as u32;
@@ -232,15 +238,17 @@ fn encode_ab(stages: &[Stage], a2b: bool, inputs: usize, outputs: usize) -> Vec<
         }
         pad4(&mut o);
         offs[4] = o.len() as u32;
-        curves(&mut o, a_curves.expect("A curves accompany the CLUT"));
+        curves(&mut o, a_curves?);
     }
     for (k, v) in offs.iter().enumerate() {
         o[12 + k * 4..16 + k * 4].copy_from_slice(&v.to_be_bytes());
     }
-    o
+    Some(o)
 }
 
-/// Encodes `p` as ICC v4.3 bytes (identical tag data is shared between tags).
+/// Encodes `p` as ICC v4.3 bytes (identical tag data is shared between tags). A LUT whose
+/// stage layout its tag type can't store (only possible for a parsed profile whose LUT tag
+/// used the other direction's type) is left out; the matrix/TRC tags still describe it.
 pub fn encode(p: &Profile) -> Vec<u8> {
     let mut tags: Vec<([u8; 4], Vec<u8>)> = Vec::new();
     tags.push((*b"desc", mluc(&p.description)));
@@ -277,7 +285,9 @@ pub fn encode(p: &Profile) -> Vec<u8> {
                     LutKind::Lut8 => encode_mft(&l.stages, false),
                     LutKind::Ab => encode_ab(&l.stages, a2b, l.inputs, l.outputs),
                 };
-                tags.push((*name, data));
+                if let Some(data) = data {
+                    tags.push((*name, data));
+                }
             }
         }
     }

crates/cms/tests/cms.rs

@@ -507,3 +507,86 @@ fn oracle_moxcms_system_cmyk() {
     let p99 = sorted[sorted.len() * 99 / 100];
     assert!(mean < 1.5 && p99 <= 6, "max {worst} p99 {p99} mean {mean}");
 }
+
+#[test]
+fn builtin_all_in_declaration_order() {
+    // `Builtin::profile` indexes its cache by discriminant.
+    for (i, b) in Builtin::ALL.iter().enumerate() {
+        assert_eq!(*b as usize, i, "{b:?}");
+    }
+}
+
+/// `icc` with one more tag appended (the tag table grows by 12 bytes, so offsets shift).
+fn with_tag(icc: &[u8], sig: &[u8; 4], data: &[u8]) -> Vec<u8> {
+    let be = |o: usize| u32::from_be_bytes(icc[o..o + 4].try_into().unwrap());
+    let count = be(128) as usize;
+    let table_end = 132 + count * 12;
+    let mut body = icc[table_end..].to_vec();
+    while !body.len().is_multiple_of(4) {
+        body.push(0);
+    }
+    let mut out = icc[..128].to_vec();
+    out.extend_from_slice(&(count as u32 + 1).to_be_bytes());
+    for i in 0..count {
+        let o = 132 + i * 12;
+        out.extend_from_slice(&icc[o..o + 4]);
+        out.extend_from_slice(&(be(o + 4) + 12).to_be_bytes());
+        out.extend_from_slice(&icc[o + 8..o + 12]);
+    }
+    let new_off = (table_end + 12 + body.len()) as u32;
+    out.extend_from_slice(sig);
+    out.extend_from_slice(&new_off.to_be_bytes());
+    out.extend_from_slice(&(data.len() as u32).to_be_bytes());
+    out.extend_from_slice(&body);
+    out.extend_from_slice(data);
+    let size = out.len() as u32;
+    out[..4].copy_from_slice(&size.to_be_bytes());
+    out
+}
+
+/// A `lutBtoA` ("mBA ") tag, 1 → 3 channels, using every element:
+/// B curves, matrix, M curves, CLUT, A curves.
+fn mba_1_to_3() -> Vec<u8> {
+    let curv = b"curv\0\0\0\0\0\0\0\0"; // identity curve, 12 bytes
+    let mut d = b"mBA \0\0\0\0".to_vec();
+    d.extend_from_slice(&[1, 3, 0, 0]);
+    // offsets: B, matrix, M, CLUT, A
+    for off in [32u32, 44, 92, 104, 132] {
+        d.extend_from_slice(&off.to_be_bytes());
+    }
+    d.extend_from_slice(curv); // B @ 32
+    for k in 0..12 {
+        let v: i32 = if k % 4 == 0 && k < 9 { 0x10000 } else { 0 };
+        d.extend_from_slice(&v.to_be_bytes()); // matrix @ 44 (identity, zero offset)
+    }
+    d.extend_from_slice(curv); // M @ 92
+    let mut grid = [0u8; 16];
+    grid[0] = 2;
+    d.extend_from_slice(&grid); // CLUT @ 104
+    d.extend_from_slice(&[1, 0, 0, 0]); // 8-bit precision
+    d.extend_from_slice(&[0, 0, 0, 255, 255, 255]); // 2 nodes x 3 outputs
+    d.extend_from_slice(&[0, 0]); // pad to 132
+    for _ in 0..3 {
+        d.extend_from_slice(curv); // A @ 132
+    }
+    d
+}
+
+/// Regression: a gray profile whose A2B0 tag is stored as "mBA " parsed fine, but
+/// re-encoding its RGB view (`gray_as_rgb`, used for gray documents' composites) hit
+/// `panic!("unsupported lutAtoB stage layout")` in the ICC writer.
+#[test]
+fn gray_profile_with_reversed_lut_type_reencodes() {
+    let sgray = Builtin::SGray.profile().to_bytes();
+    let icc = with_tag(&sgray, b"A2B0", &mba_1_to_3());
+    let p = Profile::parse(&icc).unwrap();
+    assert!(p.a2b[0].is_some(), "the crafted A2B0 tag should parse");
+    let rgb = p.gray_as_rgb().unwrap();
+    assert!(rgb.is_matrix_shaper(), "RGB view keeps no gray LUTs");
+    let back = Profile::parse(&rgb.to_bytes()).unwrap();
+    assert_eq!(back.color_space, ColorSpace::Rgb);
+    // The writer itself must not panic on a LUT it can't store either.
+    let mut odd = p.clone();
+    odd.description = "re-encoded".into();
+    let _ = Profile::parse(&odd.with_encoded_bytes().to_bytes()).unwrap();
+}