src: do not copy on failing setProperty()

In vm, the setter interceptor should not copy a value onto the sandbox, if setting it on the global object will fail. It will fail if we are in strict mode and set a value without declaring it. Fixes: #5344 PR-URL: #7908 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

Author: fhinkel

src/node_contextify.cc

@@ -380,7 +380,19 @@ class ContextifyContext{
 if (ctx->context_.IsEmpty())
 return;
 
- ctx->sandbox()->Set(property, value);
+bool is_declared =
+ ctx->global_proxy()->HasRealNamedProperty(ctx->context(),
+ property).FromJust();
+bool is_contextual_store = ctx->global_proxy() != args.This();
+
+bool set_property_will_throw =
+ args.ShouldThrowOnError() &&
+ !is_declared &&
+ is_contextual_store;
+
+if (!set_property_will_throw){
+ ctx->sandbox()->Set(property, value);
+ }
  }
 
 

test/parallel/test-vm-strict-mode.js

@@ -0,0 +1,27 @@
+'use strict';
+require('../common');
+constassert=require('assert');
+constvm=require('vm');
+constctx=vm.createContext();
+
+// Test strict mode inside a vm script, i.e., using an undefined variable
+// throws a ReferenceError. Also check that variables
+// that are not successfully set in the vm, must not be set
+// on the sandboxed context.
+
+vm.runInContext('w = 1;',ctx);
+assert.strictEqual(1,ctx.w);
+
+assert.throws(function(){vm.runInContext('"use strict" x = 1;',ctx);},
+/ReferenceError:xisnotdefined/);
+assert.strictEqual(undefined,ctx.x);
+
+vm.runInContext('"use strict" var y = 1;',ctx);
+assert.strictEqual(1,ctx.y);
+
+vm.runInContext('"use strict" this.z = 1;',ctx);
+assert.strictEqual(1,ctx.z);
+
+// w has been defined
+vm.runInContext('"use strict" w = 2;',ctx);
+assert.strictEqual(2,ctx.w);