diff --git a/quickjs.c b/quickjs.c index 8f118a9b2..81c8bccd3 100644 --- a/quickjs.c +++ b/quickjs.c @@ -41246,38 +41246,64 @@ static __exception int JS_ObjectDefineProperties(JSContext *ctx, JSValue props, desc; JSObject *p; JSPropertyEnum *atoms; - uint32_t len, i; - int ret = -1; + struct { + JSAtom atom; /* owned by atoms */ + JSPropertyDescriptor desc; + } *tab; + uint32_t len, i, count; + int ret = -1, res, desc_flags; if (!JS_IsObject(obj)) { JS_ThrowTypeError(ctx, "Object.defineProperties called on non-object"); return -1; } - desc = JS_UNDEFINED; props = JS_ToObject(ctx, properties); if (JS_IsException(props)) return -1; p = JS_VALUE_GET_OBJ(props); - if (JS_GetOwnPropertyNamesInternal(ctx, &atoms, &len, p, JS_GPN_ENUM_ONLY | JS_GPN_STRING_MASK | JS_GPN_SYMBOL_MASK) < 0) + tab = NULL; + count = 0; + if (JS_GetOwnPropertyNamesInternal(ctx, &atoms, &len, p, JS_GPN_STRING_MASK | JS_GPN_SYMBOL_MASK) < 0) goto exception; - // XXX: ECMA specifies that all descriptions should be validated before - // modifying the object. This would require allocating an array - // JSPropertyDescriptor and use 2 separate loops. + if (len > 0) { + tab = js_malloc(ctx, sizeof(tab[0]) * len); + if (!tab) + goto exception; + } + /* ECMA-262 ObjectDefineProperties: look up each key and read its + descriptor before the next key, and define the properties only + once all the descriptors are valid. */ for(i = 0; i < len; i++) { - JS_FreeValue(ctx, desc); + res = JS_GetOwnPropertyFlagsInternal(ctx, &desc_flags, p, atoms[i].atom); + if (res < 0) + goto exception; + if (!res || !(desc_flags & JS_PROP_ENUMERABLE)) + continue; desc = JS_GetProperty(ctx, props, atoms[i].atom); if (JS_IsException(desc)) goto exception; - if (JS_DefinePropertyDesc(ctx, obj, atoms[i].atom, desc, - JS_PROP_THROW | JS_PROP_DEFINE_PROPERTY) < 0) + res = js_obj_to_desc(ctx, &tab[count].desc, desc); + JS_FreeValue(ctx, desc); + if (res < 0) + goto exception; + tab[count].atom = atoms[i].atom; + count++; + } + for(i = 0; i < count; i++) { + if (JS_DefineProperty(ctx, obj, tab[i].atom, tab[i].desc.value, + tab[i].desc.getter, tab[i].desc.setter, + tab[i].desc.flags | JS_PROP_THROW | + JS_PROP_DEFINE_PROPERTY) < 0) goto exception; } ret = 0; exception: + for(i = 0; i < count; i++) + js_free_desc(ctx, &tab[i].desc); + js_free(ctx, tab); js_free_prop_enum(ctx, atoms, len); JS_FreeValue(ctx, props); - JS_FreeValue(ctx, desc); return ret; } diff --git a/tests/bug1794.js b/tests/bug1794.js new file mode 100644 index 000000000..c3f19e568 --- /dev/null +++ b/tests/bug1794.js @@ -0,0 +1,53 @@ +import { assert, assertThrows } from "./assert.js"; + +// Each key is checked for enumerability and its descriptor read before +// the next key is looked at. +let log = []; +const props = new Proxy({ a: { value: 1 }, b: { value: 2 } }, { + ownKeys(t) { log.push("ownKeys"); return Reflect.ownKeys(t); }, + getOwnPropertyDescriptor(t, k) { log.push("gopd " + k); return Reflect.getOwnPropertyDescriptor(t, k); }, + get(t, k) { log.push("get " + k); return Reflect.get(t, k); }, +}); +let o = Object.defineProperties({}, props); +assert(log.join(), "ownKeys,gopd a,get a,gopd b,get b"); +assert(o.a, 1); +assert(o.b, 2); + +log = []; +o = Object.create(null, props); +assert(log.join(), "ownKeys,gopd a,get a,gopd b,get b"); +assert(o.a, 1); +assert(o.b, 2); + +// All descriptors are validated before any property is defined. +o = {}; +assertThrows(TypeError, () => Object.defineProperties(o, { a: { value: 1 }, b: { get: 1 } })); +assert("a" in o, false); +assertThrows(TypeError, () => Object.defineProperties(o, { a: { value: 1 }, b: 2 })); +assert("a" in o, false); + +// A getter that makes a later key non-enumerable or deletes it is seen. +const src = { + get a() { + Object.defineProperty(src, "b", { enumerable: false }); + delete src.c; + return { value: 1 }; + }, + b: { value: 2 }, + c: { value: 3 }, + d: { value: 4 }, +}; +o = Object.defineProperties({}, src); +assert(Object.keys(o).length, 0); +assert(o.a, 1); +assert("b" in o, false); +assert("c" in o, false); +assert(o.d, 4); + +// Non-enumerable and symbol keys. +const sym = Symbol("s"); +const src2 = { [sym]: { value: "s" } }; +Object.defineProperty(src2, "hidden", { value: { value: "h" }, enumerable: false }); +o = Object.defineProperties({}, src2); +assert(o[sym], "s"); +assert("hidden" in o, false);