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

VERIFIED FIXED in mozilla1.8alpha4

Status

()

Core
JavaScript Engine
P1
critical
VERIFIED FIXED
13 years ago
11 years ago

People

(Reporter: Troy Farrell, Assigned: brendan)

Tracking

(4 keywords)

Trunk
mozilla1.8alpha4
crash, fixed-aviary1.0, fixed1.7.5, js1.5
Points:
---
Bug Flags:
blocking1.7.5 +
blocking-aviary1.0 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Must-fix, thanks for reporting this.

/be
Assignee: general → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.7.x+
Flags: blocking-aviary1.0+
Keywords: crash, js1.5
OS: Windows 2000 → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8alpha4
(Assignee)

Comment 2

13 years ago
Created attachment 159485 [details] [diff] [review]
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
(Assignee)

Updated

13 years ago
Attachment #159485 - Flags: review?(shaver)
Comment on attachment 159485 [details] [diff] [review]
proposed fix

Cool, but you gotta add a test!
Attachment #159485 - Flags: review?(shaver) → review+
(Assignee)

Comment 4

13 years ago
Comment on attachment 159485 [details] [diff] [review]
proposed fix

Fixed on trunk; self-approving for branches.

/be
Attachment #159485 - Flags: approval1.7.x+
Attachment #159485 - Flags: approval-aviary+
(Assignee)

Comment 5

13 years ago
Fixed all over the place.

/be
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed-aviary1.0, fixed1.7.x
Resolution: --- → FIXED

Updated

13 years ago
Severity: minor → critical

Comment 6

13 years ago
Created attachment 174899 [details]
js1_5/Regress/regress-260541.js

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

Comment 7

13 years ago
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

13 years ago
js1_5/Regress/regress-260541.js checked in.

Updated

13 years ago
Flags: testcase+

Comment 9

11 years ago
verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.