Revision: 6232
Author: [email protected]
Date: Sun Jan 9 23:20:54 2011
Log: Revert 6220 (generic descriptor support in Object.defineOwnProperty)
This change caused a webkit failure in
http/tests/security/xss-DENIED-defineProperty.html.
I will look into this and reapply when I find a solution.
Review URL: http://codereview.chromium.org/6134005
http://code.google.com/p/v8/source/detail?r=6232
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/v8natives.js
/branches/bleeding_edge/test/mjsunit/object-define-property.js
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Jan 7 03:49:09 2011
+++ /branches/bleeding_edge/src/runtime.cc Sun Jan 9 23:20:54 2011
@@ -3499,12 +3499,7 @@
args.at<Object>(1));
}
-// Implements part of 8.12.9 DefineOwnProperty.
-// There are 3 cases that lead here:
-// Step 4b - define a new accessor property.
-// Steps 9c & 12 - replace an existing data property with an accessor
property.
-// Step 12 - update an existing accessor property with an accessor or
generic
-// descriptor.
+
static MaybeObject* Runtime_DefineOrRedefineAccessorProperty(Arguments
args) {
ASSERT(args.length() == 5);
HandleScope scope;
@@ -3536,12 +3531,6 @@
return obj->DefineAccessor(name, flag_setter->value() == 0, fun, attr);
}
-// Implements part of 8.12.9 DefineOwnProperty.
-// There are 3 cases that lead here:
-// Step 4a - define a new data property.
-// Steps 9b & 12 - replace an existing accessor property with a data
property.
-// Step 12 - update an existing data property with a data or generic
-// descriptor.
static MaybeObject* Runtime_DefineOrRedefineDataProperty(Arguments args) {
ASSERT(args.length() == 4);
HandleScope scope;
=======================================
--- /branches/bleeding_edge/src/v8natives.js Fri Jan 7 05:21:34 2011
+++ /branches/bleeding_edge/src/v8natives.js Sun Jan 9 23:20:54 2011
@@ -545,12 +545,10 @@
if (IS_UNDEFINED(current) && !extensible)
throw MakeTypeError("define_disallowed", ["defineProperty"]);
- if (!IS_UNDEFINED(current)) {
+ if (!IS_UNDEFINED(current) && !current.isConfigurable()) {
// Step 5 and 6
- if ((IsGenericDescriptor(desc) ||
- IsDataDescriptor(desc) == IsDataDescriptor(current)) &&
- (!desc.hasEnumerable() ||
- SameValue(desc.isEnumerable(), current.isEnumerable())) &&
+ if ((!desc.hasEnumerable() ||
+ SameValue(desc.isEnumerable() && current.isEnumerable())) &&
(!desc.hasConfigurable() ||
SameValue(desc.isConfigurable(), current.isConfigurable())) &&
(!desc.hasWritable() ||
@@ -563,36 +561,30 @@
SameValue(desc.getSet(), current.getSet()))) {
return true;
}
- if (!current.isConfigurable()) {
- // Step 7
- if (desc.isConfigurable() ||
- (desc.hasEnumerable() &&
- desc.isEnumerable() != current.isEnumerable()))
+
+ // Step 7
+ if (desc.isConfigurable() || desc.isEnumerable() !=
current.isEnumerable())
+ throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+ // Step 9
+ if (IsDataDescriptor(current) != IsDataDescriptor(desc))
+ throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+ // Step 10
+ if (IsDataDescriptor(current) && IsDataDescriptor(desc)) {
+ if (!current.isWritable() && desc.isWritable())
throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- // Step 8
- if (!IsGenericDescriptor(desc)) {
- // Step 9a
- if (IsDataDescriptor(current) != IsDataDescriptor(desc))
- throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- // Step 10a
- if (IsDataDescriptor(current) && IsDataDescriptor(desc)) {
- if (!current.isWritable() && desc.isWritable())
- throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- if (!current.isWritable() && desc.hasValue() &&
- !SameValue(desc.getValue(), current.getValue())) {
- throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- }
- }
- // Step 11
- if (IsAccessorDescriptor(desc) && IsAccessorDescriptor(current)) {
- if (desc.hasSetter() && !SameValue(desc.getSet(),
current.getSet())){
- throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- }
- if (desc.hasGetter()
&& !SameValue(desc.getGet(),current.getGet()))
- throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
- }
+ if (!current.isWritable() && desc.hasValue() &&
+ !SameValue(desc.getValue(), current.getValue())) {
+ throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
}
}
+ // Step 11
+ if (IsAccessorDescriptor(desc) && IsAccessorDescriptor(current)) {
+ if (desc.hasSetter() && !SameValue(desc.getSet(), current.getSet())){
+ throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+ }
+ if (desc.hasGetter() && !SameValue(desc.getGet(),current.getGet()))
+ throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+ }
}
// Send flags - enumerable and configurable are common - writable is
@@ -615,16 +607,7 @@
} else
flag |= DONT_DELETE;
- if (IsDataDescriptor(desc) ||
- (IsGenericDescriptor(desc) &&
- (IS_UNDEFINED(current) || IsDataDescriptor(current)))) {
- // There are 3 cases that lead here:
- // Step 4a - defining a new data property.
- // Steps 9b & 12 - replacing an existing accessor property with a data
- // property.
- // Step 12 - updating an existing data property with a data or generic
- // descriptor.
-
+ if (IsDataDescriptor(desc) || IsGenericDescriptor(desc)) {
if (desc.hasWritable()) {
flag |= desc.isWritable() ? 0 : READ_ONLY;
} else if (!IS_UNDEFINED(current)) {
@@ -632,30 +615,20 @@
} else {
flag |= READ_ONLY;
}
-
var value = void 0; // Default value is undefined.
if (desc.hasValue()) {
value = desc.getValue();
- } else if (!IS_UNDEFINED(current) && IsDataDescriptor(current)) {
+ } else if (!IS_UNDEFINED(current)) {
value = current.getValue();
}
-
%DefineOrRedefineDataProperty(obj, p, value, flag);
- } else if (IsGenericDescriptor(desc)) {
- // Step 12 - updating an existing accessor property with a generic
- // descriptor. Changing flags only.
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, current.getGet(),
flag);
} else {
- // There are 3 cases that lead here:
- // Step 4b - defining a new accessor property.
- // Steps 9c & 12 - replacing an existing data property with an accessor
- // property.
- // Step 12 - updating an existing accessor property with an accessor
- // descriptor.
- if (desc.hasGetter()) {
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, desc.getGet(),
flag);
- }
- if (desc.hasSetter()) {
+ if (desc.hasGetter() &&
+ (IS_FUNCTION(desc.getGet()) || IS_UNDEFINED(desc.getGet()))) {
+ %DefineOrRedefineAccessorProperty(obj, p, GETTER, desc.getGet(),
flag);
+ }
+ if (desc.hasSetter() &&
+ (IS_FUNCTION(desc.getSet()) || IS_UNDEFINED(desc.getSet()))) {
%DefineOrRedefineAccessorProperty(obj, p, SETTER, desc.getSet(),
flag);
}
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-define-property.js Fri Jan
7 03:49:09 2011
+++ /branches/bleeding_edge/test/mjsunit/object-define-property.js Sun Jan
9 23:20:54 2011
@@ -749,32 +749,13 @@
assertTrue(desc.enumerable);
assertFalse(desc.configurable);
-// Can use defineProperty to change the value of a non
-// configurable property.
+// Ensure that we can't overwrite the non configurable element.
try {
Object.defineProperty(obj6, '2', descElement);
- desc = Object.getOwnPropertyDescriptor(obj6, '2');
- assertEquals(desc.value, 'foobar');
-} catch (e) {
assertUnreachable();
-}
-
-// Ensure that we can't change the descriptor of a
-// non configurable property.
-try {
- var descAccessor = { get: function() { return 0; } };
- Object.defineProperty(obj6, '2', descAccessor);
- assertUnreachable();
} catch (e) {
assertTrue(/Cannot redefine property/.test(e));
}
-
-Object.defineProperty(obj6, '2', descElementNonWritable);
-desc = Object.getOwnPropertyDescriptor(obj6, '2');
-assertEquals(desc.value, 'foofoo');
-assertFalse(desc.writable);
-assertTrue(desc.enumerable);
-assertFalse(desc.configurable);
Object.defineProperty(obj6, '3', descElementNonWritable);
desc = Object.getOwnPropertyDescriptor(obj6, '3');
@@ -846,32 +827,13 @@
assertTrue(desc.enumerable);
assertFalse(desc.configurable);
-// Can use defineProperty to change the value of a non
-// configurable property of an array.
+// Ensure that we can't overwrite the non configurable element.
try {
Object.defineProperty(arr, '2', descElement);
- desc = Object.getOwnPropertyDescriptor(arr, '2');
- assertEquals(desc.value, 'foobar');
-} catch (e) {
assertUnreachable();
-}
-
-// Ensure that we can't change the descriptor of a
-// non configurable property.
-try {
- var descAccessor = { get: function() { return 0; } };
- Object.defineProperty(arr, '2', descAccessor);
- assertUnreachable();
} catch (e) {
assertTrue(/Cannot redefine property/.test(e));
}
-
-Object.defineProperty(arr, '2', descElementNonWritable);
-desc = Object.getOwnPropertyDescriptor(arr, '2');
-assertEquals(desc.value, 'foofoo');
-assertFalse(desc.writable);
-assertTrue(desc.enumerable);
-assertFalse(desc.configurable);
Object.defineProperty(arr, '3', descElementNonWritable);
desc = Object.getOwnPropertyDescriptor(arr, '3');
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
