qr: read compressed BBQr, expanding it through the paced driver

`Z` deflates a file and only then cuts it into parts, so a scan of one collects the
compressed stream and has to expand it. On the Q1 image that is 342 codes instead of
449 -- a quarter fewer, not the half the compression ratio suggests, because the
compressor keeps its back-references inside a kilobyte so a device can expand the
result with a window it can afford.

**Nothing holds a slice over the mapped region.** `inflate_to_slice` would have been
four lines and is the wrong tool: inflate writes a byte at a time and reads its own
history back, and byte stores and unpaced reads are the two things PSRAM mis-issues. So
the window lives in ordinary memory and both ends cross the medium in whole chunks
through `StagingArea` -- `minizlib::Reader` fetching input a chunk at a time,
`minizlib::Stream` handing output back a window at a time. Between them the
decompressor never touches the bus.

Reading the stream and writing the expansion are the same area and the two callbacks
cannot each hold `&mut`. They run strictly in turn, so a `RefCell` says so honestly --
and the borrow is *tried*, because a panic partway through writing a firmware image is
the worst place here to be wrong about a lifetime.

The stream lands at 6 MiB and expands down from zero, so the expansion never overwrites
input it has not read. A test pins that rather than the comment being the only thing
that says it.

It lives in `catcard-upgrade::expand` rather than the firmware so it can be tested
against a memory-backed area: six tests, including that the chunk size changes only how
often the bus turns round, that too wide a sender window is named rather than guessed,
and that **damage survives to where the signature can see it** -- raw deflate has no
checksum, so a corrupt stream can expand to exactly the right length and simply be the
wrong bytes. That is not a gap to close here; the image signature is the check. What
would be wrong is expanding the damage away.

`catcard-image qr` compresses when that makes fewer codes and says which it used. The
compressor can make a file with little redundancy longer, so the shorter of the two
wins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: MagicalTux

Cargo.lock

@@ -22,6 +22,13 @@
 //! counted on sight would report a complete file after a write that failed, and for a
 //! firmware image the only remaining check would be the signature.
 //!
+//! # `Z` reassembles a stream, not a file
+//!
+//! [`Encoding::Zlib`] deflates the whole file and only then cuts it up, so the parts
+//! place and reassemble exactly as any others do -- what comes out is the compressed
+//! stream. [`Collector::compressed`] says so; expanding it is the caller's, because
+//! only the caller knows where the bytes landed and what it can spend.
+//!
 //! # Where the bytes go is the caller's business
 //!
 //! Two consumers want different destinations: a firmware image goes to the staging area,
