Fixes issue 712 causing non-configurable accessors to be overwritable by usin...

test/mjsunit/object-define-property.js

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 35 matching lines @@
 }
 
 // Check that an exception is thrown when non-object is passed as object.
 try {
   Object.defineProperty(0, "foo", undefined);
   assertTrue(false);
 } catch (e) {
   assertTrue(/called on non-object/.test(e));
 }
 
-// Object
+// Object.
 var obj1 = {};
 
-// Values
+// Values.
 var val1 = 0;
 var val2 = 0;
 var val3 = 0;
 
-// Descriptors
+function setter1() {val1++; }
+function getter1() {return val1; }
+
+function setter2() {val2++; }
+function getter2() {return val2; }
+
+function setter3() {val3++; }
+function getter3() {return val3; }
+
+
+// Descriptors.
 var emptyDesc = {};
 
 var accessorConfigurable = { 
-    set: function() { val1++; },
-    get: function() { return val1; },
+    set: setter1,
+    get: getter1,
     configurable: true
 };
 
 var accessorNoConfigurable = {
-    set: function() { val2++; },
-    get: function() { return val2; },
+    set: setter2,
+    get: getter2,
     configurable: false 
 };
 
 var accessorOnlySet = {
-  set: function() { val3++; },
+  set: setter3,
   configurable: true
 };
 
 var accessorOnlyGet = {
-  get: function() { return val3; },
+  get: getter3,
   configurable: true
 };
 
 var accessorDefault = {set: function(){} };
 
 var dataConfigurable = { value: 1000, configurable: true };
 
 var dataNoConfigurable = { value: 2000, configurable: false };
 
 var dataWritable = { value: 3000, writable: true};
@@ skipping 97 matching lines @@
 assertEquals(desc.set, accessorNoConfigurable.set);
 assertEquals(desc.value, undefined);
 assertEquals(1, obj1.bar = 1);
 assertEquals(2, val1);
 assertEquals(3, val2);
 assertEquals(1, obj1.bar = 1)
 assertEquals(2, val1);
 assertEquals(4, val2);
 assertEquals(4, obj1.bar);
 
-// Define an accessor that has only a setter
+// Define an accessor that has only a setter.
 Object.defineProperty(obj1, "setOnly", accessorOnlySet);
 desc = Object.getOwnPropertyDescriptor(obj1, "setOnly");
 assertTrue(desc.configurable);
 assertFalse(desc.enumerable);
 assertEquals(desc.set, accessorOnlySet.set);
 assertEquals(desc.writable, undefined);
 assertEquals(desc.value, undefined);
 assertEquals(desc.get, undefined);
 assertEquals(1, obj1.setOnly = 1);
 assertEquals(1, val3);
 
-// Add a getter - should not touch the setter
+// Add a getter - should not touch the setter.
 Object.defineProperty(obj1, "setOnly", accessorOnlyGet);
 desc = Object.getOwnPropertyDescriptor(obj1, "setOnly");
 assertTrue(desc.configurable);
 assertFalse(desc.enumerable);
 assertEquals(desc.get, accessorOnlyGet.get);
 assertEquals(desc.set, accessorOnlySet.set);
 assertEquals(desc.writable, undefined);
 assertEquals(desc.value, undefined);
 assertEquals(1, obj1.setOnly = 1);
 assertEquals(2, val3);
@@ skipping 23 matching lines @@
 assertTrue(desc.configurable);
 assertFalse(desc.writable);
 assertFalse(desc.enumerable);
 assertEquals(desc.get, undefined);
 assertEquals(desc.set, undefined);
 //Try writing to non writable attribute - should remain 1000
 obj1.foobar = 1001;
 assertEquals(obj1.foobar, 1000);
 
 
