Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 1ab796f

Browse filesBrowse files
fhinkelcjihrig
authored andcommitted
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>
1 parent 27f92ef commit 1ab796f
Copy full SHA for 1ab796f

File tree

Expand file treeCollapse file tree

2 files changed

+40
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+40
-1
lines changed
Open diff view settings
Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+13-1Lines changed: 13 additions & 1 deletion
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

Collapse file
+27Lines changed: 27 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.