Check target extensibility via IsExtensible() in the proxy invariant checks

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) ||

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"]);
+