Skip to content

Commit f0f52cb

Browse files
committed
Fix empty sub-block terminator in extension parsing
A zero-length sub-block (size byte = 0x00) is a block terminator per the GIF89a spec (§23). When ExtensionDataSubBlockStart(0) transitioned to ExtensionDataSubBlock(0) via goto!(0, ...), the current byte `b` was re-presented rather than consumed. ExtensionDataSubBlock(0) then checked `if b == 0` against the *next* record byte (e.g. 0x2c, the image descriptor intro), misinterpreting it as a new sub-block of that many bytes. This swallowed image data and resulted in UnexpectedEof or zero frames being decoded. Fix by short-circuiting directly to ExtensionBlockEnd when sub_block_len is 0, matching the existing block-terminator path in ExtensionDataSubBlock. Adds two regression tests and the real-world GIF file that triggered the bug report (a 128x128 single-frame GIF with an empty comment extension immediately before its image descriptor).
1 parent c1a6c4f commit f0f52cb

4 files changed

Lines changed: 84 additions & 1 deletion

File tree

src/reader/decoder.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,18 @@ impl StreamingDecoder {
732732
}
733733
ExtensionDataSubBlockStart(sub_block_len) => {
734734
self.ext.data.clear();
735-
goto!(0, ExtensionDataSubBlock(sub_block_len))
735+
if sub_block_len == 0 {
736+
// A zero-length sub-block is a block terminator (GIF89a spec §23).
737+
// Short-circuit here rather than entering ExtensionDataSubBlock(0),
738+
// which would re-present the next record byte and misparse it as a
739+
// new sub-block length.
740+
if self.ext.id.into_known() == Some(Extension::Control) {
741+
self.read_control_extension()?;
742+
}
743+
goto!(0, ExtensionBlockEnd, emit Decoded::SubBlock { ext: self.ext.id, is_last: true })
744+
} else {
745+
goto!(0, ExtensionDataSubBlock(sub_block_len))
746+
}
736747
}
737748
ExtensionDataSubBlock(left) => {
738749
if left > 0 {

tests/decode.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,74 @@ fn check_last_extension_returns() {
328328

329329
assert_eq!(xmp_len, EXPECTED_METADATA.len() + 257);
330330
}
331+
332+
// A GIF with an empty comment extension (`21 fe 00`) immediately before the image
333+
// descriptor should decode to exactly one frame.
334+
#[test]
335+
fn empty_comment_extension_decodes_one_frame() {
336+
// Minimal hand-crafted GIF: 1x1 pixel, global color table (2 colors),
337+
// empty comment extension (21 fe 00), then a single image frame.
338+
// The same bytes without the comment extension decode correctly (verified).
339+
#[rustfmt::skip]
340+
let gif_bytes: &[u8] = &[
341+
// Header
342+
b'G', b'I', b'F', b'8', b'9', b'a',
343+
// Logical Screen Descriptor: 1x1, GCT flag set, GCT size=0 (2 colors)
344+
0x01, 0x00, // width=1
345+
0x01, 0x00, // height=1
346+
0x80, // packed: GCT flag=1, color resolution=0, sort=0, GCT size=0
347+
0x00, // background color index
348+
0x00, // pixel aspect ratio
349+
// Global Color Table: 2 entries (black, white)
350+
0x00, 0x00, 0x00,
351+
0xFF, 0xFF, 0xFF,
352+
// Empty comment extension: intro, label, immediate block terminator
353+
0x21, 0xFE, 0x00,
354+
// Image Descriptor
355+
0x2C,
356+
0x00, 0x00, // left=0
357+
0x00, 0x00, // top=0
358+
0x01, 0x00, // width=1
359+
0x01, 0x00, // height=1
360+
0x00, // packed: no LCT
361+
// LZW minimum code size=2, one sub-block (2 bytes), block terminator
362+
0x02, 0x02, 0x4C, 0x01, 0x00,
363+
// Trailer
364+
0x3B,
365+
];
366+
367+
let mut decoder = DecodeOptions::new()
368+
.read_info(gif_bytes)
369+
.expect("decoder should initialize");
370+
371+
let frame = decoder
372+
.read_next_frame()
373+
.expect("read_next_frame should not error")
374+
.expect("should return a frame, not None");
375+
376+
assert_eq!(frame.width, 1);
377+
assert_eq!(frame.height, 1);
378+
379+
assert!(
380+
decoder.read_next_frame().unwrap().is_none(),
381+
"should have exactly one frame"
382+
);
383+
}
384+
385+
// Same as above, using a real-world GIF with an empty comment extension to ensure
386+
// it is parsed and decoded as a valid single-frame image.
387+
#[test]
388+
fn empty_comment_extension_real_world_gif() {
389+
let path = "tests/samples/bufo-thinks-about-fishsticks.gif";
390+
let mut decoder = DecodeOptions::new()
391+
.read_info(File::open(path).unwrap())
392+
.expect("decoder should initialize");
393+
394+
let frame = decoder
395+
.read_next_frame()
396+
.expect("read_next_frame should not error")
397+
.expect("bufo-thinks-about-fishsticks.gif should decode to at least one frame");
398+
399+
assert_eq!(frame.width, 128);
400+
assert_eq!(frame.height, 128);
401+
}

tests/results.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ tests/samples/2x2.gif: 3802149240
1010
tests/samples/sample_big.gif: 4184562096
1111
tests/samples/gifplayer-muybridge.gif: 4267078865
1212
tests/samples/set_hsts.gif: 224161812
13+
tests/samples/bufo-thinks-about-fishsticks.gif: 1018609591
6.17 KB
Loading

0 commit comments

Comments
 (0)