164049 - SVGTransform.prototype.setMatrix crashes on null or undefined argument

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jason Davis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020814
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020814

Calling setMatrix(null) and setMatrix(undefined) crashes the browser. Calling
setMatrix({}) and setMatrix([]) will disable rendering of any elements the
transform applies to. I believe it is because it interprets them as
matrix(0,0,0,0,0,0). Other literals, such as false, 7, and 'hello' all throw errors.

http://www.w3.org/TR/SVG11/coords.html#InterfaceSVGTransform does not claim
setMatrix throws an exception, but the IDL indicates it should only accept an
object of type SVGMatrix. Therefore, the literals should not throw an error, and
instead should do absolutely nothing. Same with the empty object and array
literals (which presently kill rendering).
Then again, throwing an error would make sense - would someone else with more
knowledge on the subject please comment?

Reproducible: Always

Steps to Reproduce:
1. View the URL

Actual Results:  
Mozilla crashed.

Expected Results:  
Do absolutely nothing as per the specs, or throw an error.

Comment 1

[Steve]

Hi

Using:Mozilla 1.1b
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020814

I confirm this behavior.

Comment 2

[Steve]

Well I'll try again, in that I believe that I've got confirm rights.

Perhaps the new Bugzilla has a bug ;-)

Using:
Mozilla 1.1b
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020814

I confirm this behavior.

Comment 3

[greer]

Steve, it would appear that you do indeed have confirm rights. Do you get at
Talkback incidentID for your efforts? Please list that here. Thanks.

Comment 4

[Steve]

Hello

No, a Talkback incidentID is not possible in that these builds are
'experimental' zips with no Talkback features.

Comment 5

[Stefan Moebius]

The URL is gone. Jason, do you still see this problem? If so, can you repost
your test case?

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: testcase - click button to crash

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: improved testcase

Lets you set and test different arguments.

Comment 8

[Jonathan Watt [:jwatt]]

Apparently the JavaScript values null and undefined get passed to the C++
function as nsnull, so a simple NS_ENSURE_ARG(matrix) will fix the crash, but I
will try and implement sensible handling of JS objects that don't implement
QueryInterface before creating a patch. 

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: crash patch

On second thoughts here's the patch to solve the crash. The problems
encountered when JS objects are passed to setMatrix extend far further than
just this function. Sanity checking of parameters would be better addressed as
a separate issue. Roughly speaking the plan will be to ensure that only objects
that are instances of *our* implementations of SVG interfaces may be passed to
such functions (since XPConnect makes it impossible for the C++ code to ensure
JS objects fully implement such interfaces and the interfaces they inherit).

Comment 10

[Alex Fritze]

Checked in.