--- a/js/src/jsapi-tests/moz.build
+++ b/js/src/jsapi-tests/moz.build
@@ -19,16 +19,17 @@ UNIFIED_SOURCES += [
'testCloneScript.cpp',
'testConservativeGC.cpp',
'testContexts.cpp',
'testCustomIterator.cpp',
'testDebugger.cpp',
'testDeepFreeze.cpp',
'testDefineGetterSetterNonEnumerable.cpp',
'testDefineProperty.cpp',
+ 'testDefinePropertyIgnoredAttributes.cpp',
'testEnclosingFunction.cpp',
'testErrorCopying.cpp',
'testException.cpp',
'testExternalStrings.cpp',
'testFindSCCs.cpp',
'testFreshGlobalEvalRedefinition.cpp',
'testFuncCallback.cpp',
'testFunctionProperties.cpp',
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
@@ -0,0 +1,94 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "jsapi-tests/tests.h"
+
+static const unsigned IgnoreWithValue = JSPROP_IGNORE_ENUMERATE | JSPROP_IGNORE_READONLY |
+ JSPROP_IGNORE_PERMANENT;
+static const unsigned IgnoreAll = IgnoreWithValue | JSPROP_IGNORE_VALUE;
+
+static const unsigned AllowConfigure = IgnoreAll & ~JSPROP_IGNORE_PERMANENT;
+static const unsigned AllowEnumerate = IgnoreAll & ~JSPROP_IGNORE_ENUMERATE;
+static const unsigned AllowWritable = IgnoreAll & ~JSPROP_IGNORE_READONLY;
+static const unsigned ValueWithConfigurable = IgnoreWithValue & ~JSPROP_IGNORE_PERMANENT;
+
+static bool
+Getter(JSContext *cx, unsigned argc, JS::Value *vp)
+{
+ JS::CallArgs args = CallArgsFromVp(argc, vp);
+ args.rval().setBoolean(true);
+ return true;
+}
+
+static bool
+CheckDescriptor(JS::Handle<JSPropertyDescriptor> desc, bool enumerable,
+ bool writable, bool configurable)
+{
+ if (!desc.object())
+ return false;
+ if (desc.isEnumerable() != enumerable)
+ return false;
+ if (desc.isReadonly() == writable)
+ return false;
+ if (desc.isPermanent() == configurable)
+ return false;
+ return true;
+}
+
+BEGIN_TEST(testDefinePropertyIgnoredAttributes)
+{
+ JS::RootedObject obj(cx, JS_NewObject(cx, nullptr, JS::NullPtr(), JS::NullPtr()));
+ JS::Rooted<JSPropertyDescriptor> desc(cx);
+ JS::RootedValue defineValue(cx);
+
+ // Try a getter. Allow it to fill in the defaults.
+ CHECK(JS_DefineProperty(cx, obj, "foo", defineValue,
+ IgnoreAll | JSPROP_NATIVE_ACCESSORS, (JSPropertyOp)Getter));
+
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "foo", &desc));
+
+ // Note that since JSPROP_READONLY means nothing for accessor properties, we will actually
+ // claim to be writable, since the flag is not included in the mask.
+ CHECK(CheckDescriptor(desc, false, true, false));
+
+ // Install another configurable property, so we can futz with it.
+ CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
+ AllowConfigure | JSPROP_NATIVE_ACCESSORS, (JSPropertyOp)Getter));
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
+ CHECK(CheckDescriptor(desc, false, true, true));
+
+ // Rewrite the descriptor to now be enumerable, ensuring that the lack of
+ // configurablity stayed.
+ CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
+ AllowEnumerate |
+ JSPROP_ENUMERATE |
+ JSPROP_NATIVE_ACCESSORS,
+ (JSPropertyOp)Getter));
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
+ CHECK(CheckDescriptor(desc, true, true, true));
+
+ // Now try the same game with a value property
+ defineValue.setObject(*obj);
+ CHECK(JS_DefineProperty(cx, obj, "baz", defineValue, IgnoreWithValue));
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "baz", &desc));
+ CHECK(CheckDescriptor(desc, false, false, false));
+
+ // Now again with a configurable property
+ CHECK(JS_DefineProperty(cx, obj, "quox", defineValue, ValueWithConfigurable));
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "quox", &desc));
+ CHECK(CheckDescriptor(desc, false, false, true));
+
+ // Just make it writable. Leave the old value and everythign else alone.
+ defineValue.setUndefined();
+ CHECK(JS_DefineProperty(cx, obj, "quox", defineValue, AllowWritable));
+ CHECK(JS_GetPropertyDescriptor(cx, obj, "quox", &desc));
+ CHECK(CheckDescriptor(desc, false, true, true));
+ CHECK_SAME(ObjectValue(*obj), desc.value());
+
+ return true;
+}
+END_TEST(testDefinePropertyIgnoredAttributes)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -889,17 +889,22 @@ class MOZ_STACK_CLASS SourceBufferHolder
size_t length_;
bool ownsChars_;
};
} /* namespace JS */
/************************************************************************/
-/* Property attributes, set in JSPropertySpec and passed to API functions. */
+/* Property attributes, set in JSPropertySpec and passed to API functions.
+ *
+ * NB: The data structure in which some of these values are stored only uses
+ * a uint8_t to store the relevant information. Proceed with caution if
+ * trying to reorder or change the the first byte worth of flags.
+ */
#define JSPROP_ENUMERATE 0x01 /* property is visible to for/in loop */
#define JSPROP_READONLY 0x02 /* not settable: assignment is no-op.
This flag is only valid when neither
JSPROP_GETTER nor JSPROP_SETTER is
set. */
#define JSPROP_PERMANENT 0x04 /* property cannot be deleted */
#define JSPROP_NATIVE_ACCESSORS 0x08 /* set in JSPropertyDescriptor.flags
if getters/setters are JSNatives */
@@ -913,16 +918,28 @@ class MOZ_STACK_CLASS SourceBufferHolder
#define JSPROP_INDEX 0x80 /* name is actually (int) index */
#define JSFUN_STUB_GSOPS 0x200 /* use JS_PropertyStub getter/setter
instead of defaulting to class gsops
for property holding function */
#define JSFUN_CONSTRUCTOR 0x400 /* native that can be called as a ctor */
+#define JSPROP_IGNORE_ENUMERATE 0x1000 /* ignore the value in JSPROP_ENUMERATE.
+ This flag only valid when defining over
+ an existing property. */
+#define JSPROP_IGNORE_READONLY 0x2000 /* ignore the value in JSPROP_READONLY.
+ This flag only valid when defining over
+ an existing property. */
+#define JSPROP_IGNORE_PERMANENT 0x4000 /* ignore the value in JSPROP_PERMANENT.
+ This flag only valid when defining over
+ an existing property. */
+#define JSPROP_IGNORE_VALUE 0x8000 /* ignore the Value in the descriptor. Nothing was
+ specified when passed to Object.defineProperty
+ from script. */
/*
* Specify a generic native prototype methods, i.e., methods of a class
* prototype that are exposed as static methods taking an extra leading
* argument: the generic |this| parameter.
*
* If you set this flag in a JSFunctionSpec struct's flags initializer, then
* that struct must live at least as long as the native static method object
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -307,36 +307,48 @@ PropDesc::initFromPropertyDescriptor(Han
hasValue_ = false;
value_.setUndefined();
hasWritable_ = false;
} else {
hasGet_ = false;
get_.setUndefined();
hasSet_ = false;
set_.setUndefined();
- hasValue_ = true;
- value_ = desc.value();
- hasWritable_ = true;
- }
- hasEnumerable_ = true;
- hasConfigurable_ = true;
+ hasValue_ = !(desc.attributes() & JSPROP_IGNORE_VALUE);
+ value_ = hasValue_ ? desc.value() : UndefinedValue();
+ hasWritable_ = !(desc.attributes() & JSPROP_IGNORE_READONLY);
+ }
+ hasEnumerable_ = !(desc.attributes() & JSPROP_IGNORE_ENUMERATE);
+ hasConfigurable_ = !(desc.attributes() & JSPROP_IGNORE_PERMANENT);
}
void
PropDesc::populatePropertyDescriptor(HandleObject obj, MutableHandle<PropertyDescriptor> desc) const
{
if (isUndefined()) {
desc.object().set(nullptr);
return;
}
desc.value().set(hasValue() ? value() : UndefinedValue());
desc.setGetter(getter());
desc.setSetter(setter());
- desc.setAttributes(attributes());
+
+ // Make sure we honor the "has" notions in some way.
+ unsigned attrs = attributes();
+ if (!hasEnumerable())
+ attrs |= JSPROP_IGNORE_ENUMERATE;
+ if (!hasWritable())
+ attrs |= JSPROP_IGNORE_READONLY;
+ if (!hasConfigurable())
+ attrs |= JSPROP_IGNORE_PERMANENT;
+ if (!hasValue())
+ attrs |= JSPROP_IGNORE_VALUE;
+ desc.setAttributes(attrs);
+
desc.object().set(obj);
}
bool
PropDesc::makeObject(JSContext *cx, MutableHandleObject obj)
{
MOZ_ASSERT(!isUndefined());
@@ -649,33 +661,91 @@ Reject(JSContext *cx, HandleId id, unsig
{
if (throwError)
return Throw(cx, id, errorNumber);
*rval = false;
return true;
}
-// See comments on CheckDefineProperty in jsobj.h.
+static unsigned
+ApplyAttributes(unsigned attrs, bool enumerable, bool writable, bool configurable)
+{
+ /*
+ * Respect the fact that some callers may want to preserve existing attributes as much as
+ * possible, or install defaults otherwise.
+ */
+ if (attrs & JSPROP_IGNORE_ENUMERATE) {
+ attrs &= ~JSPROP_IGNORE_ENUMERATE;
+ if (enumerable)
+ attrs |= JSPROP_ENUMERATE;
+ else
+ attrs &= ~JSPROP_ENUMERATE;
+ }
+ if (attrs & JSPROP_IGNORE_READONLY) {
+ attrs &= ~JSPROP_IGNORE_READONLY;
+ // Only update the writability if it's relevant
+ if (!(attrs & (JSPROP_GETTER | JSPROP_SETTER))) {
+ if (!writable)
+ attrs |= JSPROP_READONLY;
+ else
+ attrs &= ~JSPROP_READONLY;
+ }
+ }
+ if (attrs & JSPROP_IGNORE_PERMANENT) {
+ attrs &= ~JSPROP_IGNORE_PERMANENT;
+ if (!configurable)
+ attrs |= JSPROP_PERMANENT;
+ else
+ attrs &= ~JSPROP_PERMANENT;
+ }
+ return attrs;
+}
+
+static unsigned
+ApplyOrDefaultAttributes(unsigned attrs, const Shape *shape = nullptr)
+{
+ bool enumerable = shape ? shape->enumerable() : false;
+ bool writable = shape ? shape->writable() : false;
+ bool configurable = shape ? shape->configurable() : false;
+ return ApplyAttributes(attrs, enumerable, writable, configurable);
+}
+
+static unsigned
+ApplyOrDefaultAttributes(unsigned attrs, Handle<PropertyDescriptor> desc)
+{
+ bool present = !!desc.object();
+ bool enumerable = present ? desc.isEnumerable() : false;
+ bool writable = present ? !desc.isReadonly() : false;
+ bool configurable = present ? !desc.isPermanent() : false;
+ return ApplyAttributes(attrs, enumerable, writable, configurable);
+}
+
+// See comments on CheckDefineProperty in jsfriendapi.h.
//
// DefinePropertyOnObject has its own implementation of these checks.
//
JS_FRIEND_API(bool)
js::CheckDefineProperty(JSContext *cx, HandleObject obj, HandleId id, HandleValue value,
unsigned attrs, PropertyOp getter, StrictPropertyOp setter)
{
if (!obj->isNative())
return true;
// ES5 8.12.9 Step 1. Even though we know obj is native, we use generic
// APIs for shorter, more readable code.
Rooted<PropertyDescriptor> desc(cx);
if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
return false;
+ // Appropriately handle the potential for ignored attributes. Since the proxy code calls us
+ // directly, these might flow in legitimately. Ensure that we compare against the values that
+ // are intended.
+ attrs = ApplyOrDefaultAttributes(attrs, desc) & ~JSPROP_IGNORE_VALUE;
+
// This does not have to check obj's extensibility when !desc.obj (steps
// 2-3) because the low-level methods JSObject::{add,put}Property check
// for that.
if (desc.object() && desc.isPermanent()) {
// Steps 6-11, skipping step 10.a.ii. Prohibit redefining a permanent
// property with different metadata, except to make a writable property
// non-writable.
if ((getter != desc.getter() && !(getter == JS_PropertyStub && !desc.getter())) ||
@@ -4066,78 +4136,120 @@ NativeLookupOwnProperty(ExclusiveContext
bool
js::DefineNativeProperty(ExclusiveContext *cx, HandleObject obj, HandleId id, HandleValue value,
PropertyOp getter, StrictPropertyOp setter, unsigned attrs)
{
JS_ASSERT(!(attrs & JSPROP_NATIVE_ACCESSORS));
AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
+ RootedShape shape(cx);
+ RootedValue updateValue(cx, value);
+ bool shouldDefine = true;
+
/*
* If defining a getter or setter, we must check for its counterpart and
* update the attributes and property ops. A getter or setter is really
* only half of a property.
*/
- RootedShape shape(cx);
if (attrs & (JSPROP_GETTER | JSPROP_SETTER)) {
- /*
- * If we are defining a getter whose setter was already defined, or
- * vice versa, finish the job via obj->changeProperty.
- */
if (!NativeLookupOwnProperty(cx, obj, id, &shape))
return false;
if (shape) {
+ /*
+ * If we are defining a getter whose setter was already defined, or
+ * vice versa, finish the job via obj->changeProperty.
+ */
if (IsImplicitDenseOrTypedArrayElement(shape)) {
if (obj->is<TypedArrayObject>()) {
/* Ignore getter/setter properties added to typed arrays. */
return true;
}
if (!JSObject::sparsifyDenseElement(cx, obj, JSID_TO_INT(id)))
return false;
shape = obj->nativeLookup(cx, id);
}
if (shape->isAccessorDescriptor()) {
+ attrs = ApplyOrDefaultAttributes(attrs, shape);
shape = JSObject::changeProperty<SequentialExecution>(cx, obj, shape, attrs,
JSPROP_GETTER | JSPROP_SETTER,
(attrs & JSPROP_GETTER)
? getter
: shape->getter(),
(attrs & JSPROP_SETTER)
? setter
: shape->setter());
if (!shape)
return false;
- } else {
- shape = nullptr;
+ shouldDefine = false;
}
}
+ } else if (!(attrs & JSPROP_IGNORE_VALUE)) {
+ /*
+ * We might still want to ignore redefining some of our attributes, if the
+ * request came through a proxy after Object.defineProperty(), but only if we're redefining
+ * a data property.
+ * FIXME: All this logic should be removed when Proxies use PropDesc, but we need to
+ * remove JSPropertyOp getters and setters first.
+ */
+ shape = obj->nativeLookup(cx, id);
+ if (shape && shape->isDataDescriptor())
+ attrs = ApplyOrDefaultAttributes(attrs, shape);
+ } else {
+ /*
+ * We have been asked merely to update some attributes by a caller of
+ * Object.defineProperty, laundered through the proxy system, and returning here. We can
+ * get away with just using JSObject::changeProperty here.
+ */
+ if (!NativeLookupOwnProperty(cx, obj, id, &shape))
+ return false;
+
+ if (shape) {
+ attrs = ApplyOrDefaultAttributes(attrs, shape);
+
+ /* Keep everything from the shape that isn't the things we're changing */
+ unsigned attrMask = ~(JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT);
+ shape = JSObject::changeProperty<SequentialExecution>(cx, obj, shape, attrs, attrMask,
+ shape->getter(), shape->setter());
+ if (!shape)
+ return false;
+ if (shape->hasSlot())
+ updateValue = obj->getSlot(shape->slot());
+ shouldDefine = false;
+ }
}
/*
* Purge the property cache of any properties named by id that are about
* to be shadowed in obj's scope chain.
*/
if (!PurgeScopeChain(cx, obj, id))
return false;
/* Use the object's class getter and setter by default. */
const Class *clasp = obj->getClass();
if (!getter && !(attrs & JSPROP_GETTER))
getter = clasp->getProperty;
if (!setter && !(attrs & JSPROP_SETTER))
setter = clasp->setProperty;
- if (!shape) {
+ if (shouldDefine) {
+ // Handle the default cases here. Anyone that wanted to set non-default attributes has
+ // cleared the IGNORE flags by now. Since we can never get here with JSPROP_IGNORE_VALUE
+ // relevant, just clear it.
+ attrs = ApplyOrDefaultAttributes(attrs) & ~JSPROP_IGNORE_VALUE;
return DefinePropertyOrElement<SequentialExecution>(cx, obj, id, getter, setter,
attrs, value, false, false);
}
- JS_ALWAYS_TRUE(UpdateShapeTypeAndValue<SequentialExecution>(cx, obj, shape, value));
-
- return CallAddPropertyHook<SequentialExecution>(cx, clasp, obj, shape, value);
+ MOZ_ASSERT(shape);
+
+ JS_ALWAYS_TRUE(UpdateShapeTypeAndValue<SequentialExecution>(cx, obj, shape, updateValue));
+
+ return CallAddPropertyHook<SequentialExecution>(cx, clasp, obj, shape, updateValue);
}
/*
* Call obj's resolve hook.
*
* cx, id, and flags are the parameters initially passed to the ongoing lookup;
* objp and propp are its out parameters. obj is an object along the prototype
* chain from where the lookup started.
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Proxy/regress-bug1037770.js
@@ -0,0 +1,7 @@
+foo = 1;
+Object.defineProperty(this, "foo", {writable:false});
+foo = 2;
+assertEq(foo, 1);
+
+if (typeof reportCompare === "function")
+ reportCompare(true, true);