Closed
Bug 863958
Opened 9 years ago
Closed 9 years ago
ICO decoder assumes you can never have a write after you get the size [@mozilla::image::ImageMetadata::SetSize]
Categories
(Core :: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: posidron, Assigned: joe)
References
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(4 files)
17.67 KB,
image/x-icon
|
Details | |
4.43 KB,
text/plain
|
Details | |
833 bytes,
patch
|
seth
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
4.60 KB,
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
Tested with m-i changeset: 129367:9ff5c0134bbf
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Crash Signature: [@ mozilla::image::ImageMetadata::SetSize] → [@ void mozilla::Maybe<nsIntSize>::construct<nsIntSize>]
[@ mozilla::image::ImageMetadata::SetSize]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → joe
Assignee | ||
Updated•9 years ago
|
Summary: ICO crash [@mozilla::image::ImageMetadata::SetSize] → ICO decoder assumes you can never have a write after you get the size [@mozilla::image::ImageMetadata::SetSize]
Assignee | ||
Comment 2•9 years ago
|
||
In this case the ICO decoder's logic was just plain wrong; it assumed it could call SetSize as willy-nilly as it liked, which isn't the case. If we happened to have a sufficiently-large ICO, we'd get this assertion failure. https://tbpl.mozilla.org/?tree=Try&rev=9663d8686bc0
Attachment #740386 -
Flags: review?(seth)
Assignee | ||
Comment 3•9 years ago
|
||
This is Christoph's testcase.
Attachment #740387 -
Flags: review?(seth)
Assignee | ||
Comment 4•9 years ago
|
||
Another try at try, since we had infra problems that gave us lots of false timeouts: https://tbpl.mozilla.org/?tree=Try&rev=d3f8ea4f8696
Comment 5•9 years ago
|
||
Comment on attachment 740386 [details] [diff] [review] Don't call SetSize if we already have the size Review of attachment 740386 [details] [diff] [review]: ----------------------------------------------------------------- Excellent.
Attachment #740386 -
Flags: review?(seth) → review+
Comment 6•9 years ago
|
||
Ironically this patch has almost identical code to the patch for bug 858818, which didn't get checked in because I was having trouble producing a working test case. I'm glad that we've got one in this bug. I'll dupe that bug to this one.
Comment 8•9 years ago
|
||
Comment on attachment 740387 [details] [diff] [review] crashtest Review of attachment 740387 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. I'm a bit puzzled as to why I couldn't reproduce this in the .png I generated for the other bug. I had a much bigger one (more than 10 times the size!) but it still didn't work. Strange. At any rate, it's great that we have a test case for this now!
Attachment #740387 -
Flags: review?(seth) → review+
Assignee | ||
Comment 9•9 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/8658d3607e1a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/506e1fb34b30
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8658d3607e1a https://hg.mozilla.org/mozilla-central/rev/506e1fb34b30
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 740386 [details] [diff] [review] Don't call SetSize if we already have the size [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 716140 User impact if declined: Assertion failures in debug builds Testing completed (on m-c, etc.): On m-c for a couple of days Risk to taking this patch (and alternatives if risky): Not risky. String or IDL/UUID changes made by this patch: none
Attachment #740386 -
Flags: approval-mozilla-aurora?
Comment 12•9 years ago
|
||
Comment on attachment 740386 [details] [diff] [review] Don't call SetSize if we already have the size In support of developers, approving for uplift to Aurora.
Attachment #740386 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/9b664bf6ac2a
status-firefox22:
--- → fixed
status-firefox23:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•