Skip to content

Commit c4129f9

Browse files
devsnekcodebytere
authored andcommitted
vm: allow proxy callbacks to throw
Fixes: #33806 PR-URL: #33808 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent f39ee7d commit c4129f9

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

src/node_contextify.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ void ContextifyContext::PropertySetterCallback(
419419
args.GetReturnValue().Set(false);
420420
}
421421

422-
ctx->sandbox()->Set(ctx->context(), property, value).Check();
422+
USE(ctx->sandbox()->Set(ctx->context(), property, value));
423423
}
424424

425425
// static
@@ -437,9 +437,10 @@ void ContextifyContext::PropertyDescriptorCallback(
437437
Local<Object> sandbox = ctx->sandbox();
438438

439439
if (sandbox->HasOwnProperty(context, property).FromMaybe(false)) {
440-
args.GetReturnValue().Set(
441-
sandbox->GetOwnPropertyDescriptor(context, property)
442-
.ToLocalChecked());
440+
Local<Value> desc;
441+
if (sandbox->GetOwnPropertyDescriptor(context, property).ToLocal(&desc)) {
442+
args.GetReturnValue().Set(desc);
443+
}
443444
}
444445
}
445446

@@ -482,8 +483,7 @@ void ContextifyContext::PropertyDefinerCallback(
482483
desc_for_sandbox->set_configurable(desc.configurable());
483484
}
484485
// Set the property on the sandbox.
485-
sandbox->DefineProperty(context, property, *desc_for_sandbox)
486-
.Check();
486+
USE(sandbox->DefineProperty(context, property, *desc_for_sandbox));
487487
};
488488

489489
if (desc.has_get() || desc.has_set()) {

test/parallel/test-vm-context-property-forwarding.js

+20
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,23 @@ assert.deepStrictEqual(pd_actual, pd_expected);
4343
assert.strictEqual(ctx2[1], 5);
4444
delete ctx2[1];
4545
assert.strictEqual(ctx2[1], undefined);
46+
47+
// https://github.com/nodejs/node/issues/33806
48+
{
49+
const ctx = vm.createContext();
50+
51+
Object.defineProperty(ctx, 'prop', {
52+
get() {
53+
return undefined;
54+
},
55+
set(val) {
56+
throw new Error('test error');
57+
},
58+
});
59+
60+
assert.throws(() => {
61+
vm.runInContext('prop = 42', ctx);
62+
}, {
63+
message: 'test error',
64+
});
65+
}

0 commit comments

Comments
 (0)