Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/reader/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,17 @@ impl StreamingDecoder {
self.version
}

/// Whether the decoder has cleanly finished a frame and the next byte it
/// expects is a block introducer that has not yet been read. Used to treat a
/// trailing EOF as a clean end-of-stream for GIFs missing the `0x3B` trailer.
///
/// Only `BlockEnd` qualifies. `BlockStart(_)` does not: reaching it means the
/// introducer byte was already read, so a block has started and its body is
/// now expected — an EOF there is a truncated block, not a missing trailer.
pub(crate) fn is_at_block_boundary(&self) -> bool {
matches!(self.state, BlockEnd)
}

#[inline]
fn next_state(
&mut self,
Expand Down
25 changes: 25 additions & 0 deletions src/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ struct ReadDecoder<R: Read> {
reader: io::BufReader<R>,
decoder: StreamingDecoder,
at_eof: bool,
/// Whether at least one frame's image data has been fully decoded.
///
/// Used to tolerate GIFs missing the final trailer byte (`0x3B`): a clean
/// EOF at a block boundary after a complete frame is treated as
/// end-of-stream rather than an error. EOF before any frame, or partway
/// through a frame, is still a genuine truncation error.
any_frame_decoded: bool,
}

impl<R: Read> ReadDecoder<R> {
Expand All @@ -226,6 +233,18 @@ impl<R: Read> ReadDecoder<R> {
let (consumed, result) = {
let buf = self.reader.fill_buf()?;
if buf.is_empty() {
// Tolerate GIFs missing the final trailer byte (`0x3B`): if we
// have already decoded at least one complete frame and the input
// ends exactly at a block boundary (the decoder is waiting for the
// next block introducer / trailer), treat it as a clean
// end-of-stream. Many real-world GIFs omit the trailer and other
// decoders (browsers, ImageMagick, PIL) accept them. EOF before any
// frame, or partway through a frame's data, remains a genuine
// truncation error.
if self.any_frame_decoded && self.decoder.is_at_block_boundary() {
self.at_eof = true;
break;
}
return Err(DecodingError::UnexpectedEof);
}

Expand All @@ -234,6 +253,11 @@ impl<R: Read> ReadDecoder<R> {
self.reader.consume(consumed);
match result {
Decoded::Nothing => (),
Decoded::DataEnd => {
// A frame's image data has been fully decoded.
self.any_frame_decoded = true;
return Ok(Some(Decoded::DataEnd));
}
Decoded::BlockStart(Block::Trailer) => {
self.at_eof = true;
}
Expand Down Expand Up @@ -315,6 +339,7 @@ where
reader: io::BufReader::new(reader),
decoder,
at_eof: false,
any_frame_decoded: false,
},
bg_color: None,
pixel_converter: PixelConverter::new(options.color_output),
Expand Down
132 changes: 132 additions & 0 deletions tests/missing_trailer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
//! Tests for tolerating GIFs that are missing the final trailer byte (`0x3B`).
//!
//! Many real-world GIFs omit the trailer block; browsers, ImageMagick, and PIL
//! decode them. When EOF is hit at a clean block boundary *after* a complete
//! frame, the decoder reports a clean end-of-stream rather than `UnexpectedEof`.
//! EOF in the middle of a frame, after a bare block introducer, or before any
//! frame, is still a genuine truncation error.
#![cfg(feature = "std")]

use gif::{ColorOutput, DecodeOptions, DecodingError, DisposalMethod, Encoder, Frame};

/// Build a valid, complete two-frame GIF (with the `0x3B` trailer).
fn build_two_frame_gif() -> Vec<u8> {
let mut data = Vec::new();
{
let mut encoder = Encoder::new(&mut data, 2, 2, &[0, 0, 0, 255, 255, 255]).unwrap();

let mut frame = Frame {
delay: 1,
dispose: DisposalMethod::Any,
transparent: None,
needs_user_input: false,
top: 0,
left: 0,
width: 2,
height: 2,
interlaced: false,
palette: None,
buffer: vec![0, 1, 1, 0].into(),
};
encoder.write_frame(&frame).unwrap();

frame.buffer = vec![1, 0, 0, 1].into();
encoder.write_frame(&frame).unwrap();
}
data
}

/// Decode every frame, returning the per-frame indexed buffers, or the error
/// that terminated decoding.
fn decode_all(bytes: &[u8]) -> Result<Vec<Vec<u8>>, DecodingError> {
let mut options = DecodeOptions::new();
options.set_color_output(ColorOutput::Indexed);
let mut decoder = options.read_info(bytes)?;

let mut frames = Vec::new();
loop {
match decoder.read_next_frame()? {
Some(frame) => frames.push(frame.buffer.to_vec()),
None => return Ok(frames),
}
}
}

/// Positive: a valid multi-frame GIF with the trailer stripped decodes ALL
/// frames cleanly, pixel-identical to the with-trailer decode.
#[test]
fn missing_trailer_decodes_all_frames_cleanly() {
let with_trailer = build_two_frame_gif();
assert_eq!(*with_trailer.last().unwrap(), 0x3B);

let trailerless = &with_trailer[..with_trailer.len() - 1];
let baseline = decode_all(&with_trailer).expect("with-trailer GIF must decode");
let stripped = decode_all(trailerless).expect("trailerless GIF must decode cleanly");

assert_eq!(baseline.len(), 2);
assert_eq!(
stripped, baseline,
"trailerless decode must match with-trailer"
);
}

/// Negative: truncation in the MIDDLE of a frame's image data must still error.
#[test]
fn truncation_mid_frame_still_errors() {
let full = build_two_frame_gif();
let second_image_sep = full
.iter()
.enumerate()
.filter(|&(_, &b)| b == 0x2C)
.nth(1)
.map(|(i, _)| i)
.expect("expected two image separators");
// Cut a couple of bytes into the second frame's LZW data.
let truncated = &full[..second_image_sep + 12];

let err = decode_all(truncated).expect_err("mid-frame truncation must error");
assert!(matches!(
err,
DecodingError::UnexpectedEof | DecodingError::Truncated
));
}

/// Negative (the review fix): a complete first frame followed by a bare block
/// introducer and then EOF must error. Once the next introducer byte has been
/// read the decoder is mid-block, so a missing body is genuine truncation — not
/// a missing trailer. The `any_frame_decoded` guard does not catch this (a frame
/// *was* decoded), so the boundary check must reject the `BlockStart` state.
#[test]
fn introducer_then_eof_after_frame_still_errors() {
let full = build_two_frame_gif();
// Cut right after the second image separator: frame one is complete, the
// `0x2C` introducer has been read, but its descriptor/body is gone.
let second_image_sep = full
.iter()
.enumerate()
.filter(|&(_, &b)| b == 0x2C)
.nth(1)
.map(|(i, _)| i)
.expect("expected two image separators");
let truncated = &full[..second_image_sep + 1];

let err = decode_all(truncated)
.expect_err("introducer-then-EOF after a frame must error, not silently truncate");
assert!(matches!(
err,
DecodingError::UnexpectedEof | DecodingError::Truncated
));
}

/// Negative: zero frames then EOF (header only) must still error.
#[test]
fn zero_frames_then_eof_still_errors() {
let full = build_two_frame_gif();
let first_image_sep = full.iter().position(|&b| b == 0x2C).unwrap();
let truncated = &full[..first_image_sep + 1];

assert!(
decode_all(truncated).is_err(),
"zero-frame truncation must error"
);
}
Loading