Skip to content

Commit a0ba4ce

Browse files
committed
Tolerate GIFs missing the final trailer byte (0x3B)
Many real-world GIFs omit the GIF trailer block (0x3B). Browsers, ImageMagick, and PIL all decode them, but previously the decoder hit EOF while looking for the next block introducer and surfaced UnexpectedEof, rejecting a fully decodable GIF as corrupt. This relaxes only the clean-boundary case: when at least one frame has been fully decoded and the input ends exactly in the `BlockEnd` state (a complete frame finished, the next block introducer not yet read), a trailing EOF is reported as a clean end-of-stream. Only `BlockEnd` qualifies — `BlockStart(_)` means the next introducer byte was already read, so a block has started and its body is owed; an EOF there is genuine truncation. EOF mid-frame, after a bare introducer, or before any frame still errors. Adds tests covering the positive case and the three truncation cases (mid-frame, introducer-then-EOF after a frame, and zero-frame).
1 parent a5f89dd commit a0ba4ce

3 files changed

Lines changed: 125 additions & 0 deletions

File tree

src/reader/decoder.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,13 @@ impl StreamingDecoder {
543543
self.version
544544
}
545545

546+
/// At a post-frame block boundary, with the next block introducer not yet
547+
/// read. `BlockStart(_)` is excluded: the introducer is already read there,
548+
/// so an EOF is a truncated block, not a missing trailer.
549+
pub(crate) fn is_at_block_boundary(&self) -> bool {
550+
matches!(self.state, BlockEnd)
551+
}
552+
546553
#[inline]
547554
fn next_state(
548555
&mut self,

src/reader/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ struct ReadDecoder<R: Read> {
214214
reader: io::BufReader<R>,
215215
decoder: StreamingDecoder,
216216
at_eof: bool,
217+
/// At least one frame fully decoded; gates missing-trailer tolerance.
218+
any_frame_decoded: bool,
217219
}
218220

219221
impl<R: Read> ReadDecoder<R> {
@@ -226,6 +228,12 @@ impl<R: Read> ReadDecoder<R> {
226228
let (consumed, result) = {
227229
let buf = self.reader.fill_buf()?;
228230
if buf.is_empty() {
231+
// Missing-trailer tolerance: clean EOF at a post-frame block
232+
// boundary is end-of-stream, not truncation.
233+
if self.any_frame_decoded && self.decoder.is_at_block_boundary() {
234+
self.at_eof = true;
235+
break;
236+
}
229237
return Err(DecodingError::UnexpectedEof);
230238
}
231239

@@ -234,6 +242,10 @@ impl<R: Read> ReadDecoder<R> {
234242
self.reader.consume(consumed);
235243
match result {
236244
Decoded::Nothing => (),
245+
Decoded::DataEnd => {
246+
self.any_frame_decoded = true;
247+
return Ok(Some(Decoded::DataEnd));
248+
}
237249
Decoded::BlockStart(Block::Trailer) => {
238250
self.at_eof = true;
239251
}
@@ -315,6 +327,7 @@ where
315327
reader: io::BufReader::new(reader),
316328
decoder,
317329
at_eof: false,
330+
any_frame_decoded: false,
318331
},
319332
bg_color: None,
320333
pixel_converter: PixelConverter::new(options.color_output),

tests/missing_trailer.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
//! Tolerating GIFs missing the final trailer byte (`0x3B`).
2+
#![cfg(feature = "std")]
3+
4+
use gif::{ColorOutput, DecodeOptions, DecodingError, DisposalMethod, Encoder, Frame};
5+
6+
/// A valid, complete two-frame GIF (with the `0x3B` trailer).
7+
fn build_two_frame_gif() -> Vec<u8> {
8+
let mut data = Vec::new();
9+
{
10+
let mut encoder = Encoder::new(&mut data, 2, 2, &[0, 0, 0, 255, 255, 255]).unwrap();
11+
12+
let mut frame = Frame {
13+
delay: 1,
14+
dispose: DisposalMethod::Any,
15+
transparent: None,
16+
needs_user_input: false,
17+
top: 0,
18+
left: 0,
19+
width: 2,
20+
height: 2,
21+
interlaced: false,
22+
palette: None,
23+
buffer: vec![0, 1, 1, 0].into(),
24+
};
25+
encoder.write_frame(&frame).unwrap();
26+
27+
frame.buffer = vec![1, 0, 0, 1].into();
28+
encoder.write_frame(&frame).unwrap();
29+
}
30+
data
31+
}
32+
33+
fn decode_all(bytes: &[u8]) -> Result<Vec<Vec<u8>>, DecodingError> {
34+
let mut options = DecodeOptions::new();
35+
options.set_color_output(ColorOutput::Indexed);
36+
let mut decoder = options.read_info(bytes)?;
37+
38+
let mut frames = Vec::new();
39+
loop {
40+
match decoder.read_next_frame()? {
41+
Some(frame) => frames.push(frame.buffer.to_vec()),
42+
None => return Ok(frames),
43+
}
44+
}
45+
}
46+
47+
/// Index of the `n`th image separator (`0x2C`).
48+
fn nth_image_sep(gif: &[u8], n: usize) -> usize {
49+
gif.iter()
50+
.enumerate()
51+
.filter(|&(_, &b)| b == 0x2C)
52+
.nth(n)
53+
.map(|(i, _)| i)
54+
.expect("image separator not found")
55+
}
56+
57+
fn is_truncation(err: DecodingError) -> bool {
58+
matches!(err, DecodingError::UnexpectedEof | DecodingError::Truncated)
59+
}
60+
61+
#[test]
62+
fn missing_trailer_decodes_all_frames_cleanly() {
63+
let with_trailer = build_two_frame_gif();
64+
assert_eq!(*with_trailer.last().unwrap(), 0x3B);
65+
66+
let trailerless = &with_trailer[..with_trailer.len() - 1];
67+
let baseline = decode_all(&with_trailer).expect("with-trailer GIF must decode");
68+
let stripped = decode_all(trailerless).expect("trailerless GIF must decode cleanly");
69+
70+
assert_eq!(baseline.len(), 2);
71+
assert_eq!(stripped, baseline, "trailerless decode must match");
72+
}
73+
74+
#[test]
75+
fn truncation_mid_frame_still_errors() {
76+
let full = build_two_frame_gif();
77+
// A few bytes into the second frame's LZW data.
78+
let truncated = &full[..nth_image_sep(&full, 1) + 12];
79+
assert!(is_truncation(
80+
decode_all(truncated).expect_err("mid-frame truncation must error")
81+
));
82+
}
83+
84+
/// The review fix: once the next introducer byte is read the decoder is
85+
/// mid-block, so EOF is truncation — even though a frame was already decoded.
86+
#[test]
87+
fn introducer_then_eof_after_frame_still_errors() {
88+
let full = build_two_frame_gif();
89+
// Right after the second image separator: frame one complete, `0x2C` read,
90+
// its descriptor gone.
91+
let truncated = &full[..nth_image_sep(&full, 1) + 1];
92+
assert!(is_truncation(
93+
decode_all(truncated).expect_err("introducer-then-EOF must error")
94+
));
95+
}
96+
97+
#[test]
98+
fn zero_frames_then_eof_still_errors() {
99+
let full = build_two_frame_gif();
100+
let truncated = &full[..nth_image_sep(&full, 0) + 1];
101+
assert!(
102+
decode_all(truncated).is_err(),
103+
"zero-frame truncation must error"
104+
);
105+
}

0 commit comments

Comments
 (0)