Bug 15141 - GIFImageReader can decode incorrectly at different packet boundaries
Summary: GIFImageReader can decode incorrectly at different packet boundaries
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-04 11:48 PDT by Peter Kasting
Modified: 2007-10-14 04:38 PDT (History)
0 users

See Also:


Attachments
patch v1 (1.65 KB, patch)
2007-09-04 11:56 PDT, Peter Kasting
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2007-09-04 11:48:12 PDT
WebCore/platform/image-decoders/gif/GIFImageReader.cpp (not used by Safari, but used by Cairo/QT) has bugs relating to how it was borrowed from the Mozilla codebase, which provides data to the decoder differently.

In the Mozilla world, the decoder is called with only the new data seen since the last call, so a hold buffer is used to store not-yet-decoded data that should be prepended to the next incoming chunk.  In the WebKit world, the decoder is provided the entire data stream up to the current point every time.  To fit these two together, GIFImageDecoder.cpp contains some wrapper code that lets the reader call back to say "I've decoded up to this point", and then the wrapper will call the reader with only the data from there the next time.

The problem is that this callback isn't actually used in a couple places where it should be, meaning that depending on where your packet boundaries are, GIFs fail to decode or decode incorrectly.

The true fix to this would be to rewrite the reader and its wrapper to eliminate the hold buffer entirely and be fully aware of the way data is going to be provided.  However, that's a slightly trickier task than what I propose to do in this bug, which is to at least insert the necessary callback calls so that the decoder always gets data from the right point.  Patch coming shortly.
Comment 1 Peter Kasting 2007-09-04 11:56:15 PDT
Created attachment 16198 [details]
patch v1

Contains the aforementioned fix, plus one other tiny change: when the GIF reader fails, return failure rather than success, so we stop decoding (as Mozilla does) instead of trying again and again.
Comment 2 Maciej Stachowiak 2007-09-29 18:11:55 PDT
Comment on attachment 16198 [details]
patch v1

r=me
Comment 3 Mark Rowe (bdash) 2007-10-14 04:38:02 PDT
Landed in r26580.