RESOLVED DUPLICATE of bug 62405 55033
Array.prototype.push should throw RangeError if the length overflows 
https://bugs.webkit.org/show_bug.cgi?id=55033
Summary Array.prototype.push should throw RangeError if the length overflows
Kent Hansen
Reported 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.
Attachments
Add attachment proposed patch, testcase, etc.
Gavin Barraclough
Comment 1 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.
Kent Hansen
Comment 2 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.
Ariya Hidayat
Comment 3 2011-11-03 11:22:23 PDT
AFAICS this is also addressed in bug 62405 (fixed in http://trac.webkit.org/changeset/88503).
Gavin Barraclough
Comment 4 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 ***
Note You need to log in before you can comment on or make changes to this bug.