Fails to load a gif with corrupted dimensions

VERIFIED FIXED in Firefox 36

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Fanolian+bugzilla, Assigned: seth)

Tracking

(Blocks 1 bug, {regression})

36 Branch
mozilla38
x86_64
Windows 8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 unaffected, firefox36+ verified, firefox37+ verified, firefox38+ verified, firefox-esr31 unaffected, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(2 attachments)

Sorry for the vague title...

Steps to reproduce:
1. Load the attached gif.

Actual result:
1. The gif should load.

From mozregression:
Last good revision: cef590a6f946
First bad revision: 17de0f463944
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cef590a6f946&tochange=17de0f463944

IE and Chrome can load the gif properly.

p.s. Source of the gif: https://imgur.com/Y286dyS
Flags: needinfo?(seth)
Version: 35 Branch → 36 Branch
Duplicate of this bug: 1128247
Tracking because, well, gifs are pretty popular these days. :)

Seth - As this is a regression from bug 1060869, can you take this?
Assignee: nobody → seth
Hmm. So this GIF is corrupt. It reports its size as 398x233, but then its first frame is at offset (2,0) with size 398x233, which doesn't fit.

Viewing the GIF in Chrome, I can see a two-pixel column on the left of the image where nothing is drawn. So what Chrome has done is decode the frames with the bad offset intact, and discard the 399th and 400th columns of pixels.

That's precisely what we will do if we permit these malformed frames to be created, because when they get drawn into the compositing frame to be animated the extra data on the right side will get cut off. So we'll get the exact same behavior as Chrome here if we just ignore this error.
Here's the patch. I've confirmed that it fixes this issue.
Attachment #8558419 - Flags: review?(tnikkel)
Here's a try job:

https://tbpl.mozilla.org/?tree=Try&rev=116dbf2efc71
Flags: needinfo?(seth)
Comment on attachment 8558419 [details] [diff] [review]
Allow creation of animated frames that do not fit inside the bounds of their image

Switching this one over to Jeff.
Attachment #8558419 - Flags: review?(tnikkel) → review?(jmuizelaar)
Comment on attachment 8558419 [details] [diff] [review]
Allow creation of animated frames that do not fit inside the bounds of their image

Josh, can you review this? Jeff is traveling and we need this on beta ASAP.
Attachment #8558419 - Flags: review?(jmuizelaar) → review?(josh)
(In reply to Seth Fowler [:seth] from comment #8)
> Comment on attachment 8558419 [details] [diff] [review]
> Allow creation of animated frames that do not fit inside the bounds of their
> image
> 
> Josh, can you review this? Jeff is traveling and we need this on beta ASAP.

Can you explain why/how bug 1060869 regressed this? Also a reftest would be good.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> Can you explain why/how bug 1060869 regressed this?

I added the AllowedImageAndFrameDimensions function as part of the refactoring in bug 1060869 part 2, and I included this check that was not included in the pre-refactoring code. So this is just returning us to the pre-refactoring state in that respect.

> Also a reftest would be good.

Agreed! I will file a followup to add one. Unfortunately I don't have time to create one right now.
Blocks: 1130704
(In reply to Seth Fowler [:seth] from comment #10)
> I will file a followup to add one.

Filed bug 1130704.
Comment on attachment 8558419 [details] [diff] [review]
Allow creation of animated frames that do not fit inside the bounds of their image

Review of attachment 8558419 [details] [diff] [review]:
-----------------------------------------------------------------

Seems reasonable.
Attachment #8558419 - Flags: review?(josh) → review+
Thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/5f618e1b85c5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8558419 [details] [diff] [review]
Allow creation of animated frames that do not fit inside the bounds of their image

Approval Request Comment
[Feature/regressing bug #]: 1060869
[User impact if declined]: Certain corrupt GIFs will not load. The same GIFs load in Blink and WebKit.
[Describe test coverage new/current, TreeHerder]: On m-c.
[Risks and why]: Low risk; returns us to the state before bug 1060869 with regards to error handling for this type of corrupt GIF.
[String/UUID change made/needed]: None.
Attachment #8558419 - Flags: approval-mozilla-beta?
Comment on attachment 8558419 [details] [diff] [review]
Allow creation of animated frames that do not fit inside the bounds of their image

[Triage Comment]
Since it landed in 38 and the uplift is for 36. I guess we want it for aurora (37) too.
Attachment #8558419 - Flags: approval-mozilla-beta?
Attachment #8558419 - Flags: approval-mozilla-beta+
Attachment #8558419 - Flags: approval-mozilla-aurora+
Summary: Cannot load a gif → Fails to load a gif with corrupted dimensions
QA Whiteboard: [good first verify]
Verified as fixed using the following environment:

FF 36.0b10
Build id: 20150212154903

FF 37
Build Id: 20150212004049

FF 38
Build Id: 20150218030228

OS: Win 7 x64, Mac Os X 10.10, Ubuntu 14.03 x86
You need to log in before you can comment on or make changes to this bug.