75788 2012-01-07 22:51:22 -0800 Array.prototype.pop should throw if property is not configurable 2012-07-02 12:26:15 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 barraclough webkit-unassigned metaweta oldest_to_newest 531718 0 barraclough 2012-01-07 22:51:22 -0800 See ES5.1 15.4.4.6 step 5c. 656761 1 metaweta 2012-06-25 11:47:12 -0700 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. 660926 2 barraclough 2012-06-29 18:47:20 -0700 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. 661669 3 barraclough 2012-07-02 12:26:15 -0700 Fixed in r121700