Commit c4129f9
vm: allow proxy callbacks to throw
Fixes: #33806
PR-URL: #33808
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>1 parent f39ee7d commit c4129f9
2 files changed
Lines changed: 26 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -419,7 +419,7 @@ void ContextifyContext::PropertySetterCallback( | |||
| 419 | 419 | args.GetReturnValue().Set(false); | |
| 420 | 420 | } | |
| 421 | 421 | ||
| 422 | - ctx->sandbox()->Set(ctx->context(), property, value).Check(); | ||
| 422 | + USE(ctx->sandbox()->Set(ctx->context(), property, value)); | ||
| 423 | 423 | } | |
| 424 | 424 | ||
| 425 | 425 | // static | |
@@ -437,9 +437,10 @@ void ContextifyContext::PropertyDescriptorCallback( | |||
| 437 | 437 | Local<Object> sandbox = ctx->sandbox(); | |
| 438 | 438 | ||
| 439 | 439 | 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 | + } | ||
| 443 | 444 | } | |
| 444 | 445 | } | |
| 445 | 446 | ||
@@ -482,8 +483,7 @@ void ContextifyContext::PropertyDefinerCallback( | |||
| 482 | 483 | desc_for_sandbox->set_configurable(desc.configurable()); | |
| 483 | 484 | } | |
| 484 | 485 | // 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)); | ||
| 487 | 487 | }; | |
| 488 | 488 | ||
| 489 | 489 | if (desc.has_get() || desc.has_set()) { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -43,3 +43,23 @@ assert.deepStrictEqual(pd_actual, pd_expected); | |||
| 43 | 43 | assert.strictEqual(ctx2[1], 5); | |
| 44 | 44 | delete ctx2[1]; | |
| 45 | 45 | 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