Fix empty sub-block terminator in extension parsing#228
Merged
197g merged 1 commit intoimage-rs:masterfrom Apr 9, 2026
Merged
Conversation
davidalecrim1
force-pushed
the
fix-empty-extension-sub-block
branch
2 times, most recently
from
5150206 to
f7304d3
Compare
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).
davidalecrim1
force-pushed
the
fix-empty-extension-sub-block
branch
from
f7304d3 to
f0f52cb
Compare
197g
approved these changes
Apr 9, 2026
Member
197g
left a comment
197g
left a comment
There was a problem hiding this comment.
That is indeed on weird edge case.
Contributor
Author
|
Thanks @197g ! Can you create a tag for this? |
Member
|
A crates.io release or just a tag? Will probably do both, just the tag is quicker. |
Contributor
Author
|
I would need both to open a PR for https://github.com/image-rs/image with the bump. After that one is released as well I should have all I need for Zed. |
Member
|
You don't need to open a PR against
|
Contributor
Author
|
Nice to know. I will keep that in mind. |
davidalecrim1
added a commit
to davidalecrim1/zed
that referenced
this pull request
Apr 9, 2026
The fix for GIFs with empty comment extensions was merged upstream in image-rs/image-gif#228 and released as gif 0.14.2. Bump the dependency so those GIFs now render correctly in the markdown preview.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A GIF with an empty comment extension (
21 fe 00) immediately before its image descriptor fails to decode. The decoder returns zero frames or errors withUnexpectedEof, despite the file being valid per the GIF89a spec.This was caught while investigating zed-industries/zed#53426, where several valid GIFs rendered fine in VS Code but failed silently in Zed.
Root cause
ExtensionDataSubBlockStart(sub_block_len)always transitions toExtensionDataSubBlock(sub_block_len)viagoto!(0, ...), which re-presents the current bytebwithout consuming it. Whensub_block_len == 0(a block terminator per GIF89a §23), the state machine falls intoExtensionDataSubBlock(0)and checksif b == 0against whatever byte comes next — which is the first byte of the next record (e.g.0x2c, the image descriptor intro). Since that byte is not zero, it's misinterpreted as a new sub-block of that many bytes, swallowing image data and causingUnexpectedEofor missing frames.Fix
Short-circuit in
ExtensionDataSubBlockStartwhensub_block_len == 0, going directly toExtensionBlockEndthe same wayExtensionDataSubBlockdoes when it sees the actual terminator byte.Tests
empty_comment_extension_decodes_one_frame: minimal hand-crafted GIF bytes with21 fe 00before the image descriptor — verifies the parser handles the spec-compliant caseempty_comment_extension_real_world_gif: real-world 128x128 GIF file that triggered the bug report