Commit 1854231f authored by oliver@apple.com's avatar oliver@apple.com
Browse files

2011-01-15 Oliver Hunt <oliver@apple.com>

        Reviewed by Maciej Stachowiak.

        Incorrect behavior changing attributes of an accessor
        https://bugs.webkit.org/show_bug.cgi?id=52515

        defineProperty doesn't correctly handle changing attributes of an accessor
        property.  This is because we don't pass the full descriptor to the
        putDescriptor helper function, which means we have insufficient information
        to do the right thing. Once that's passed the correct behavior is relatively
        simple to implement.

        * runtime/JSObject.cpp:
        (JSC::putDescriptor):
        (JSC::JSObject::defineOwnProperty):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75884 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent d673f252
2011-01-15 Oliver Hunt <oliver@apple.com>
Reviewed by Maciej Stachowiak.
Incorrect behavior changing attributes of an accessor
https://bugs.webkit.org/show_bug.cgi?id=52515
Add an additional test for validating changing attributes of
an accessor property.s
* fast/js/Object-defineProperty-expected.txt:
* fast/js/script-tests/Object-defineProperty.js:
2011-01-15 David Kilzer <ddkilzer@apple.com>
 
<http://webkit.org/b/52512> REGRESSION(r73818): range.cloneContents() ignores end offset
......
......@@ -23,6 +23,7 @@ PASS Object.defineProperty('foo') threw exception TypeError: Properties can only
PASS Object.defineProperty({}) threw exception TypeError: Property description must be an object..
PASS Object.defineProperty({}, 'foo') threw exception TypeError: Property description must be an object..
PASS Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo is true
PASS Object.defineProperty({get foo() { return true; } }, 'foo', {configurable:false}).foo is true
PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {configurable: true}) threw exception TypeError: Attempting to configurable attribute of unconfigurable property..
PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {writable: true}) threw exception TypeError: Attempting to change writable attribute of unconfigurable property..
PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {enumerable: true}) threw exception TypeError: Attempting to change enumerable attribute of unconfigurable property..
......
......@@ -31,6 +31,7 @@ shouldThrow("Object.defineProperty('foo')");
shouldThrow("Object.defineProperty({})");
shouldThrow("Object.defineProperty({}, 'foo')");
shouldBeTrue("Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo");
shouldBeTrue("Object.defineProperty({get foo() { return true; } }, 'foo', {configurable:false}).foo");
function createUnconfigurableProperty(o, prop, writable, enumerable) {
writable = writable || false;
......
2011-01-15 Oliver Hunt <oliver@apple.com>
Reviewed by Maciej Stachowiak.
Incorrect behavior changing attributes of an accessor
https://bugs.webkit.org/show_bug.cgi?id=52515
defineProperty doesn't correctly handle changing attributes of an accessor
property. This is because we don't pass the full descriptor to the
putDescriptor helper function, which means we have insufficient information
to do the right thing. Once that's passed the correct behavior is relatively
simple to implement.
* runtime/JSObject.cpp:
(JSC::putDescriptor):
(JSC::JSObject::defineOwnProperty):
2011-01-14 Oliver Hunt <oliver@apple.com>
 
Reviewed by Maciej Stachowiak.
......
......@@ -588,10 +588,28 @@ bool JSObject::getPropertyDescriptor(ExecState* exec, const Identifier& property
}
}
static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& propertyName, PropertyDescriptor& descriptor, unsigned attributes, JSValue oldValue)
static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& propertyName, PropertyDescriptor& descriptor, unsigned attributes, const PropertyDescriptor& oldDescriptor)
{
if (descriptor.isGenericDescriptor() || descriptor.isDataDescriptor()) {
target->putWithAttributes(exec, propertyName, descriptor.value() ? descriptor.value() : oldValue, attributes & ~(Getter | Setter));
if (descriptor.isGenericDescriptor() && oldDescriptor.isAccessorDescriptor()) {
GetterSetter* accessor = new (exec) GetterSetter(exec);
if (oldDescriptor.getter()) {
attributes |= Getter;
accessor->setGetter(asObject(oldDescriptor.getter()));
}
if (oldDescriptor.setter()) {
attributes |= Setter;
accessor->setSetter(asObject(oldDescriptor.setter()));
}
target->putWithAttributes(exec, propertyName, accessor, attributes);
return true;
}
JSValue newValue = jsUndefined();
if (descriptor.value())
newValue = descriptor.value();
else if (oldDescriptor.value())
newValue = oldDescriptor.value();
target->putWithAttributes(exec, propertyName, newValue, attributes & ~(Getter | Setter));
return true;
}
attributes &= ~ReadOnly;
......@@ -608,8 +626,11 @@ bool JSObject::defineOwnProperty(ExecState* exec, const Identifier& propertyName
{
// If we have a new property we can just put it on normally
PropertyDescriptor current;
if (!getOwnPropertyDescriptor(exec, propertyName, current))
return putDescriptor(exec, this, propertyName, descriptor, descriptor.attributes(), jsUndefined());
if (!getOwnPropertyDescriptor(exec, propertyName, current)) {
PropertyDescriptor oldDescriptor;
oldDescriptor.setValue(jsUndefined());
return putDescriptor(exec, this, propertyName, descriptor, descriptor.attributes(), oldDescriptor);
}
if (descriptor.isEmpty())
return true;
......@@ -635,7 +656,7 @@ bool JSObject::defineOwnProperty(ExecState* exec, const Identifier& propertyName
if (descriptor.isGenericDescriptor()) {
if (!current.attributesEqual(descriptor)) {
deleteProperty(exec, propertyName);
putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value());
putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
}
return true;
}
......@@ -648,7 +669,7 @@ bool JSObject::defineOwnProperty(ExecState* exec, const Identifier& propertyName
return false;
}
deleteProperty(exec, propertyName);
return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value() ? current.value() : jsUndefined());
return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
}
// Changing the value and attributes of an existing property
......@@ -676,7 +697,7 @@ bool JSObject::defineOwnProperty(ExecState* exec, const Identifier& propertyName
return true;
}
deleteProperty(exec, propertyName);
return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value());
return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
}
// Changing the accessor functions of an existing accessor property
......
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