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 <ofrobots@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

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');
+const assert = require('assert');
+const vm = require('vm');
+const ctx = 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: x is not defined/);
+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);