From 2f88de1ce3b09631e6b4babe0f16ca266f19ca1c Mon Sep 17 00:00:00 2001 From: AnnaMag Date: Sun, 26 Mar 2017 18:53:26 +0100 Subject: [PATCH] vm: use SetterCallback to set func declarations Currently, when in strict mode, function declarations are copied on the sandbox by CopyProperties(), which is not necessary and will break when CP is removed. This change maintains current behavior, letting GlobalPropertySetterCallback copy functions on the sandbox instead of using CP to do the task. PR-URL: https://github.com/nodejs/node/pull/12051 Reviewed-By: Franziska Hinkelmann Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen --- src/node_contextify.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 456c273b2b6d9b..3a0d4d3a1fd642 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -411,7 +411,17 @@ class ContextifyContext { // false for vmResult.x = 5 where vmResult = vm.runInContext(); bool is_contextual_store = ctx->global_proxy() != args.This(); - if (!is_declared && args.ShouldThrowOnError() && is_contextual_store) + // Indicator to not return before setting (undeclared) function declarations + // on the sandbox in strict mode, i.e. args.ShouldThrowOnError() = true. + // True for 'function f() {}', 'this.f = function() {}', + // 'var f = function()'. + // In effect only for 'function f() {}' because + // var f = function(), is_declared = true + // this.f = function() {}, is_contextual_store = false. + bool is_function = value->IsFunction(); + + if (!is_declared && args.ShouldThrowOnError() && is_contextual_store && + !is_function) return; ctx->sandbox()->Set(property, value);