Bug 15971 – Public GIF image decoder erroneously fails if no new data has come in

• Component: Images
• Status: RESOLVED FIXED

[Peter Kasting]

GIFImageReader.cpp (not used by Safari, but used by the Cairo/Qt ports) erroneously marks GIF decoding as having failed if we ask it to decode when no new data has come in since the last request to decode.  Instead, this should just result in doing nothing.

This looks like an artifact of the process of porting from the Mozilla codebase.  I think in Mozilla's codebase, this function is never called unless new data has come in, whereas in WebKit, the wrapper code does not guarantee that.  Therefore, I could fix this by putting a check in the wrapper that doesn't try to decode when there isn't new data.  But I think making the underlying decoder fail in this case is wrong, since there's nothing dangerous about such a call (in fact, if the check in the decoder is removed entirely, we'd run all the way through the function without doing anything, and return successfully).  So instead I'm planning to fix this by just changing the "return false" to "return true" in the beginning of GIFImageReader.cpp:read().

Patch coming in a bit.

[Peter Kasting]

Created attachment 17240 [details]
patch v1

Simple fix.

[Alp Toker]

I'm curious, how did you notice this? Is there a test case for this issue? A link to some online content will do..

We are getting crashes and corruption with the GIF image loader in the GTK+ port, particularly with animated GIFs, but I don't notice this patch fixing that problem.

[Peter Kasting]

I was able to repro this pretty consistently with http://transit.511.org/providers/maps/SF_7122007102947.gif (a 3201x2840 GIF).

[Darin Adler]

Comment on attachment 17240 [details]
patch v1

Looks fine, r=me.

[Mark Rowe (bdash)]

Landed in r27897.