diff --git a/quickjs.c b/quickjs.c index bbac00c33..0cfdca674 100644 --- a/quickjs.c +++ b/quickjs.c @@ -52224,7 +52224,7 @@ static int js_proxy_has(JSContext *ctx, JSValueConst obj, JSAtom atom) int res; JSObject *p; JSValueConst args[2]; - bool ret, res2; + bool ret; s = get_proxy_method(ctx, &method, obj, JS_ATOM_has); if (!s) @@ -52250,8 +52250,15 @@ static int js_proxy_has(JSContext *ctx, JSValueConst obj, JSAtom atom) if (res < 0) return -1; if (res) { - res2 = !(desc_flags & JS_PROP_CONFIGURABLE); - if (res2 || !p->extensible) { + if (!(desc_flags & JS_PROP_CONFIGURABLE)) + goto inconsistent; + /* must go through IsExtensible(): the target can be a proxy + itself, whose isExtensible trap is observable */ + res = JS_IsExtensible(ctx, s->target); + if (res < 0) + return -1; + if (!res) { + inconsistent: JS_ThrowTypeError(ctx, "proxy: inconsistent has"); return -1; } @@ -52447,7 +52454,14 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc js_free_desc(ctx, &target_desc); if (JS_IsUndefined(trap_result_obj)) { if (target_desc_ret) { - if (!(target_desc.flags & JS_PROP_CONFIGURABLE) || !p->extensible) + if (!(target_desc.flags & JS_PROP_CONFIGURABLE)) + goto fail; + /* must go through IsExtensible(): the target can be a proxy + itself, whose isExtensible trap is observable */ + res = JS_IsExtensible(ctx, s->target); + if (res < 0) + return -1; + if (!res) goto fail; } ret = false; @@ -52510,7 +52524,7 @@ static int js_proxy_define_own_property(JSContext *ctx, JSValueConst obj, { JSProxyData *s; JSValue method, ret1, prop_val, desc_val; - int res; + int res, extensible_target; JSObject *p; JSValueConst args[3]; JSPropertyDescriptor desc; @@ -52554,11 +52568,19 @@ static int js_proxy_define_own_property(JSContext *ctx, JSValueConst obj, res = JS_GetOwnPropertyInternal(ctx, &desc, p, prop); if (res < 0) return -1; + /* must go through IsExtensible(): the target can be a proxy itself, in + which case its isExtensible trap is observable and may throw */ + extensible_target = JS_IsExtensible(ctx, s->target); + if (extensible_target < 0) { + if (res) + js_free_desc(ctx, &desc); + return -1; + } setting_not_configurable = ((flags & (JS_PROP_HAS_CONFIGURABLE | JS_PROP_CONFIGURABLE)) == JS_PROP_HAS_CONFIGURABLE); if (!res) { - if (!p->extensible || setting_not_configurable) + if (!extensible_target || setting_not_configurable) goto fail; } else { if (!check_define_prop_flags(desc.flags, flags) || diff --git a/tests/bug1626.js b/tests/bug1626.js new file mode 100644 index 000000000..1f2371327 --- /dev/null +++ b/tests/bug1626.js @@ -0,0 +1,74 @@ +import { assert, assertArrayEquals, assertThrows } from "./assert.js"; + +/* The post-trap invariant checks of the proxy [[DefineOwnProperty]], + [[GetOwnProperty]] and [[HasProperty]] internal methods must obtain the + target's extensibility through IsExtensible(). When the target is itself a + proxy that is observable: its isExtensible trap runs, and it can throw. */ + +function innerProxy(target, isExtensible) { + return new Proxy(target, { isExtensible }); +} + +/* A non-callable isExtensible trap on the target proxy must surface as a + TypeError instead of being silently ignored. */ +assertThrows(TypeError, () => Reflect.defineProperty( + new Proxy(innerProxy({}, 0), { defineProperty: () => true }), "x", {})); + +/* IsExtensible(target) is performed whether or not the target already has + the property. */ +assertThrows(TypeError, () => Reflect.defineProperty( + new Proxy(innerProxy({ x: 1 }, 0), { defineProperty: () => true }), "x", {})); + +assertThrows(TypeError, () => Reflect.getOwnPropertyDescriptor( + new Proxy(innerProxy({ x: 1 }, 0), { getOwnPropertyDescriptor: () => undefined }), "x")); + +assertThrows(TypeError, () => Reflect.has( + new Proxy(innerProxy({ x: 1 }, 0), { has: () => false }), "x")); + +/* An isExtensible trap that lies about an extensible target is rejected. */ +assertThrows(TypeError, () => Reflect.defineProperty( + new Proxy(innerProxy({}, () => false), { defineProperty: () => true }), "x", {})); + +/* An exception thrown by the trap propagates unchanged. */ +for (const op of [ + (p) => Reflect.defineProperty(new Proxy(p, { defineProperty: () => true }), "x", {}), + (p) => Reflect.getOwnPropertyDescriptor(new Proxy(p, { getOwnPropertyDescriptor: () => undefined }), "x"), + (p) => Reflect.has(new Proxy(p, { has: () => false }), "x"), +]) { + assertThrows(RangeError, () => op(innerProxy({ x: 1 }, () => { throw new RangeError(); }))); +} + +/* A well-behaved target proxy still allows the operations to complete, and the + traps are called in the order the spec prescribes: [[GetOwnProperty]] on the + target first, then IsExtensible(target). */ +function trapLog(target, op, expected) { + const log = []; + const p = new Proxy(target, { + getOwnPropertyDescriptor(t, k) { log.push("gOPD"); return Reflect.getOwnPropertyDescriptor(t, k); }, + isExtensible(t) { log.push("isExtensible"); return Reflect.isExtensible(t); }, + }); + assert(op(p), expected); + return log; +} + +assertArrayEquals(trapLog({ x: 1 }, (p) => Reflect.defineProperty( + new Proxy(p, { defineProperty: () => true }), "x", { value: 2 }), true), ["gOPD", "isExtensible"]); + +assertArrayEquals(trapLog({ x: 1 }, (p) => Reflect.getOwnPropertyDescriptor( + new Proxy(p, { getOwnPropertyDescriptor: () => undefined }), "x"), undefined), ["gOPD", "isExtensible"]); + +assertArrayEquals(trapLog({ x: 1 }, (p) => Reflect.has( + new Proxy(p, { has: () => false }), "x"), false), ["gOPD", "isExtensible"]); + +/* When the property is absent from the target, the configurability check + cannot fail, so [[HasProperty]] and [[GetOwnProperty]] stop before + IsExtensible(). [[DefineOwnProperty]] always performs it. */ +assertArrayEquals(trapLog({}, (p) => Reflect.has( + new Proxy(p, { has: () => false }), "x"), false), ["gOPD"]); + +assertArrayEquals(trapLog({}, (p) => Reflect.getOwnPropertyDescriptor( + new Proxy(p, { getOwnPropertyDescriptor: () => undefined }), "x"), undefined), ["gOPD"]); + +assertArrayEquals(trapLog({}, (p) => Reflect.defineProperty( + new Proxy(p, { defineProperty: () => true }), "x", { value: 1 }), true), ["gOPD", "isExtensible"]); +