399286 - port fix for mac large image crasher to 1.8 branch

Status: VERIFIED FIXED

(Core :: Graphics, defect, P1)

Description

[John Daggett (:jtd)]

Images with heights larger than 32K cause a crash within Quartz drawing code on the Mac.  The bug has been reported to Apple but in the meantime we need to restrict our code to avoid loading images over 32K in height.  See bug 328258 for testcase and details.

Comment 1

[Daniel Veditz [:dveditz]]

Ok, here's your blocking, where's our patch? :-)

Comment 2

[John Daggett (:jtd)]

I'll investigate what's needed for a patch by the end of this week

Comment 3

[Samuel Sidler (old account; do not CC)]

John, any update here?

Comment 4

[Samuel Sidler (old account; do not CC)]

No work done here, pushing out to the next release.

Comment 5

[John Daggett (:jtd)]

Steps to reproduce:

1. Download testcase-2 from bug xxx:

https://bugzilla.mozilla.org/attachment.cgi?id=213053

2. Open the file bug-328258.html

Result: crash in nsImageMac::Draw

Comment 6

[John Daggett (:jtd)]

Attachment: patch, simplified version of the fix for 328258

Wrap all calls to CGContextDrawImage with a width/height check.  Nothing fancy but it will prevent the crash situation.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 302990 [details] [diff] [review]
patch, simplified version of the fix for 328258

Looks good to me!

Comment 8

[John Daggett (:jtd)]

checked in

Comment 9

[Samuel Sidler (old account; do not CC)]

(In reply to comment #8)
> checked in

Err... as stated on the tinderbox tree, "Bugs must have approval1.8.1.13+ from the 1.8.1.13 triage team".

Comment 10

[Samuel Sidler (old account; do not CC)]

Comment on attachment 302990 [details] [diff] [review]
patch, simplified version of the fix for 328258

Retroactively requesting approval for this patch. It's already been checked in.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/gfx/src/mac&command=DIFF_FRAMESET&file=nsImageMac.cpp&rev1=1.79.4.3&rev2=1.79.4.4&root=/cvsroot

Comment 11

[John Daggett (:jtd)]

(In reply to comment #10)
> Retroactively requesting approval for this patch. It's already been checked in.

argh, sorry about that! had an itchy checkin finger...

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 302990 [details] [diff] [review]
patch, simplified version of the fix for 328258

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 13

[Jeff Walden [:Waldo]]

Could we get an automated testcase for this and land it after the next 1.8 branch release, please?

Comment 14

[John Daggett (:jtd)]

Received a mail from Apple stating the underlying bug should be fixed in the next 10.5 release, no mention of a 10.4 fix:

Hi John,

This is a courtesy email regarding Bug ID# 5514949.  We believe this issue has been addressed in Mac OS X Client 10.5.2, build 9C31.  Please verify with this release, and update this report with your results.

Mac OS X Client 10.5.2, build 9C31:
http://www.apple.com/support/downloads/macosx1052comboupdate.html

Bug reports requiring your update will appear under ‘My Originated Problems’.  Please review this bug report and provide the requested information via the Apple Bug Reporter. Once your report has been updated, Engineering will be alerted of the new information.

<http://bugreport.apple.com>

Thank you for your assistance in helping us discover and isolate bugs within our products. 

Best Regards,

Stoney Gamble
Apple Developer Connection 
Worldwide Developer Relations

Comment 15

[John Daggett (:jtd)]

Confirmed fixed in Mac OS X Client 10.5.2, build 9C31.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Verified fixed in 10.4.11 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.13) Gecko/2008031115 Firefox/2.0.0.13. I repro'd the crash with 2.0.0.12 on the same machine.

Comment 17

[Alexander Sack]

Comment on attachment 302990 [details] [diff] [review]
patch, simplified version of the fix for 328258

applies unmodified on 1.8.0