-// Redefine to writable descriptor - now writing to foobar should be allowed
+// Redefine to writable descriptor - now writing to foobar should be allowed.
 Object.defineProperty(obj1, "foobar", dataWritable);
 desc = Object.getOwnPropertyDescriptor(obj1, "foobar");
 assertEquals(obj1.foobar, 3000);
 assertEquals(desc.value, 3000);
 // Note that since dataWritable does not define configurable the configurable
 // setting from the redefined property (in this case true) is used.
 assertTrue(desc.configurable);
 assertTrue(desc.writable);
 assertFalse(desc.enumerable);
 assertEquals(desc.get, undefined);
@@ skipping 98 matching lines @@
 assertEquals(obj2.bar, 1000);
 assertEquals(desc.value, 1000);
 assertTrue(desc.configurable);
 assertFalse(desc.writable);
 assertFalse(desc.enumerable);
 assertEquals(desc.get, undefined);
 assertEquals(desc.set, undefined);
 
 
 // Redefinition of an accessor defined using __defineGetter__ and 
-// __defineSetter__
+// __defineSetter__.
 function get(){return this.x}
 function set(x){this.x=x};
 
 var obj3 = {x:1000};
 obj3.__defineGetter__("foo", get);
 obj3.__defineSetter__("foo", set);
 
 desc = Object.getOwnPropertyDescriptor(obj3, "foo");
 assertTrue(desc.configurable);
 assertTrue(desc.enumerable);
@@ skipping 46 matching lines @@
 assertTrue(desc.configurable);
 assertTrue(desc.enumerable);
 assertEquals(desc.writable, undefined);
 assertEquals(desc.get, accessorConfigurable.get);
 assertEquals(desc.set, accessorConfigurable.set);
 assertEquals(desc.value, undefined);
 assertEquals(1, obj4.bar = 1);
 assertEquals(5, val1);
 assertEquals(5, obj4.bar);
 
-// Make sure an error is thrown when trying to access to redefined function
+// Make sure an error is thrown when trying to access to redefined function.
 try {
   obj4.bar();
   assertTrue(false);
 } catch (e) {
   assertTrue(/is not a function/.test(e));
 }
 
 
 // Test runtime calls to DefineOrRedefineDataProperty and
 // DefineOrRedefineAccessorProperty - make sure we don't 
-// crash
+// crash.
 try {
   %DefineOrRedefineAccessorProperty(0, 0, 0, 0, 0);
 } catch (e) {
   assertTrue(/illegal access/.test(e));
 }
 
 try {
   %DefineOrRedefineDataProperty(0, 0, 0, 0);
 } catch (e) {
   assertTrue(/illegal access/.test(e));
@@ skipping 23 matching lines @@
   %DefineOrRedefineAccessorProperty(null, 'foo', 0, func, 0);
 } catch (e) {
   assertTrue(/illegal access/.test(e));
 }
 
 try {
   %DefineOrRedefineDataProperty(null, 'foo', 0, 0);
 } catch (e) {
   assertTrue(/illegal access/.test(e));
 }
+
+// Test that all possible differences in step 6 in DefineOwnProperty is

[Erik Corry, 2010/05/21 13:21:57]

differences ... is -> differences ... are

[Rico, 2010/05/25 06:24:50]

Done.

