From 4faec56b8aaf4750167c8883ead90244c1e17341 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Tue, 9 Jun 2020 09:25:39 -0500 Subject: [PATCH] vm: allow proxy callbacks to throw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/33806 PR-URL: https://github.com/nodejs/node/pull/33808 Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso Reviewed-By: James M Snell Reviewed-By: Gerhard Stöbich Reviewed-By: Colin Ihrig --- src/node_contextify.cc | 12 +++++------ .../test-vm-context-property-forwarding.js | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 69663afe423f58..536d761c637928 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -422,7 +422,7 @@ void ContextifyContext::PropertySetterCallback( args.GetReturnValue().Set(false); } - ctx->sandbox()->Set(ctx->context(), property, value).Check(); + USE(ctx->sandbox()->Set(ctx->context(), property, value)); } // static @@ -440,9 +440,10 @@ void ContextifyContext::PropertyDescriptorCallback( Local sandbox = ctx->sandbox(); if (sandbox->HasOwnProperty(context, property).FromMaybe(false)) { - args.GetReturnValue().Set( - sandbox->GetOwnPropertyDescriptor(context, property) - .ToLocalChecked()); + Local desc; + if (sandbox->GetOwnPropertyDescriptor(context, property).ToLocal(&desc)) { + args.GetReturnValue().Set(desc); + } } } @@ -485,8 +486,7 @@ void ContextifyContext::PropertyDefinerCallback( desc_for_sandbox->set_configurable(desc.configurable()); } // Set the property on the sandbox. - sandbox->DefineProperty(context, property, *desc_for_sandbox) - .Check(); + USE(sandbox->DefineProperty(context, property, *desc_for_sandbox)); }; if (desc.has_get() || desc.has_set()) { diff --git a/test/parallel/test-vm-context-property-forwarding.js b/test/parallel/test-vm-context-property-forwarding.js index 800a9fa2438988..53d38c1467a5a2 100644 --- a/test/parallel/test-vm-context-property-forwarding.js +++ b/test/parallel/test-vm-context-property-forwarding.js @@ -43,3 +43,23 @@ assert.deepStrictEqual(pd_actual, pd_expected); assert.strictEqual(ctx2[1], 5); delete ctx2[1]; assert.strictEqual(ctx2[1], undefined); + +// https://github.com/nodejs/node/issues/33806 +{ + const ctx = vm.createContext(); + + Object.defineProperty(ctx, 'prop', { + get() { + return undefined; + }, + set(val) { + throw new Error('test error'); + }, + }); + + assert.throws(() => { + vm.runInContext('prop = 42', ctx); + }, { + message: 'test error', + }); +}