Skip to content

V
V8
Commit 8c1377a5 authored by neis's avatar neis Committed by Commit bot
Browse files
Options

Fix corner-case behavior of Object::SetSuperProperty. 

When the property is an accessor property in the receiver but not on the
holder (ES6 "target"), we must fail.

R=rossberg, verwaest@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1427113002

Cr-Commit-Position: refs/heads/master@{#31733}
parent 5f4828a1
Hide whitespace changes
Inline Side-by-side
Showing with 42 additions and 72 deletions
src/objects.cc
View file @ 8c1377a5
......@@ -3738,6 +3738,9 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
SetPropertyInternal(it, value, language_mode, store_mode, &found);
if (found) return result;
// The property either doesn't exist on the holder or exists there as a data
// property.
if (!it->GetReceiver()->IsJSReceiver()) {
return WriteToReadOnlyProperty(it, value, should_throw);
}
......@@ -3758,8 +3761,8 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
break;
case LookupIterator::INTEGER_INDEXED_EXOTIC:
return RedefineNonconfigurableProperty(it->isolate(), it->GetName(),
value, should_throw);
return RedefineIncompatibleProperty(it->isolate(), it->GetName(), value,
should_throw);
case LookupIterator::DATA: {
PropertyDetails details = own_lookup.property_details();
......@@ -3771,14 +3774,8 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
}
case LookupIterator::ACCESSOR: {
PropertyDetails details = own_lookup.property_details();
if (details.IsConfigurable()) {
return JSObject::DefineOwnPropertyIgnoreAttributes(
&own_lookup, value, details.attributes(), should_throw);
}
return RedefineNonconfigurableProperty(it->isolate(), it->GetName(),
value, should_throw);
return RedefineIncompatibleProperty(it->isolate(), it->GetName(), value,
should_throw);
}
case LookupIterator::INTERCEPTOR:
......@@ -3857,10 +3854,10 @@ Maybe<bool> Object::WriteToReadOnlyProperty(Isolate* isolate,
}
Maybe<bool> Object::RedefineNonconfigurableProperty(Isolate* isolate,
Handle<Object> name,
Handle<Object> value,
ShouldThrow should_throw) {
Maybe<bool> Object::RedefineIncompatibleProperty(Isolate* isolate,
Handle<Object> name,
Handle<Object> value,
ShouldThrow should_throw) {
RETURN_FAILURE(isolate, should_throw,
NewTypeError(MessageTemplate::kRedefineDisallowed, name));
}
......@@ -4850,8 +4847,8 @@ Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
return Just(true);
}
case LookupIterator::INTEGER_INDEXED_EXOTIC:
return RedefineNonconfigurableProperty(it->isolate(), it->GetName(),
value, should_throw);
return RedefineIncompatibleProperty(it->isolate(), it->GetName(), value,
should_throw);
case LookupIterator::DATA: {
PropertyDetails details = it->property_details();
......@@ -4864,8 +4861,8 @@ Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
// Special case: properties of typed arrays cannot be reconfigured to
// non-writable nor to non-enumerable.
if (it->IsElement() && object->HasFixedTypedArrayElements()) {
return RedefineNonconfigurableProperty(it->isolate(), it->GetName(),
value, should_throw);
return RedefineIncompatibleProperty(it->isolate(), it->GetName(),
value, should_throw);
}
// Reconfigure the data property if the attributes mismatch.
......
src/objects.h
View file @ 8c1377a5
......@@ -1264,7 +1264,7 @@ class Object {
MUST_USE_RESULT static Maybe<bool> WriteToReadOnlyProperty(
Isolate* isolate, Handle<Object> receiver, Handle<Object> name,
Handle<Object> value, ShouldThrow should_throw);
MUST_USE_RESULT static Maybe<bool> RedefineNonconfigurableProperty(
MUST_USE_RESULT static Maybe<bool> RedefineIncompatibleProperty(
Isolate* isolate, Handle<Object> name, Handle<Object> value,
ShouldThrow should_throw);
MUST_USE_RESULT static Maybe<bool> SetDataProperty(LookupIterator* it,
......
test/mjsunit/es6/super.js
View file @ 8c1377a5
......@@ -949,21 +949,14 @@
assertEquals(15, this.ownReadonlyAccessor);
super.ownReadonlyAccessor = 25;
assertEquals(25, this.ownReadonlyAccessor);
var descr = Object.getOwnPropertyDescriptor(this, 'ownReadonlyAccessor');
assertEquals(25, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertSame(undefined, super.ownReadonlyAccessor);
assertEquals(15, this.ownReadonlyAccessor);
assertFalse(Base.prototype.hasOwnProperty('ownReadonlyAccessor'));
super.ownSetter = 35;
assertEquals(35, this.ownSetter);
assertSame(undefined, super.ownSetter);
var descr = Object.getOwnPropertyDescriptor(this, 'ownSetter');
assertEquals(35, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertTrue('set' in descr);
assertFalse(Base.prototype.hasOwnProperty('ownSetter'));
},
mStrict() {
......@@ -979,22 +972,15 @@
assertFalse(Base.prototype.hasOwnProperty('ownReadOnly'));
assertEquals(15, this.ownReadonlyAccessor);
super.ownReadonlyAccessor = 25;
assertEquals(25, this.ownReadonlyAccessor);
var descr = Object.getOwnPropertyDescriptor(this, 'ownReadonlyAccessor');
assertEquals(25, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertThrows(() => {super.ownReadonlyAccessor = 25}, TypeError);
assertSame(undefined, super.ownReadonlyAccessor);
assertEquals(15, this.ownReadonlyAccessor);
assertFalse(Base.prototype.hasOwnProperty('ownReadonlyAccessor'));
super.ownSetter = 35;
assertEquals(35, this.ownSetter);
assertThrows(() => {super.ownSetter = 35}, TypeError);
assertSame(undefined, super.ownSetter);
var descr = Object.getOwnPropertyDescriptor(this, 'ownSetter');
assertEquals(35, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertTrue('set' in descr);
assertFalse(Base.prototype.hasOwnProperty('ownSetter'));
},
};
......@@ -1052,6 +1038,7 @@
assertEquals(15, this.ownReadonlyAccessor);
super.ownReadonlyAccessor = 25;
assertSame(undefined, super.ownReadonlyAccessor);
assertEquals(15, this.ownReadonlyAccessor);
var descr = Object.getOwnPropertyDescriptor(this, 'ownReadonlyAccessor');
assertFalse(descr.configurable);
......@@ -1190,21 +1177,14 @@ function TestKeyedSetterCreatingOwnPropertiesReconfigurable(ownReadOnly,
assertEquals(15, this[ownReadonlyAccessor]);
super[ownReadonlyAccessor] = 25;
assertEquals(25, this[ownReadonlyAccessor]);
var descr = Object.getOwnPropertyDescriptor(this, ownReadonlyAccessor);
assertEquals(25, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertSame(undefined, super[ownReadonlyAccessor]);
assertEquals(15, this[ownReadonlyAccessor]);
assertFalse(Base.prototype.hasOwnProperty(ownReadonlyAccessor));
super[ownSetter] = 35;
assertEquals(35, this[ownSetter]);
assertSame(undefined, super[ownSetter]);
var descr = Object.getOwnPropertyDescriptor(this, ownSetter);
assertEquals(35, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertTrue('set' in descr);
assertFalse(Base.prototype.hasOwnProperty(ownSetter));
},
mStrict() {
......@@ -1220,22 +1200,15 @@ function TestKeyedSetterCreatingOwnPropertiesReconfigurable(ownReadOnly,
assertFalse(Base.prototype.hasOwnProperty(ownReadOnly));
assertEquals(15, this[ownReadonlyAccessor]);
super[ownReadonlyAccessor] = 25;
assertEquals(25, this[ownReadonlyAccessor]);
var descr = Object.getOwnPropertyDescriptor(this, ownReadonlyAccessor);
assertEquals(25, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertThrows(() => {super[ownReadonlyAccessor] = 25}, TypeError);
assertSame(undefined, super[ownReadonlyAccessor]);
assertEquals(15, this[ownReadonlyAccessor]);
assertFalse(Base.prototype.hasOwnProperty(ownReadonlyAccessor));
super[ownSetter] = 35;
assertEquals(35, this[ownSetter]);
assertThrows(() => {super[ownSetter] = 35}, TypeError);
assertSame(undefined, super[ownSetter]);
var descr = Object.getOwnPropertyDescriptor(this, ownSetter);
assertEquals(35, descr.value);
assertTrue(descr.configurable);
assertFalse(descr.enumerable);
assertTrue(descr.writable);
assertTrue('set' in descr);
assertFalse(Base.prototype.hasOwnProperty(ownSetter));
},
};
......@@ -1298,6 +1271,7 @@ function TestKeyedSetterCreatingOwnPropertiesNonConfigurable(
assertEquals(15, this[ownReadonlyAccessor]);
super[ownReadonlyAccessor] = 25;
assertSame(undefined, super[ownReadonlyAccessor]);
assertEquals(15, this[ownReadonlyAccessor]);
var descr = Object.getOwnPropertyDescriptor(this, ownReadonlyAccessor);
assertFalse(descr.configurable);
......
test/mjsunit/harmony/reflect.js
View file @ 8c1377a5
......@@ -248,11 +248,10 @@ function prepare(target) {
// assertFalse(Reflect.set({}, "nowrite", value, target));
// Data vs Accessor
// TODO(neis): These must return false but currently don't.
// assertFalse(Reflect.set(target, "unknown", value, {set bla(x) {}}));
// assertFalse(Reflect.set(target, "unknown", value, {get bla() {}}));
// assertFalse(Reflect.set(target, "bla", value, {set bla(x) {}}));
// assertFalse(Reflect.set(target, "bla", value, {get bla() {}}));
assertFalse(Reflect.set({}, "unknown", 0, {set unknown(x) {}}));
assertFalse(Reflect.set(target, "unknown", value, {set unknown(x) {}}));
assertFalse(Reflect.set(target, "bla", value, {set bla(x) {}}));
assertFalse(Reflect.set(target, "bla", value, {get bla() {}}));
// Accessor vs Data
assertTrue(Reflect.set({set bla(x) {}}), "bla", value, target);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment