Bug 55033
| Summary: | Array.prototype.push should throw RangeError if the length overflows | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Kent Hansen <kent.hansen> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | ariya.hidayat, barraclough, cmarcelo |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
Kent Hansen
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
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
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
AFAICS this is also addressed in bug 62405 (fixed in http://trac.webkit.org/changeset/88503).
Gavin Barraclough
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 ***