260541 - Referencing an Error object on its .name attribute causes a crash

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Troy Farrell]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10

Browser go boom due to poor coding ( self-referencing objects when the JS engine
probably expected a string...)

Reproducible: Always
Steps to Reproduce:
1. Enable Javascript

2. Load a page with code such as
var myErr = new Error( "Error Text" );
myErr.name = myErr;

3. Watch the Mozilla Talkback client do it's thing.
Actual Results:  
The browser crashed.

Expected Results:  
The browser should have laughed at my poor coding ability and moved along.

TB896851K is the most specific.
TB895978Q and TB895966E were sent before I tracked down the problem.

Comment 1

[Brendan Eich [:brendan]]

Must-fix, thanks for reporting this.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: proposed fix

Old jsexn.c code (not mine) was too trusting about the values of read/write
Error properties such as name and message, leading to stack overflow.

Alas, ECMA-262 Edition 3 leaves Error.prototype.toString completely up to the
implementation to define.  This patch avoids recursive divergence and useless
results, but may result in an exception object converting to the empty string,
if you set both name and message to non-string or empty-string values.	If
anyone has a better idea for how Error.prototype.toString should work, feel
free to file an RFE.

/be

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 159485 [details] [diff] [review]
proposed fix

Cool, but you gotta add a test!

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 159485 [details] [diff] [review]
proposed fix

Fixed on trunk; self-approving for branches.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed all over the place.

/be

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-260541.js

Troy, with your permission this will be included in the javascript test
library.

Comment 7

[Troy Farrell]

Heh.  It's funny to hear you ask for permission for two lines of code.  Need a 
license?  Do whate'er you want with the code, just don't sue me ;-)  I guess 
that would make it an MIT license.

BTW, who is QA?  I guess per the workflow they are supposed to "Mark bug as 
Verified."  I'm new to bugzilla.

Comment 8

[Bob Clary [:bc] (inactive)]

js1_5/Regress/regress-260541.js checked in.

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed.