Change calls to undefined property setters to not throw (fixes issue 1355).

We currently throw when there is only a getter defined on the
property, but this should only be the case in strict mode.
Review URL: http://codereview.chromium.org/7064027

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8054 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
ricow@chromium.org 2011-05-25 08:37:38 +00:00
parent 2f36b16343
commit f675db651d
5 changed files with 147 additions and 40 deletions

View File

@ -1771,7 +1771,8 @@ MaybeObject* JSObject::SetProperty(String* name,
MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
String* name,
Object* value,
JSObject* holder) {
JSObject* holder,
StrictModeFlag strict_mode) {
Isolate* isolate = GetIsolate();
HandleScope scope(isolate);
@ -1819,6 +1820,9 @@ MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
if (setter->IsJSFunction()) {
return SetPropertyWithDefinedSetter(JSFunction::cast(setter), value);
} else {
if (strict_mode == kNonStrictMode) {
return value;
}
Handle<String> key(name);
Handle<Object> holder_handle(holder, isolate);
Handle<Object> args[2] = { key, holder_handle };
@ -1876,9 +1880,11 @@ void JSObject::LookupCallbackSetterInPrototypes(String* name,
}
MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
Object* value,
bool* found) {
MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(
uint32_t index,
Object* value,
bool* found,
StrictModeFlag strict_mode) {
Heap* heap = GetHeap();
for (Object* pt = GetPrototype();
pt != heap->null_value();
@ -1892,8 +1898,11 @@ MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
PropertyDetails details = dictionary->DetailsAt(entry);
if (details.type() == CALLBACKS) {
*found = true;
return SetElementWithCallback(
dictionary->ValueAt(entry), index, value, JSObject::cast(pt));
return SetElementWithCallback(dictionary->ValueAt(entry),
index,
value,
JSObject::cast(pt),
strict_mode);
}
}
}
@ -2074,10 +2083,12 @@ void JSObject::LookupRealNamedPropertyInPrototypes(String* name,
// We only need to deal with CALLBACKS and INTERCEPTORS
MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
String* name,
Object* value,
bool check_prototype) {
MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(
LookupResult* result,
String* name,
Object* value,
bool check_prototype,
StrictModeFlag strict_mode) {
if (check_prototype && !result->IsProperty()) {
LookupCallbackSetterInPrototypes(name, result);
}
@ -2093,7 +2104,8 @@ MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
return SetPropertyWithCallback(result->GetCallbackObject(),
name,
value,
result->holder());
result->holder(),
strict_mode);
}
}
break;
@ -2104,8 +2116,11 @@ MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
LookupResult r;
LookupRealNamedProperty(name, &r);
if (r.IsProperty()) {
return SetPropertyWithFailedAccessCheck(&r, name, value,
check_prototype);
return SetPropertyWithFailedAccessCheck(&r,
name,
value,
check_prototype,
strict_mode);
}
break;
}
@ -2149,7 +2164,11 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
// Check access rights if needed.
if (IsAccessCheckNeeded()
&& !heap->isolate()->MayNamedAccess(this, name, v8::ACCESS_SET)) {
return SetPropertyWithFailedAccessCheck(result, name, value, true);
return SetPropertyWithFailedAccessCheck(result,
name,
value,
true,
strict_mode);
}
if (IsJSGlobalProxy()) {
@ -2169,7 +2188,8 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
return SetPropertyWithCallback(accessor_result.GetCallbackObject(),
name,
value,
accessor_result.holder());
accessor_result.holder(),
strict_mode);
}
}
if (!result->IsFound()) {
@ -2213,7 +2233,8 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
return SetPropertyWithCallback(result->GetCallbackObject(),
name,
value,
result->holder());
result->holder(),
strict_mode);
case INTERCEPTOR:
return SetPropertyWithInterceptor(name, value, attributes, strict_mode);
case CONSTANT_TRANSITION: {
@ -2266,7 +2287,11 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
if (IsAccessCheckNeeded()) {
Heap* heap = GetHeap();
if (!heap->isolate()->MayNamedAccess(this, name, v8::ACCESS_SET)) {
return SetPropertyWithFailedAccessCheck(&result, name, value, false);
return SetPropertyWithFailedAccessCheck(&result,
name,
value,
false,
kNonStrictMode);
}
}
@ -7406,7 +7431,8 @@ MaybeObject* JSObject::GetElementWithCallback(Object* receiver,
MaybeObject* JSObject::SetElementWithCallback(Object* structure,
uint32_t index,
Object* value,
JSObject* holder) {
JSObject* holder,
StrictModeFlag strict_mode) {
Isolate* isolate = GetIsolate();
HandleScope scope(isolate);
@ -7445,10 +7471,13 @@ MaybeObject* JSObject::SetElementWithCallback(Object* structure,
}
if (structure->IsFixedArray()) {
Object* setter = FixedArray::cast(structure)->get(kSetterIndex);
Handle<Object> setter(FixedArray::cast(structure)->get(kSetterIndex));
if (setter->IsJSFunction()) {
return SetPropertyWithDefinedSetter(JSFunction::cast(setter), value);
return SetPropertyWithDefinedSetter(JSFunction::cast(*setter), value);
} else {
if (strict_mode == kNonStrictMode) {
return value;
}
Handle<Object> holder_handle(holder, isolate);
Handle<Object> key(isolate->factory()->NewNumberFromUint(index));
Handle<Object> args[2] = { key, holder_handle };
@ -7482,8 +7511,10 @@ MaybeObject* JSObject::SetFastElement(uint32_t index,
if (check_prototype &&
(index >= elms_length || elms->get(index)->IsTheHole())) {
bool found;
MaybeObject* result =
SetElementWithCallbackSetterInPrototypes(index, value, &found);
MaybeObject* result = SetElementWithCallbackSetterInPrototypes(index,
value,
&found,
strict_mode);
if (found) return result;
}
@ -7627,7 +7658,11 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
Object* element = dictionary->ValueAt(entry);
PropertyDetails details = dictionary->DetailsAt(entry);
if (details.type() == CALLBACKS) {
return SetElementWithCallback(element, index, value, this);
return SetElementWithCallback(element,
index,
value,
this,
strict_mode);
} else {
dictionary->UpdateMaxNumberKey(index);
// If put fails instrict mode, throw exception.
@ -7647,7 +7682,10 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
bool found;
MaybeObject* result =
// Strict mode not needed. No-setter case already handled.
SetElementWithCallbackSetterInPrototypes(index, value, &found);
SetElementWithCallbackSetterInPrototypes(index,
value,
&found,
strict_mode);
if (found) return result;
}
// When we set the is_extensible flag to false we always force

View File

@ -1425,11 +1425,14 @@ class JSObject: public HeapObject {
LookupResult* result,
String* name,
Object* value,
bool check_prototype);
MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(Object* structure,
String* name,
Object* value,
JSObject* holder);
bool check_prototype,
StrictModeFlag strict_mode);
MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(
Object* structure,
String* name,
Object* value,
JSObject* holder,
StrictModeFlag strict_mode);
MUST_USE_RESULT MaybeObject* SetPropertyWithDefinedSetter(JSFunction* setter,
Object* value);
MUST_USE_RESULT MaybeObject* SetPropertyWithInterceptor(
@ -1656,7 +1659,7 @@ class JSObject: public HeapObject {
void LookupRealNamedPropertyInPrototypes(String* name, LookupResult* result);
void LookupCallbackSetterInPrototypes(String* name, LookupResult* result);
MUST_USE_RESULT MaybeObject* SetElementWithCallbackSetterInPrototypes(
uint32_t index, Object* value, bool* found);
uint32_t index, Object* value, bool* found, StrictModeFlag strict_mode);
void LookupCallback(String* name, LookupResult* result);
// Returns the number of properties on this object filtering out properties
@ -1868,7 +1871,8 @@ class JSObject: public HeapObject {
MaybeObject* SetElementWithCallback(Object* structure,
uint32_t index,
Object* value,
JSObject* holder);
JSObject* holder,
StrictModeFlag strict_mode);
MUST_USE_RESULT MaybeObject* SetElementWithInterceptor(
uint32_t index,
Object* value,

View File

@ -25,8 +25,9 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Test that exceptions are thrown when setting properties on object
// that have only a getter in a prototype object.
// Test that exceptions are not thrown when setting properties on object
// that have only a getter in a prototype object, except when we are in strict
// mode where exceptsions should be thrown.
var o = {};
var p = {};
@ -34,25 +35,44 @@ p.__defineGetter__('x', function(){});
p.__defineGetter__(0, function(){});
o.__proto__ = p;
assertThrows("o.x = 42");
assertThrows("o[0] = 42");
assertDoesNotThrow("o.x = 42");
assertDoesNotThrow("o[0] = 42");
assertThrows(function() { 'use strict'; o.x = 42; });
assertThrows(function() { 'use strict'; o[0] = 42; });
function f() {
with(o) {
x = 42;
}
}
assertThrows("f()");
assertDoesNotThrow(f);
__proto__ = p;
function g() {
eval('1');
x = 42;
}
assertThrows("g()");
function g_strict() {
'use strict';
eval('1');
x = 42;
}
assertDoesNotThrow(g);
assertThrows(g_strict);
__proto__ = p;
function g2() {
this[0] = 42;
}
assertThrows("g2()");
function g2_strict() {
'use strict';
this[0] = 42;
}
assertDoesNotThrow(g2);
assertThrows(g2_strict);

View File

@ -81,10 +81,11 @@ testArray();
expected[0] = 111;
testArray();
// Using a setter where only a getter is defined throws an exception.
// Using a setter where only a getter is defined does not throw an exception,
// unless we are in strict mode.
var q = {};
q.__defineGetter__('0', function() { return 42; });
assertThrows('q[0] = 7');
assertDoesNotThrow('q[0] = 7');
// Using a getter where only a setter is defined returns undefined.
var q1 = {};

View File

@ -0,0 +1,44 @@
// Copyright 2011 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
// with the distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived
// from this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Test that an exception is not thrown when trying to set a value for
// a property that has only a defined getter, except when in strict mode.
var foo = Object.defineProperty({}, "bar", {
get: function () {
return 10;
}
});
assertDoesNotThrow("foo.bar = 20");
function shouldThrow() {
'use strict';
foo.bar = 20;
}
assertThrows("shouldThrow()");