Skip to content

Commit 1ab796f

Browse files
fhinkelcjihrig
authored andcommittedAug 10, 2016
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]>
1 parent 27f92ef commit 1ab796f

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed
 

‎src/node_contextify.cc

+13-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,19 @@ class ContextifyContext {
380380
if (ctx->context_.IsEmpty())
381381
return;
382382

383-
ctx->sandbox()->Set(property, value);
383+
bool is_declared =
384+
ctx->global_proxy()->HasRealNamedProperty(ctx->context(),
385+
property).FromJust();
386+
bool is_contextual_store = ctx->global_proxy() != args.This();
387+
388+
bool set_property_will_throw =
389+
args.ShouldThrowOnError() &&
390+
!is_declared &&
391+
is_contextual_store;
392+
393+
if (!set_property_will_throw) {
394+
ctx->sandbox()->Set(property, value);
395+
}
384396
}
385397

386398

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

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const vm = require('vm');
5+
const ctx = vm.createContext();
6+
7+
// Test strict mode inside a vm script, i.e., using an undefined variable
8+
// throws a ReferenceError. Also check that variables
9+
// that are not successfully set in the vm, must not be set
10+
// on the sandboxed context.
11+
12+
vm.runInContext('w = 1;', ctx);
13+
assert.strictEqual(1, ctx.w);
14+
15+
assert.throws(function() { vm.runInContext('"use strict"; x = 1;', ctx); },
16+
/ReferenceError: x is not defined/);
17+
assert.strictEqual(undefined, ctx.x);
18+
19+
vm.runInContext('"use strict"; var y = 1;', ctx);
20+
assert.strictEqual(1, ctx.y);
21+
22+
vm.runInContext('"use strict"; this.z = 1;', ctx);
23+
assert.strictEqual(1, ctx.z);
24+
25+
// w has been defined
26+
vm.runInContext('"use strict"; w = 2;', ctx);
27+
assert.strictEqual(2, ctx.w);

0 commit comments

Comments
 (0)
Please sign in to comment.