Improve processing of truncated GIFs

src/reader/converter.rs

@@ -103,7 +103,8 @@ impl PixelConverter {
         current_frame: &Frame<'_>,
         mut buf: &mut [u8],
         data_callback: FillBufferCallback<'_>,
-    ) -> Result<bool, DecodingError> {
+    ) -> Result<usize, DecodingError> {
+        let original_len = buf.len();
         loop {
             let decode_into = match self.color_output {
                 // When decoding indexed data, LZW can write the pixels directly
@@ -121,9 +122,9 @@ impl PixelConverter {
                     &mut self.buffer[..buffer_size]
                 }
             };
-            match data_callback(&mut OutputBuffer::Slice(decode_into))? {
-                0 => return Ok(false),
-                bytes_decoded => {
+            match data_callback(&mut OutputBuffer::Slice(decode_into)) {
+                Ok(0) => return Ok(original_len - buf.len()),
+                Ok(bytes_decoded) => {
                     match self.color_output {
                         ColorOutput::RGBA => {
                             let transparent = current_frame.transparent;
@@ -160,9 +161,13 @@ impl PixelConverter {
                         }
                     }
                     if buf.is_empty() {
-                        return Ok(true);
+                        return Ok(original_len);
                     }
                 }
+                Err(DecodingError::UnexpectedEof) => {
+                    return Ok(original_len - buf.len());
+                }
+                Err(e) => return Err(e),
             }
         }
     }
@@ -190,29 +195,46 @@ impl PixelConverter {
     ) -> Result<(), DecodingError> {
         if frame.interlaced {
             let width = self.line_length(frame);
-            for row in (InterlaceIterator {
+            let mut row_iter = InterlaceIterator {
                 len: frame.height,
                 next: 0,
                 pass: 0,
-            }) {
+            };
+            let mut truncated = false;
+            for (row, pass) in &mut row_iter {
                 // this can't overflow 32-bit, because row never equals (maximum) height
                 let start = row * width;
                 // Handle a too-small buffer and 32-bit usize overflow without panicking
                 let line = buf
                     .get_mut(start..)
                     .and_then(|b| b.get_mut(..width))
                     .ok_or_else(|| DecodingError::format("buffer too small"))?;
-                if !self.fill_buffer(frame, line, data_callback)? {
-                    return Err(DecodingError::format("image truncated"));
+                let filled = self.fill_buffer(frame, line, data_callback)?;
+                if filled < line.len() {
+                    truncated = true;
+                    match pass {
+                        0 => line[filled..].fill(0),
+                        1 => buf.copy_within((row - 4) * width..(row - 4) * width + width, start),
+                        2 => buf.copy_within((row - 2) * width..(row - 2) * width + width, start),
+                        3 => buf.copy_within((row - 1) * width..(row - 1) * width + width, start),
+                        _ => unreachable!(),
+                    };
                 }
             }
+            if truncated {
+                return Err(DecodingError::Truncated);
+            }
         } else {
             let buf = self
                 .buffer_size(frame)
                 .and_then(|buffer_size| buf.get_mut(..buffer_size))
                 .ok_or_else(|| DecodingError::format("buffer too small"))?;
-            if !self.fill_buffer(frame, buf, data_callback)? {
-                return Err(DecodingError::format("image truncated"));
+            let filled = self.fill_buffer(frame, buf, data_callback)?;
+            if filled < buf.len() {
+                // Once MSRV is >= 1.95:
+                // core::hint::cold_path();
+                buf[filled..].fill(0);
+                return Err(DecodingError::Truncated);
             }
         };
         Ok(())
@@ -226,13 +248,14 @@ struct InterlaceIterator {
 }
 
 impl iter::Iterator for InterlaceIterator {
-    type Item = usize;
+    type Item = (usize, usize);
 
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
         if self.len == 0 {
             return None;
         }
+        let current_pass = self.pass;
         // although the pass never goes out of bounds thanks to len==0,
         // the optimizer doesn't see it. get()? avoids costlier panicking code.
         let mut next = self.next + *[8, 8, 4, 2].get(self.pass)?;
@@ -242,7 +265,7 @@ impl iter::Iterator for InterlaceIterator {
             self.pass += 1;
         }
         mem::swap(&mut next, &mut self.next);
-        Some(next)
+        Some((next, current_pass))
     }
 }
 
@@ -254,7 +277,7 @@ mod test {
 
     #[rustfmt::skip]
     #[test]
-    fn test_interlace_iterator() {
+    fn test_interlace_iterator_row() {
         for &(len, expect) in &[
             (0, &[][..]),
             (1, &[0][..]),
@@ -276,18 +299,47 @@ mod test {
             (17, &[0, 8, 16, 4, 12, 2, 6, 10, 14, 1, 3, 5, 7, 9, 11, 13, 15][..]),
         ] {
             let iter = InterlaceIterator { len, next: 0, pass: 0 };
-            let lines = iter.collect::<Vec<_>>();
+            let lines = iter.map(|(r, _)| r).collect::<Vec<_>>();
             assert_eq!(lines, expect);
         }
     }
 
+    #[rustfmt::skip]
+    #[test]
+    fn test_interlace_iterator_pass() {
+        for &(len, expect) in &[
+            (0, &[][..]),
+            (1, &[0][..]),
+            (2, &[0, 3][..]),
+            (3, &[0, 2, 3][..]),
+            (4, &[0, 2, 3, 3][..]),
+            (5, &[0, 1, 2, 3, 3][..]),
+            (6, &[0, 1, 2, 3, 3, 3][..]),
+            (7, &[0, 1, 2, 2, 3, 3, 3][..]),
+            (8, &[0, 1, 2, 2, 3, 3, 3, 3][..]),
+            (9, &[0, 0, 1, 2, 2, 3, 3, 3, 3][..]),
+            (10, &[0, 0, 1, 2, 2, 3, 3, 3, 3, 3][..]),
+            (11, &[0, 0, 1, 2, 2, 2, 3, 3, 3, 3, 3][..]),
+            (12, &[0, 0, 1, 2, 2, 2, 3, 3, 3, 3, 3, 3][..]),
+            (13, &[0, 0, 1, 1, 2, 2, 2, 3, 3, 3, 3, 3, 3][..]),
+            (14, &[0, 0, 1, 1, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3][..]),
+            (15, &[0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3][..]),
+            (16, &[0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3][..]),
+            (17, &[0, 0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3][..]),
+        ] {
+            let iter = InterlaceIterator { len, next: 0, pass: 0 };
+            let passes = iter.map(|(_, p)| p).collect::<Vec<_>>();
+            assert_eq!(passes, expect);
+        }
+    }
+
     #[test]
     fn interlace_max() {
         let iter = InterlaceIterator {
             len: 0xFFFF,
             next: 0,
             pass: 0,
         };
-        assert_eq!(65533, iter.last().unwrap());
+        assert_eq!((65533, 3), iter.last().unwrap());
     }
 }

src/reader/decoder.rs

@@ -57,6 +57,8 @@ pub enum DecodingError {
     LzwError(LzwError),
     /// Returned if the image is found to be malformed.
     Format(DecodingFormatError),
+    /// Image truncated. The unfilled output buffer has been zeroed and is safe to use.
+    Truncated,
     /// Wraps `std::io::Error`.
     Io(io::Error),
 }
@@ -79,6 +81,7 @@ impl fmt::Display for DecodingError {
             Self::DecoderNotFound => fmt.write_str("Decoder Not Found"),
             Self::EndCodeNotFound => fmt.write_str("End-Code Not Found"),
             Self::UnexpectedEof => fmt.write_str("Unexpected End of File"),
+            Self::Truncated => fmt.write_str("Image truncated"),
             Self::LzwError(ref err) => err.fmt(fmt),
             Self::Format(ref d) => d.fmt(fmt),
             Self::Io(ref err) => err.fmt(fmt),
@@ -98,6 +101,7 @@ impl error::Error for DecodingError {
             Self::LzwError(ref err) => Some(err),
             Self::Format(ref err) => Some(err),
             Self::Io(ref err) => Some(err),
+            Self::Truncated => None,
         }
     }
 }

src/reader/mod.rs

@@ -563,10 +563,12 @@ where
     /// `Self::next_frame_info` needs to be called beforehand. Returns `true` if the supplied
     /// buffer could be filled completely. Should not be called after `false` had been returned.
     pub fn fill_buffer(&mut self, buf: &mut [u8]) -> Result<bool, DecodingError> {
-        self.pixel_converter
+        let filled = self
+            .pixel_converter
             .fill_buffer(&self.current_frame, buf, &mut |out| {
                 self.decoder.decode_next_bytes(out)
-            })
+            })?;
+        Ok(filled == buf.len())
     }
 
     /// Output buffer size

tests/truncated.rs

@@ -0,0 +1,88 @@
+#![cfg(feature = "std")]
+use gif::{ColorOutput, DecodeOptions, DecodingError};
+use std::fs::File;
+use std::io::{BufWriter, Read};
+use std::path::Path;
+
+fn test_truncation(gif_path: &str, png_path: &str, truncate_len: usize) {
+    let mut file = File::open(gif_path).expect("Failed to open GIF");
+    let mut data = Vec::new();
+    file.read_to_end(&mut data).expect("Failed to read GIF");
+
+    data.truncate(truncate_len);
+
+    let mut options = DecodeOptions::new();
+    options.set_color_output(ColorOutput::RGBA);
+
+    let mut decoder = options.read_info(&data[..]).expect("Failed to read info");
+
+    let mut hit_truncated = false;
+    let mut buf = Vec::new();
+
+    while let Ok(Some(_)) = decoder.next_frame_info() {
+        buf.resize(decoder.buffer_size(), 0);
+        match decoder.read_into_buffer(&mut buf) {
+            Ok(()) => {
+                println!("Decoded a frame!");
+            }
+            Err(DecodingError::Truncated) => {
+                println!("Hit Truncated error!");
+                hit_truncated = true;
+                break;
+            }
+            Err(e) => panic!("Unexpected error: {:?}", e),
+        }
+    }
+
+    assert!(hit_truncated);
+
+    // Save PNG if it hits truncated, for verification!
+    if !Path::new(png_path).exists() {
+        let width = decoder.width() as u32;
+        let height = decoder.height() as u32;
+        let file = File::create(png_path).expect("Failed to create PNG");
+        let ref mut w = BufWriter::new(file);
+        let mut encoder = png::Encoder::new(w, width, height);
+        encoder.set_color(png::ColorType::Rgba);
+        encoder.set_depth(png::BitDepth::Eight);
+        let mut writer = encoder.write_header().expect("Failed to write header");
+        writer
+            .write_image_data(&buf)
+            .expect("Failed to write image data");
+        println!("Generated expected PNG: {}", png_path);
+    } else {
+        println!("Comparing against existing PNG: {}", png_path);
+        // Read expected PNG and compare
+        let file = File::open(png_path).expect("Failed to open PNG");
+        let decoder = png::Decoder::new(std::io::BufReader::new(file));
+        let mut reader = decoder.read_info().expect("Failed to read PNG info");
+        let mut expected_buf = vec![
+            0;
+            reader
+                .output_buffer_size()
+                .expect("Failed to get output buffer size")
+        ];
+        reader
+            .next_frame(&mut expected_buf)
+            .expect("Failed to read PNG frame");
+        assert_eq!(buf, expected_buf);
+    }
+}
+
+#[test]
+fn test_truncated_non_interlaced() {
+    test_truncation(
+        "tests/samples/moon_impact.gif",
+        "tests/truncated/moon_impact-truncated.png",
+        5000,
+    );
+}
+
+#[test]
+fn test_truncated_interlaced() {
+    test_truncation(
+        "tests/samples/interlaced.gif",
+        "tests/truncated/interlaced-truncated.png",
+        5000,
+    );
+}