+// exercised, i.e., any difference in the given property descriptor and the
+// existing properties should not return true, but throw an error if the
+// existing configurable property is false. 
+
+var obj5 = {};
+// Enumerable will default to false.
+Object.defineProperty(obj5, 'foo', accessorNoConfigurable);
+desc = Object.getOwnPropertyDescriptor(obj5, 'foo');
+// First, test that we are actually allowed to set the accessor if all
+// values are of the descriptor are the same as the existing one.
+Object.defineProperty(obj5, 'foo', accessorNoConfigurable);
+
+// Different setter.
+var descDifferent = {
+  configurable:false,
+  enumerable:false,
+  set: setter1,
+  get: getter2
+};
+
+try {
+  Object.defineProperty(obj5, 'foo', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// Different getter.
+descDifferent = {
+  configurable:false,
+  enumerable:false,
+  set: setter2,
+  get: getter1
+};
+
+try {
+  Object.defineProperty(obj5, 'foo', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// Different enumerable.
+descDifferent = {
+  configurable:false,
+  enumerable:true,
+  set: setter2,
+  get: getter2
+};
+
+try {
+  Object.defineProperty(obj5, 'foo', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// Different configurable.
+descDifferent = {
+  configurable:false,
+  enumerable:true,
+  set: setter2,
+  get: getter2
+};
+
+try {
+  Object.defineProperty(obj5, 'foo', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// No difference.
+descDifferent = {
+  configurable:false,
+  enumerable:false,
+  set: setter2,
+  get: getter2
+};
+// Make sure we can still redefine if all properties are the same.
+Object.defineProperty(obj5, 'foo', descDifferent);
+
+// Make sure that obj5 still holds the original values.
+desc = Object.getOwnPropertyDescriptor(obj5, 'foo');
+assertEquals(desc.get, getter2);
+assertEquals(desc.set, setter2);
+assertFalse(desc.enumerable);
+assertFalse(desc.configurable);
+
+
+// Also exercise step 6 on data property, writable and enumerable
+// defaults to false.
+Object.defineProperty(obj5, 'bar', dataNoConfigurable);
+
+// Test that redefinition with the same property descriptor is possible
+Object.defineProperty(obj5, 'bar', dataNoConfigurable);
+
+// Different value.
+descDifferent = {
+  configurable:false,
+  enumerable:false,
+  writable: false,
+  value: 1999
+};
+
+try {
+  Object.defineProperty(obj5, 'bar', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// Different writable.
+descDifferent = {
+  configurable:false,
+  enumerable:false,
+  writable: true,
+  value: 2000
+};
+
+try {
+  Object.defineProperty(obj5, 'bar', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+
+// Different enumerable.
+descDifferent = {
+  configurable:false,
+  enumerable:true ,
+  writable:false,
+  value: 2000
+};
+
+try {
+  Object.defineProperty(obj5, 'bar', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+
+// Different configurable.
+descDifferent = {
+  configurable:true,
+  enumerable:false,
+  writable:false,
+  value: 2000
+};
+
+try {
+  Object.defineProperty(obj5, 'bar', descDifferent);
+  assertTrue(false);
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+// No difference.
+descDifferent = {
+  configurable:false,
+  enumerable:false,
+  writable:false,
+  value:2000
+};
+// Make sure we can still redefine if all properties are the same.
+Object.defineProperty(obj5, 'bar', descDifferent);
+
+// Make sure that obj5 still holds the original values.
+desc = Object.getOwnPropertyDescriptor(obj5, 'bar');
+assertEquals(desc.value, 2000);
+assertFalse(desc.writable);
+assertFalse(desc.enumerable);
+assertFalse(desc.configurable);
+
+
+// Make sure that we can't overwrite +0 with -0 and vise verca.

[Erik Corry, 2010/05/21 13:21:57]

vise verca -> vice versa

(or was that deliberate :-)

[Rico, 2010/05/25 06:24:50]

No :-) done

+var descMinusZero = {value: -0, configurable: false};
+var descPlusZero = {value: +0, configurable: false};
+
+Object.defineProperty(obj5, 'minuszero', descMinusZero);
+
+// Make sure we can redefine with -0.
+Object.defineProperty(obj5, 'minuszero', descMinusZero);
+
+try {
+  Object.defineProperty(obj5, 'minuszero', descPlusZero);
+  assertUnreachable();
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}
+
+
+Object.defineProperty(obj5, 'pluszero', descPlusZero);
+
+// Make sure we can redefine with +0.
+Object.defineProperty(obj5, 'pluszero', descPlusZero);
+
+try {
+  Object.defineProperty(obj5, 'pluszero', descMinusZero);
+  assertUnreachable();
+} catch (e) {
+  assertTrue(/Cannot redefine property/.test(e));
+}