Bug 75788 - Array.prototype.pop should throw if property is not configurable
Summary: Array.prototype.pop should throw if property is not configurable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-07 22:51 PST by Gavin Barraclough
Modified: 2012-07-02 12:26 PDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2012-01-07 22:51:22 PST
See ES5.1 15.4.4.6 step 5c.
Comment 1 metaweta 2012-06-25 11:47:12 PDT
This is far worse than merely not throwing, which is acceptable outside of strict mode.  The current behavior violates the frozenness invariant:

var a = [1,2,3];
Object.freeze(a);
a.pop(); // returns 3
a; // [1,2]
Object.isFrozen(a); // true

It's a security bug and should be escalated.
Comment 2 Gavin Barraclough 2012-06-29 18:47:20 PDT
I don't think this bug still exists in ToT:

var a = [1,2,3];
Object.freeze(a);
a.pop(); // Exception: TypeError: Attempted to assign to readonly property.
a; // 1,2,3
Object.isFrozen(a); // true

Technically I think we now implement the spec correctly, though we do give a misleading error message (which is outside of the spec).

We should be throwing a type error at the point pop() tries to delete the old property,
    15.4.4.6 step 5.c. -> 8.12.7 step 4
Instead we're silently ignoring the failed delete, but we'll throw a type error when pop tries to change the array's length.
    15.4.4.6 step 5.d. -> 15.4.5.1 step 3.g.

We will generate the right error message if the last property in the array is not configurable but length is writable:

var a = [1,2,3];
Object.defineProperty(a, 2, {configurable:false})
a.pop() // Exception: TypeError: Unable to delete property.

This works because the first attempted delete will (erroneously) silently fail, but when the length property is changed we'll attempt to remove all properties beyond the new array bounds, and this will throw as expected with the correct error message.
    15.4.4.6 step 5.d. -> 15.4.5.1 step 3.l.iii.4.

I'll put up a patch to make the first delete catch the error, as it should, so the TypeError has the right error message.
Comment 3 Gavin Barraclough 2012-07-02 12:26:15 PDT
Fixed in r121700