diff --git a/src/reader/converter.rs b/src/reader/converter.rs index 9833765..0f9480a 100644 --- a/src/reader/converter.rs +++ b/src/reader/converter.rs @@ -103,7 +103,8 @@ impl PixelConverter { current_frame: &Frame<'_>, mut buf: &mut [u8], data_callback: FillBufferCallback<'_>, - ) -> Result { + ) -> Result { + 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,11 +195,13 @@ 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, - }) { + pass: 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 @@ -202,47 +209,73 @@ impl PixelConverter { .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 { + 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(()) } } +/// Represents one of the four GIF interlace passes. +/// +/// GIF interlacing works in four passes: +/// - Pass 0: every 8th row, starting from row 0 +/// - Pass 1: every 8th row, starting from row 4 +/// - Pass 2: every 4th row, starting from row 2 +/// - Pass 3: every 2nd row, starting from row 1 +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Pass(pub usize); + struct InterlaceIterator { len: u16, next: usize, - pass: usize, + pass: Pass, } impl iter::Iterator for InterlaceIterator { - type Item = usize; + type Item = (usize, Pass); #[inline] fn next(&mut self) -> Option { 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)?; + let mut next = self.next + *[8, 8, 4, 2].get(self.pass.0)?; while next >= self.len as usize { - debug_assert!(self.pass < 4); - next = *[4, 2, 1, 0].get(self.pass)?; - self.pass += 1; + debug_assert!(self.pass.0 < 4); + next = *[4, 2, 1, 0].get(self.pass.0)?; + self.pass.0 += 1; } mem::swap(&mut next, &mut self.next); - Some(next) + Some((next, current_pass)) } } @@ -250,11 +283,11 @@ impl iter::Iterator for InterlaceIterator { mod test { use alloc::vec::Vec; - use super::InterlaceIterator; + use super::{InterlaceIterator, Pass}; #[rustfmt::skip] #[test] - fn test_interlace_iterator() { + fn test_interlace_iterator_row() { for &(len, expect) in &[ (0, &[][..]), (1, &[0][..]), @@ -275,19 +308,48 @@ mod test { (16, &[0, 8, 4, 12, 2, 6, 10, 14, 1, 3, 5, 7, 9, 11, 13, 15][..]), (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::>(); + let iter = InterlaceIterator { len, next: 0, pass: Pass(0) }; + let lines = iter.map(|(r, _)| r).collect::>(); 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: Pass(0) }; + let passes = iter.map(|(_, p)| p.0).collect::>(); + assert_eq!(passes, expect); + } + } + #[test] fn interlace_max() { let iter = InterlaceIterator { len: 0xFFFF, next: 0, - pass: 0, + pass: Pass(0), }; - assert_eq!(65533, iter.last().unwrap()); + assert_eq!((65533, Pass(3)), iter.last().unwrap()); } } diff --git a/src/reader/decoder.rs b/src/reader/decoder.rs index bd86cbb..d58fd3a 100644 --- a/src/reader/decoder.rs +++ b/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, } } } diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 2f6eaf6..96a9ff5 100644 --- a/src/reader/mod.rs +++ b/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 { - 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 diff --git a/tests/truncated.rs b/tests/truncated.rs new file mode 100644 index 0000000..37ed380 --- /dev/null +++ b/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, + ); +} diff --git a/tests/truncated/interlaced-truncated.png b/tests/truncated/interlaced-truncated.png new file mode 100644 index 0000000..1395255 Binary files /dev/null and b/tests/truncated/interlaced-truncated.png differ diff --git a/tests/truncated/moon_impact-truncated.png b/tests/truncated/moon_impact-truncated.png new file mode 100644 index 0000000..8d386c3 Binary files /dev/null and b/tests/truncated/moon_impact-truncated.png differ