Bug 55033 - Array.prototype.push should throw RangeError if the length overflows
Summary: Array.prototype.push should throw RangeError if the length overflows
Status: RESOLVED DUPLICATE of bug 62405
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-23 03:50 PST by Kent Hansen
Modified: 2012-03-12 14:33 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Hansen 2011-02-23 03:50:28 PST
The following script

a = new Array(4294967295); a.push('foo')

doesn't cause an error with JavaScriptCore. Instead, the push seemingly succeeds, but you end up with an array of length 0.
Both V8 and SpiderMonkey throw a RangeError in this case. That's correct as per ECMA-262 5th edition; it happens due to step 6 in section 15.4.4.7. In particular, see section 15.4.5.1, steps 3c and 3d.
In summary: push() should create a property with name "4294967295" (not an array index), but the array length should not be changed, and a RangeError should be thrown.
Comment 1 Gavin Barraclough 2011-03-09 01:57:55 PST
I don't think our behavior i correct here, and I do agree that other browsers are following the spec, but I don't think the spec is particularly sane either.

* Allowing push to write a property but then also throw seems wrong.

* Allowing push to write non-array properties at all (but only one!) on an array object seems wrong.

I seems if would be nicer if the spec were written such that the range check falls before the property write.

Hmmmm.  I guess it may be best to fix this to comply with the spec as it currently stands, but we might want to also propose a fix for this.
Comment 2 Kent Hansen 2011-03-09 03:18:54 PST
Hey Gavin. I reported this possible spec issue to es-discuss@ a long time ago, but there wasn't any tangible outcome: http://www.mail-archive.com/es-discuss@mozilla.org/msg01264.html

Grepping through the spec, it seems at least Array.prototype.splice has the same issue. E.g.

a = new Array(4294967295); a.splice(a.length, 0, 'foo'); a.length

JSC: 0
SpiderMonkey: 0 (interesting -- since Array.prototype.push throws)
V8: RangeError: Invalid array length (as per spec)

And Array.prototype.unshift as well.
Comment 3 Ariya Hidayat 2011-11-03 11:22:23 PDT
AFAICS this is also addressed in bug 62405 (fixed in http://trac.webkit.org/changeset/88503).
Comment 4 Gavin Barraclough 2012-03-12 14:33:40 PDT
Ah, yep, we've fixed this now, that looks like the right revision.  Cheers.

*** This bug has been marked as a duplicate of bug 62405 ***