crw: fix the low-bits discriminator on payloads that end in the trailer

A no-plane PowerShot frame whose payload is shorter than plane + header ends
inside the scan window with the record's real `ff d9 00` trailer, so rule (a)
read the `ff d9` as a plane byte, reported 12 bits and then refused a valid
file. Both discriminator signs now require a payload long enough to hold a
plane, so a flat G1 frame decodes instead of being rejected.

Also from the review: clamp samples to the declared depth so a damaged stream
cannot put more than `bits` into a consumer's `1 << bits` LUT, report zero
sensor dimensions as corrupt rather than as a limit, take a signed ImageInfo
rotation modulo 360 (so -90 is 270), warn against pairing BlackLevel::values
with Header::cfa by index, and pin actions/cache in the crate's workflow.

Tests cover the trailer payload, end-of-block chain values, the clamp, zero
dimensions, both white-balance slot layouts with every Canon setting, and each
rotation spelling. The 11 CC0 samples still decode exact with
CRAFT_CRW_REQUIRE_SAMPLES=1.

.github/workflows/crw.yml

@@ -67,7 +67,7 @@ jobs:
         with:
           key: craft-crw-samples
       - name: Cache the CC0 sample files
-        uses: actions/cache@v4
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
         with:
           path: ${{ runner.temp }}/crw-samples
           key: crw-samples-${{ hashFiles('crates/crw/tests/samples.json') }}

crates/crw/src/lib.rs

@@ -114,6 +114,11 @@ impl Rect {
 }
 
 /// Black level per 2×2 CFA site, row-major, anchored at the active area's origin.