@@ -41,14 +48,6 @@ pub use outscript::bbqr::{
 pub enum Error {
     /// The format itself refused it: not a header, bad base32, an index out of range.
     Codec(outscript::bbqr::Error),
-    /// The parts are compressed as a whole, which this cannot reassemble in place.
-    ///
-    /// `Z` deflates the file *before* cutting it up, so no part means anything on its
-    /// own and the entire compressed stream has to be whole before any of it is data.
-    /// For a firmware image the only memory that holds it is the staging area the image
-    /// is being written into, so inflating would read that part while writing to it --
-    /// the documented way to corrupt it (`hw-reference/storage.md`).
-    Compressed,
     /// This part disagrees with the ones already seen about what file this is.
     Mismatch,
     /// The payload decodes to more than the caller left room for.
@@ -119,6 +118,15 @@ impl Collector {
         self.header
     }
 
+    /// Whether what is being collected is a deflate stream rather than the file.
+    ///
+    /// `Z` compresses the whole file before cutting it up, so the parts reassemble into
+    /// something that still has to be expanded. Nothing here does that -- the caller
+    /// knows where the bytes went and what it can spend on expanding them.
+    pub fn compressed(&self) -> bool {
+        self.header.is_some_and(|h| h.encoding == Encoding::Zlib)
+    }
+
     /// Whether every part has been seen.
     pub fn complete(&self) -> bool {
         self.header.is_some_and(|h| self.count == h.num_parts)
@@ -135,9 +143,6 @@ impl Collector {
     /// written it.
     pub fn accept(&mut self, line: &str) -> Result<Placed, Error> {
         let (header, body) = Header::parse(line)?;
-        if header.encoding == Encoding::Zlib {
-            return Err(Error::Compressed);
-        }
         let len = decoded_len_bound(header.encoding, body.len());
         let is_last = header.index + 1 == header.num_parts;
 

crates/catcard-bbqr/src/lib.rs

@@ -263,14 +263,28 @@ fn hex_works_too() {
     assert_eq!(out, file);
 }
 
-/// `Z` deflates the whole file before cutting it, so no part is data on its own and the
-/// compressed stream has to be reassembled before any of it can be inflated. Refused
-/// with its own reason, so the screen can say why rather than "bad code".
+/// `Z` deflates the whole file before cutting it, so the parts place like any others
+/// and what they reassemble into is the compressed stream. The collector says so and
+/// leaves the expanding to whoever knows where the bytes went.
 #[test]
-fn compressed_parts_are_refused_by_name() {
+fn compressed_parts_place_like_any_other() {
+    let stream = firmwareish(200);
+    let mut out = vec![0u8; stream.len()];
     let mut c = Collector::new();
-    let line = part_with(Encoding::Zlib, &[1, 2, 3, 4, 5], 2, 0);
-    assert_eq!(c.accept(&line), Err(Error::Compressed));
+    assert!(!c.compressed(), "nothing seen yet");
+    for i in 0..2u16 {
+        let at = i as usize * 100;
+        c.take(
+            &part_with(Encoding::Zlib, &stream[at..at + 100], 2, i),
+            &mut out,
+        )
+        .expect("a part");
+    }
+    assert!(c.compressed());
+    assert!(c.complete());
+    // The reassembled bytes are the deflate stream, not the file.
+    assert_eq!(c.file_len(), Some(stream.len()));
+    assert_eq!(out, stream);
 }
 
 #[test]

crates/catcard-bbqr/src/tests.rs

@@ -62,6 +62,7 @@ catcard-log.workspace = true
 catcard-wallet = { workspace = true, default-features = false }
 catcard-ui.workspace = true
 anyd.workspace = true
+minizlib.workspace = true
 cortex-m.workspace = true
 heapless.workspace = true
 cortex-m-rt.workspace = true

crates/catcard-fw/Cargo.toml

@@ -0,0 +1,58 @@
+//! The heap buffers a paced inflate needs, and where a compressed scan lands.
+//!
+//! The expansion itself is [`catcard_upgrade::expand`], which is where it can be tested
+//! against a memory-backed area rather than only against the real part. This is the
+//! wrapper that knows what this device can spend on it.
+//!
+//! BBQr's `Z` compresses a file and only then cuts it into parts, so a scan of one
+//! collects the compressed stream. On the Q1 image that is 342 codes instead of 449 --
+//! a quarter fewer, not the half the ratio suggests, because the compressor keeps its
+//! back-references inside a kilobyte so that a device can expand the result with a
+//! window it can afford.
+
+use crate::staging::Area;
+
+/// The history window given to the decompressor, from the heap.
+///
+/// BBQr compresses within a kilobyte, so this is eight times what a conforming sender
+/// needs. A stream made with a wider one fails with a reason rather than quietly
+/// producing wrong bytes -- a back-reference past the window is exactly the case the
+/// decompressor cannot serve, and it says so.
+pub const WINDOW: usize = 8 * 1024;
+
+/// Compressed input fetched per bus turnaround.
+///
+/// Every chunk costs a direction change and the driver pays a CE# gap for each, so a
+/// kilobyte at a time makes those rare without asking much of the heap.
+const CHUNK: usize = 1024;
+
+/// Where the scanner puts a compressed stream, as an offset into the staging area.
+///
+/// Past anything that can be expanded out of it: the expanded image starts at zero and
+/// the bootloader will not take one larger than the board's flash, which is a fraction
+/// of this. Keeping the two apart is what lets the expansion run forwards without ever
+/// overwriting input it has not read yet.
+pub const COMPRESSED_AT: u32 = 6 * 1024 * 1024;
+
+/// Expand `len` compressed bytes at [`COMPRESSED_AT`] into the area from offset zero.
+///
+/// The buffers come from the heap, which is the only reason this is not just the call
+/// underneath. Returns how many bytes came out.
+pub fn staged(area: &mut Area, len: u32, max: u32) -> Result<u32, &'static str> {
+    let (Some(mut window), Some(mut chunk)) = (crate::heap::take(WINDOW), crate::heap::take(CHUNK))
+    else {
+        return Err("not enough memory to expand it");
+    };
+    catcard_upgrade::expand::inflate(area, COMPRESSED_AT, len, max, window.bytes(), chunk.bytes())
+        .map_err(|why| {
+            crate::catlog!("inflate: refused: {:?}", why);
+            match why {
+                catcard_upgrade::expand::Error::WindowTooSmall => {
+                    "compressed with too wide a window"
+                }
+                catcard_upgrade::expand::Error::TooLong => "it expands to more than can be staged",
+                catcard_upgrade::expand::Error::Storage => "staging write failed",
+                catcard_upgrade::expand::Error::Damaged => "the compressed data is damaged",
+            }
+        })
+}

crates/catcard-fw/src/inflate.rs

@@ -44,6 +44,9 @@ mod keywork;
 mod logbuf;
 #[cfg(feature = "usb-debug-mem")]
 mod debug_mem;
+/// Expanding a deflate stream that is already in the staging area.
+#[cfg(feature = "board-q1")]
+mod inflate;
 mod interrupts;
 mod ktest;
 mod menu;

crates/catcard-fw/src/main.rs

@@ -40,6 +40,22 @@ pub(crate) trait Sink {
 
     /// Put `bytes` at `offset`. An error abandons the transfer.
     fn place(&mut self, offset: usize, bytes: &[u8]) -> Result<(), &'static str>;
+
+    /// What is arriving is a deflate stream, not the payload.
+    ///
+    /// Said once, before any bytes, because where a sink puts a stream it will have to
+    /// expand is not where it puts a file it can use as it stands.
+    fn compressed(&mut self) -> Result<(), &'static str> {
+        Ok(())
+    }
+}
+
+/// What a completed scan left behind.
+pub(crate) struct Received {
+    /// Bytes handed to the sink.
+    pub len: usize,
+    /// Whether they are a deflate stream rather than the payload itself.
+    pub compressed: bool,
 }
 
 /// What one code turned out to be worth.
@@ -49,6 +65,8 @@ struct Landed {
     total: u32,
     /// The payload's length, once every part is in. `None` while any are missing.
     complete: Option<usize>,
+    /// Whether what is being placed is a deflate stream.
+    compressed: bool,
 }
 
 /// Which format is being read, decided by the first line that parsed.
@@ -66,7 +84,7 @@ enum Which {
 ///
 /// `None` if the owner cancelled or the scanner could not be used; the reason has
 /// already been shown in either case.
-pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> Option<usize> {
+pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> Option<Received> {
     // Big enough for the largest line either format can hand over, decoded. BBQr's
     // base32 is five bits a character, so a line's payload is never more than five
     // eighths of it; BC-UR's bytewords are two characters a byte, so never more than a
@@ -79,12 +97,15 @@ pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> O
     };
 
     let mut which = Which::Unknown;
+    // The sink is told once that a stream is coming, not once per part.
+    let mut told = false;
     let mut failure: Option<&'static str> = None;
     // Redrawn only when a part lands that was not already held. Every other code is a
     // repeat of one already caught, and redrawing for those would make the screen flicker
     // through the whole animation without the count ever moving.
     let mut shown = (0u32, 0u32);
     let mut done = 0usize;
+    let mut compressed = false;
 
     let outcome = qrscan::scan_many(ui, head, &mut |ui, line| {
         let scratch = scratch_mem.bytes();
@@ -105,8 +126,9 @@ pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> O
         let Some(text) = as_text(line) else {
             return Next::More;
         };
-        match read_one(&mut which, text, scratch, sink) {
+        match read_one(&mut which, &mut told, text, scratch, sink) {
             Ok(Some(landed)) => {
+                compressed = landed.compressed;
                 if (landed.have, landed.total) != shown {
                     shown = (landed.have, landed.total);
                     progress(ui, head, landed.have, landed.total);
@@ -136,7 +158,10 @@ pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> O
         return None;
     }
     match outcome {
-        Ok(()) if done > 0 => Some(done),
+        Ok(()) if done > 0 => Some(Received {
+            len: done,
+            compressed,
+        }),
         Ok(()) => None,
         Err(qrscan::Fault::Cancelled) => None,
         Err(why) => {
@@ -153,6 +178,7 @@ pub(crate) fn collect_any(ui: &mut Ui<'_>, head: &str, sink: &mut dyn Sink) -> O
 /// fatal: the sink refused, or two different files are in shot.
 fn read_one(
     which: &mut Which,
+    told: &mut bool,
     line: &str,
     scratch: &mut [u8],
     sink: &mut dyn Sink,
@@ -173,17 +199,18 @@ fn read_one(
     match which {
         Which::Unknown => Ok(None),
         Which::Bbqr(collector) => {
-            let placed = match collector.accept(line) {
-                Ok(placed) => placed,
-                // The one refusal worth a screen: the sender compressed the file, which
-                // cannot be reassembled in place. Everything else is a bad frame.
-                Err(catcard_bbqr::Error::Compressed) => return Err("send it uncompressed"),
-                Err(_) => return Ok(None),
+            let Ok(placed) = collector.accept(line) else {
+                return Ok(None);
             };
             // An upper bound: every part but the last is this long, and the last is
             // shorter. Told before anything is written, because a sink that sizes itself
             // from this cannot be told after the fact.
             sink.expect(placed.len * placed.total as usize)?;
+            // Before any bytes land, because it decides where they land.
+            if collector.compressed() && !*told {
+                sink.compressed()?;
+                *told = true;
+            }
             if placed.fresh {
                 let room = scratch.get_mut(..placed.len).ok_or("a part was too long")?;
                 if catcard_bbqr::decode_part_to_slice(line, room).is_err() {
@@ -202,6 +229,7 @@ fn read_one(
                 have: placed.have as u32,
                 total: placed.total as u32,
                 complete: len,
+                compressed: collector.compressed(),
             }))
         }
         Which::Bcur(collector) => {
@@ -235,6 +263,8 @@ fn read_one(
                 have,
                 total,
                 complete: collector.complete().then_some(total_len).flatten(),
+                // BC-UR has no compressed form; a UR carries what it carries.
+                compressed: false,
             }))
         }
     }
@@ -297,11 +327,15 @@ fn progress(ui: &mut Ui<'_>, head: &str, have: u32, total: u32) {
 /// satisfied it.
 pub(crate) struct Staging {
     area: crate::staging::Area,
+    /// Where offset zero of the payload goes. Zero for a file, and out of the way for a
+    /// deflate stream, which has to survive being read while what it expands to is
+    /// written over the front of the area.
+    base: u32,
 }
 
 impl Staging {
     pub fn new(area: crate::staging::Area) -> Self {
-        Staging { area }
+        Staging { area, base: 0 }
     }
 
     /// The area back, with whatever was written in it.
@@ -314,18 +348,25 @@ impl Sink for Staging {
     fn expect(&mut self, about: usize) -> Result<(), &'static str> {
         use catcard_upgrade::StagingArea as _;
 
-        if about as u32 > self.area.capacity() {
+        let end = self.base.checked_add(about as u32).ok_or("bad length")?;
+        if end > self.area.capacity() {
             return Err("too large for this device");
         }
         Ok(())
     }
 
+    fn compressed(&mut self) -> Result<(), &'static str> {
+        self.base = crate::inflate::COMPRESSED_AT;
+        Ok(())
+    }
+
     fn place(&mut self, offset: usize, bytes: &[u8]) -> Result<(), &'static str> {
         use catcard_upgrade::StagingArea as _;
 
         let offset: u32 = offset.try_into().map_err(|_| "bad offset")?;
+        let at = self.base.checked_add(offset).ok_or("bad offset")?;
         self.area
-            .write(offset, bytes)
+            .write(at, bytes)
             .map_err(|_| "staging write failed")
     }
 }

crates/catcard-fw/src/qrload.rs

@@ -498,8 +498,34 @@ pub(crate) fn screen(gate: &Callgate, login: &mut catcard_pin::Login, ui: &mut U
         // Cancelled, or a reason already shown.
         return;
     };
-    crate::catlog!("qr: received {} bytes", got);
-    offer(gate, login, ui, HEAD, sink.into_area(), got);
+    let mut area = sink.into_area();
+    crate::catlog!(
+        "qr: received {} bytes{}",
+        got.len,
+        if got.compressed { ", compressed" } else { "" }
+    );
+
+    // A `Z` transfer reassembles the deflate stream, not the file. Expanding it is a
+    // pass over the whole thing, so it says so -- on a payload this size it is not
+    // instant, and a screen that has stopped changing reads as a device that has hung.
+    let len = if got.compressed {
+        menu::blocking_screen(ui.panel, HEAD, "expanding");
+        let max = catcard_board::BOARD.memory.firmware_flash_len;
+        match crate::inflate::staged(&mut area, got.len as u32, max) {
+            Ok(n) => {
+                crate::catlog!("qr: expanded to {} bytes", n);
+                n as usize
+            }
+            Err(why) => {
+                menu::message(ui.panel, HEAD, why, "any key to go back");
+                menu::wait_for_any_key(ui);
+                return;
+            }
+        }
+    } else {
+        got.len
+    };
+    offer(gate, login, ui, HEAD, area, len);
 }
 
 /// What the scanned bytes look like.