Merge pull request #76 from image-rs/no-progress-with-small-buffer

Fix NoProgress indication despite buffered code

Author: 197g

Cargo.toml

@@ -97,5 +97,9 @@ required-features = ["alloc"]
 name = "fuzz_panic_regression"
 required-features = ["alloc"]
 
+[[test]]
+name = "progress_into_buffer"
+required-features = ["alloc"]
+
 [package.metadata.docs.rs]
 all-features = true

src/decode.rs

@@ -179,6 +179,8 @@ struct Buffer {
     bytes: Box<[u8]>,
     read_mark: usize,
     write_mark: usize,
+    /// Co-operatively used to track if decoding progress was written to the buffer, not `out`.
+    pub(crate) reconstructed_another_code: bool,
 }
 
 /// Mask for indexing into fixed-size arrays. Since MAX_ENTRIES = 4096 = 2^12,
@@ -795,6 +797,9 @@ impl<C: CodeBuffer, CgC: CodegenConstants> Stateful for DecodeState<C, CgC> {
         // The status, which is written to on an invalid code.
         let mut status = Ok(LzwStatus::Ok);
 
+        // Reset if a value was restored into the buffer.
+        self.buffer.reconstructed_another_code = false;
+
         match self.last.take() {
             // No last state? This is the first code after a reset?
             None => {
@@ -1161,8 +1166,11 @@ impl<C: CodeBuffer, CgC: CodegenConstants> Stateful for DecodeState<C, CgC> {
         }
 
         // Ensure we don't indicate that no progress was made if we read some bytes from the input
-        // (which is progress).
-        if o_in > inp.len() {
+        // (which is progress). The field `new_value` will have been written if any reconstruction
+        // was buffered. This is mostly a fail-safe in case the above loop does nothing else but
+        // buffer the value, without consuming any of the bytes. (It really should right now but
+        // that may change and it's not structurally required to).
+        if o_in > inp.len() || self.buffer.reconstructed_another_code {
             if let Ok(LzwStatus::NoProgress) = status {
                 status = Ok(LzwStatus::Ok);
             }
@@ -1403,6 +1411,7 @@ impl Buffer {
             bytes: vec![0; MAX_ENTRIES].into_boxed_slice(),
             read_mark: 0,
             write_mark: 0,
+            reconstructed_another_code: false,
         }
     }
 
@@ -1412,6 +1421,7 @@ impl Buffer {
     /// with the reconstruction of `B`.
     fn fill_cscsc(&mut self) -> u8 {
         self.bytes[self.write_mark] = self.bytes[0];
+        self.reconstructed_another_code = true;
         self.write_mark += 1;
         self.read_mark = 0;
         self.bytes[0]
@@ -1425,6 +1435,7 @@ impl Buffer {
         let mut memory = core::mem::replace(&mut self.bytes, Box::default());
 
         let out = &mut memory[..usize::from(depth)];
+        self.reconstructed_another_code = true;
         let last = table.reconstruct(code, out);
 
         self.bytes = memory;

tests/progress_into_buffer.rs

@@ -0,0 +1,30 @@
+use weezl::{decode::Configuration, encode, BitOrder, LzwStatus};
+
+#[test]
+fn fill_into_buffer_counts_as_progress() {
+    let data = vec![42u8; 8];
+    let encoded = encode::Encoder::new(BitOrder::Lsb, 8)
+        .encode(&data)
+        .unwrap();
+
+    let mut dec = Configuration::new(BitOrder::Lsb, 8)
+        .with_yield_on_full_buffer(true)
+        .build();
+
+    let mut result = Vec::new();
+    let mut inp = encoded.as_slice();
+    loop {
+        let mut tmp = [0u8; 1]; // 1-byte output buffer
+        let r = dec.decode_bytes(inp, &mut tmp);
+        inp = &inp[r.consumed_in..];
+        result.extend_from_slice(&tmp[..r.consumed_out]);
+        match r.status {
+            Ok(LzwStatus::Done) => break,
+            Ok(LzwStatus::NoProgress) if r.consumed_in == 0 && r.consumed_out == 0 => break,
+            Ok(_) => {}
+            Err(_) => break,
+        }
+    }
+
+    assert_eq!(result.len(), 8); // FAILS: result.len() == 1
+}