+///
+/// The anchor differs from [`Header::cfa`]'s: `cfa` is the phase at sensor (0, 0), `values` is
+/// indexed from the active area's origin. The active area can start at an odd sensor row (the
+/// PowerShot G1 starts at y = 15), so `values[i]` does not line up with `cfa.pattern()[i]`: read a
+/// level through [`BlackLevel::at`] rather than pairing the two by index.
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub struct BlackLevel {
     pub values: [f32; 4],
@@ -302,7 +307,10 @@ fn parse(bytes: &[u8], limits: &Limits, full: bool) -> Result<Image> {
         (Some(&w), Some(&h)) => (w as usize, h as usize),
         _ => return Err(corrupt("SensorInfo too short")),
     };
-    let n = w.checked_mul(h).filter(|&n| n > 0 && n <= limits.max_samples).ok_or(Error::LimitExceeded("image too large"))?;
+    if w == 0 || h == 0 {
+        return Err(corrupt("zero sensor dimensions"));
+    }
+    let n = w.checked_mul(h).filter(|&n| n <= limits.max_samples).ok_or(Error::LimitExceeded("image too large"))?;
 
     let &(dto, dtl) = rec.get(&DECODER_TABLE).ok_or_else(|| corrupt("no DecoderTable record"))?;
     if dtl < 16 {
@@ -352,8 +360,12 @@ fn parse(bytes: &[u8], limits: &Limits, full: bool) -> Result<Image> {
     let q = n.div_ceil(4);
     let span = q.saturating_add(ent_off);
     let head = payload.get(..span.min(payload.len())).unwrap_or(payload);
-    let header_after_plane = ent_off >= 64 && payload.len() > span && payload.get(q..span).is_some_and(|z| z.iter().all(|&b| b == 0));
-    let has_low = header_after_plane || head.windows(2).any(|p| p[0] == 0xff && p[1] != 0x00);
+    // Both signs need a payload long enough to hold a plane and the header behind it. A shorter
+    // payload ends in the PowerShot `ff d9 00` trailer, and `ff d9` is a 0xff not followed by
+    // 0x00: without this gate a flat no-plane G1 frame read as 12-bit and was then refused.
+    let plane_fits = payload.len() > span;
+    let header_after_plane = plane_fits && ent_off >= 64 && payload.get(q..span).is_some_and(|z| z.iter().all(|&b| b == 0));
+    let has_low = header_after_plane || (plane_fits && head.windows(2).any(|p| p[0] == 0xff && p[1] != 0x00));
     let bits = if has_low { 12 } else { 10 };
     let low_len = if has_low { q } else { 0 };
     let samples = if full {
@@ -364,13 +376,17 @@ fn parse(bytes: &[u8], limits: &Limits, full: bool) -> Result<Image> {
         }
         let hi = decode_entropy(entropy, w, h, &tables)?;
         let mut samples = vec![0u16; n];
+        // `samples` is documented as `bits` deep, so a damaged stream whose chain value runs past
+        // the range is clamped here: a consumer indexing a `1 << bits` LUT must not see more.
+        let top = ((1u32 << bits) - 1) as u16;
         for (i, (dst, &hi)) in samples.iter_mut().zip(hi.iter()).enumerate() {
             *dst = match low {
                 Some(low) => {
                     let lb = low.get(i >> 2).map_or(0, |&b| (b >> ((i & 3) * 2)) & 3);
-                    ((((hi as u32) << 2) | lb as u32) & 0xffff) as u16
+                    let v = (((hi as u32) << 2) | lb as u32) & 0xffff;
+                    v.min(u32::from(top)) as u16
                 }
-                None => hi,
+                None => hi.min(top),
             };
         }
         samples
@@ -387,7 +403,11 @@ fn parse(bytes: &[u8], limits: &Limits, full: bool) -> Result<Image> {
 
     // ImageInfo rotation: degrees; 270 observed on portrait files whose scene is upright after a
     // 90° counter-clockwise turn (Exif 8), so 90 is its mirror (Exif 6)
-    let exif_orientation = match rec.get(&IMAGE_INFO).and_then(|&(io, il)| (il >= 16).then(|| u32_at(bytes, io + 12, o)).flatten()) {
+    // The record holds a signed degree count, so a file may write −90 for 270: take it modulo
+    // 360 before mapping, and leave anything off-axis (0, ±360) at the identity.
+    let rotation_deg =
+        rec.get(&IMAGE_INFO).and_then(|&(io, il)| (il >= 16).then(|| u32_at(bytes, io + 12, o)).flatten()).map(|r| (r as i32).rem_euclid(360));
+    let exif_orientation = match rotation_deg {
         Some(90) => 6,
         Some(180) => 3,
         Some(270) => 8,

crates/crw/src/tests.rs

@@ -99,16 +99,17 @@ fn encode_hi(hi: &[u16], w: usize, t: &tables::SelTables) -> Vec<u8> {
             bits.extend((0..s).rev().map(|i| (m >> i) & 1 == 1));
         }
     }
-    let mut bytes = Vec::new();
+    pack_entropy(&bits)
+}
+
+/// MSB-first packing into bytes, zero padding in the last byte, then ff00 stuffing.
+fn pack_entropy(bits: &[bool]) -> Vec<u8> {
+    let mut stuffed = Vec::new();
     for chunk in bits.chunks(8) {
         let mut b = 0u8;
         for (i, &bit) in chunk.iter().enumerate() {
             b |= (bit as u8) << (7 - i);
         }
-        bytes.push(b);
-    }
-    let mut stuffed = Vec::new();
-    for b in bytes {
         stuffed.push(b);
         if b == 0xff {
             stuffed.push(0x00);
@@ -117,6 +118,25 @@ fn encode_hi(hi: &[u16], w: usize, t: &tables::SelTables) -> Vec<u8> {
     stuffed
 }
 
+/// One end-of-block token pair per 64-sample block: the block's first sample takes the carry
+/// (zero here), the other 63 keep their chain values. A flat frame like this compresses to a
+/// small fraction of its low-bits plane, the shape a lens-cap or blown-out frame has.
+fn encode_eob(n: usize, t: &tables::SelTables) -> Vec<u8> {
+    let code = |tab: &[(u16, u8, u8, u8)]| {
+        let &(c, l, ..) = tab.iter().find(|e| e.2 == 0 && e.3 == 0).expect("an end-of-block code");
+        (c, l)
+    };
+    let (fc, fl) = code(t.first);
+    let (mc, ml) = code(t.main);
+    let mut bits: Vec<bool> = Vec::new();
+    for _ in 0..n.div_ceil(64) {
+        for (c, l) in [(fc, fl), (mc, ml)] {
+            bits.extend((0..l).rev().map(|i| (c >> i) & 1 == 1));
+        }
+    }
+    pack_entropy(&bits)
+}
+
 /// A high plane whose diffs are all nonzero, bounded around 512: values follow
 /// `(3 qi) mod 5 + 1`, so same-parity neighbours differ by 1 or −4 (never 0) and
 /// row starts sit 1–5 above the reset value.
@@ -132,6 +152,22 @@ fn payload(low: Option<&[u8]>, entropy: &[u8]) -> Vec<u8> {
     p
 }
 
+/// The real PowerShot record shape: the payload closes with `ff d9` and a pad byte, not `00 00`.
+fn payload_with_trailer(low: Option<&[u8]>, entropy: &[u8]) -> Vec<u8> {
+    let mut p = low.map(|l| l.to_vec()).unwrap_or_default();
+    p.extend_from_slice(&[0; 514]);
+    p.extend_from_slice(entropy);
+    p.extend_from_slice(&[0xff, 0xd9, 0x00]);
+    p
+}
+
+/// A file whose raw record is `payload` verbatim, with `extra` records beside the usual three.
+fn file_with_payload(w: u16, h: u16, model: &str, payload: &[u8], extra: &[(u16, Vec<u8>)], o: Order) -> Vec<u8> {
+    let mut props = vec![(SENSOR_INFO, sensor_info(w, h, None, o)), (DECODER_TABLE, decoder_table(0, o)), (MAKE_MODEL, make_model(model))];
+    props.extend_from_slice(extra);
+    ciff(&[(RAW_DATA, payload.to_vec()), (0x300a, heap(&props, o))], o)
+}
+
 fn make_model(model: &str) -> Vec<u8> {
     let mut v = b"Canon\0".to_vec();
     v.extend_from_slice(model.as_bytes());
@@ -458,22 +494,17 @@ fn white_balance_comes_from_the_shot_info_slot() {
 fn rotation_maps_to_exif_orientation_and_the_time_stamp_is_unix_seconds() {
     let o = Order::Le;
     let hi = test_hi(128);
-    let image_info: Vec<u8> = [3072u32, 2048, 0, 270, 12, 12, 0].iter().flat_map(|&v| put32(v, o)).collect();
-    let props = heap(
-        &[
-            (SENSOR_INFO, sensor_info(16, 8, None, o)),
-            (DECODER_TABLE, decoder_table(0, o)),
-            (MAKE_MODEL, make_model("Canon EOS 300D DIGITAL")),
-            (IMAGE_INFO, image_info),
-            (TIME_STAMP, [1083410638u32, 0, 0].iter().flat_map(|&v| put32(v, o)).collect()),
-        ],
-        o,
-    );
     let tables = tables::tables(0).expect("selector 0");
-    let f = ciff(&[(RAW_DATA, payload(None, &encode_hi(&hi, 16, &tables))), (0x300a, props)], o);
-    let img = full(&f).expect("decodes");
-    assert_eq!(img.header.exif_orientation, 8);
-    assert_eq!(img.header.capture_unix, Some(1083410638));
+    // The record holds a signed degree count, so 270 may also be written as −90.
+    for (deg, want) in [(0u32, 1u8), (90, 6), (180, 3), (270, 8), (-90i32 as u32, 8), (-270i32 as u32, 6)] {
+        let image_info: Vec<u8> = [3072u32, 2048, 0, deg, 12, 12, 0].iter().flat_map(|&v| put32(v, o)).collect();
+        let stamp: Vec<u8> = [1083410638u32, 0, 0].iter().flat_map(|&v| put32(v, o)).collect();
+        let extra = [(IMAGE_INFO, image_info), (TIME_STAMP, stamp)];
+        let f = file_with_payload(16, 8, "Canon EOS 300D DIGITAL", &payload(None, &encode_hi(&hi, 16, &tables)), &extra, o);
+        let img = full(&f).expect("decodes");
+        assert_eq!(img.header.exif_orientation, want, "rotation {deg}");
+        assert_eq!(img.header.capture_unix, Some(1083410638));
+    }
 }
 
 #[test]
@@ -528,6 +559,106 @@ fn self_referencing_sub_heaps_terminate() {
     }
 }
 
+/// A no-plane G1 frame that compresses well closes with the record's real `ff d9 00` trailer and
+/// is still shorter than plane + header — the shape of a lens-cap or blown-out frame. Rule (a)
+/// read that `ff d9` as a plane byte, reported 12 bits and then refused the file.
+#[test]
+fn a_short_powershot_payload_closing_with_the_trailer_has_no_plane() {
+    let o = Order::Le;
+    let (w, h) = (64u16, 64u16);
+    let n = w as usize * h as usize;
+    let tables = tables::tables(0).expect("selector 0");
+    let entropy = encode_eob(n, &tables);
+    let p = payload_with_trailer(None, &entropy);
+    let span = n.div_ceil(4) + 514;
+    assert!(p.len() < span, "the payload must be shorter than plane + header: {} vs {span}", p.len());
+    assert!(p.windows(2).any(|q| q[0] == 0xff && q[1] != 0x00), "the trailer is the unstuffed ff pair");
+
+    let f = file_with_payload(w, h, "Canon PowerShot G1", &p, &[], o);
+    let hdr = header(&f, &Limits::default()).expect("header");
+    assert_eq!(hdr.bits, 10, "the trailer is not a low-bits plane");
+    // Every block is one end-of-block token, so the chain values stand (512 at each row start):
+    // the frame decodes instead of being refused for a plane it does not have.
+    let img = full(&f).expect("decodes");
+    assert_eq!((img.header.bits, img.samples.iter().filter(|&&v| v == 512).count()), (10, n));
+
+    // The same frame without the trailer was already read as 10-bit.
+    let f = file_with_payload(w, h, "Canon PowerShot G1", &payload(None, &entropy), &[], o);
+    assert_eq!(header(&f, &Limits::default()).expect("header").bits, 10);
+}
+
+/// An end-of-block token at the head of a block emits the carry and leaves the other 63 samples
+/// at their chain values: the runs the sample hashes cover, on a file the unit tests can read.
+#[test]
+fn end_of_block_keeps_the_chain_values() {
+    let o = Order::Le;
+    let (w, h) = (64u16, 64u16);
+    let n = w as usize * h as usize;
+    let tables = tables::tables(0).expect("selector 0");
+    let f = file_with_payload(w, h, "Canon EOS D30", &payload(None, &encode_eob(n, &tables)), &[], o);
+    let img = full(&f).expect("decodes");
+    assert_eq!(img.samples.len(), n);
+    assert!(img.samples.iter().all(|&v| v == 512), "every sample keeps its row's chain value");
+}
+
+/// Zero sensor dimensions are damage, not a request for too much memory.
+#[test]
+fn zero_sensor_dimensions_are_corrupt_not_a_limit() {
+    let f = roundtrip_file(0, 0, "Canon EOS D60", None, &[], Order::Le);
+    assert!(matches!(full(&f), Err(Error::Corrupt(_))));
+    assert!(matches!(header(&f, &Limits::default()), Err(Error::Corrupt(_))));
+}
+
+/// A hostile stream can run the chain past the bit depth (here a "10-bit" body writing 1535).
+/// `samples` is documented as `bits` deep, so the surplus never reaches a `1 << bits` LUT.
+#[test]
+fn samples_are_clamped_to_the_declared_depth() {
+    let f = roundtrip_file(2, 1, "Canon PowerShot G1", None, &[512, 1535], Order::Le);
+    let img = full(&f).expect("decodes");
+    assert_eq!((img.header.bits, img.samples.as_slice()), (10, &[512u16, 1023][..]));
+    // the 12-bit merge of the same chain cannot leave 0..=4095 either
+    let low = vec![0xffu8; 1];
+    let f = roundtrip_file(2, 1, "Canon EOS D60", Some(&low), &[512, 1535], Order::Le);
+    let img = full(&f).expect("decodes");
+    assert_eq!(img.header.bits, 12);
+    assert!(img.samples.iter().all(|&v| v <= 4095), "{:?}", img.samples);
+}
+
+/// The two ColorBalance layouts and every Canon setting that picks a slot. Every corpus file is
+/// set to Auto, so a wrong arm would still pass on the samples alone.
+#[test]
+fn white_balance_slot_arms_follow_the_camera_setting() {
+    let o = Order::Le;
+    // Quadruple k is (100+k, 200, 200, 300+k): the multipliers name the slot chosen, and the last
+    // quadruple is black levels, which is never a slot.
+    let wb = |usable: u16, setting: u16| {
+        let mut vals = vec![(usable + 1) * 8];
+        for k in 0..=usable {
+            vals.extend([100 + k, 200, 200, 300 + k]);
+        }
+        let mut bytes: Vec<u8> = vals.iter().flat_map(|&v| put16(v, o)).collect();
+        let cb_len = bytes.len();
+        let si_off = bytes.len();
+        let mut shot = [0u16; 8];
+        shot[7] = setting;
+        bytes.extend(shot.iter().flat_map(|&v| put16(v, o)));
+        let mut rec = BTreeMap::new();
+        rec.insert(COLOR_BALANCE, (0usize, cb_len));
+        rec.insert(SHOT_INFO, (si_off, 16usize));
+        wb_from_color_balance(&bytes, &rec, o)
+    };
+    let expect = |k: u16| Some([(100 + k) as f32 / 200.0, 1.0, (300 + k) as f32 / 200.0]);
+    // 10D/300D: Auto, Daylight, Shade, Cloudy, Tungsten, Fluorescent, Flash, Custom, Kelvin
+    for (setting, slot) in [(0u16, 0u16), (1, 1), (8, 2), (2, 3), (3, 4), (4, 5), (5, 6), (6, 7), (9, 8), (7, 0)] {
+        assert_eq!(wb(9, setting), expect(slot), "nine-slot setting {setting}");
+    }
+    // D60: Auto, Daylight, Cloudy, Tungsten, Fluorescent, Flash, Custom. Shade and Kelvin have no
+    // slot of their own and fall back to Auto.
+    for (setting, slot) in [(0u16, 0u16), (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (8, 0), (9, 0)] {
+        assert_eq!(wb(7, setting), expect(slot), "seven-slot setting {setting}");
+    }
+}
+
 #[test]
 fn errors_display_their_cause() {
     assert_eq!(Error::NotCrw.to_string(), "not a Canon CRW (CIFF) file");