Skip to content

Commit f087206

Browse files
committed
Verify that the argument passed to vm.runInContext() is a context object.
Fixes #558.
1 parent b74d119 commit f087206

2 files changed

Lines changed: 21 additions & 2 deletions

File tree

src/node_script.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class WrappedContext : ObjectWrap {
5050

5151
Persistent<Context> GetV8Context();
5252
static Local<Object> NewInstance();
53+
static bool InstanceOf(Handle<Value> value);
5354

5455
protected:
5556

@@ -110,6 +111,11 @@ void WrappedContext::Initialize(Handle<Object> target) {
110111
}
111112

112113

114+
bool WrappedContext::InstanceOf(Handle<Value> value) {
115+
return !value.IsEmpty() && constructor_template->HasInstance(value);
116+
}
117+
118+
113119
Handle<Value> WrappedContext::New(const Arguments& args) {
114120
HandleScope scope;
115121

@@ -282,7 +288,9 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
282288
}
283289

284290
const int sandbox_index = input_flag == compileCode ? 1 : 0;
285-
if (context_flag == userContext && args.Length() < (sandbox_index + 1)) {
291+
if (context_flag == userContext
292+
&& !WrappedContext::InstanceOf(args[sandbox_index]))
293+
{
286294
return ThrowException(Exception::TypeError(
287295
String::New("needs a 'context' argument.")));
288296
}

test/simple/test-script-context.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
var common = require('../common');
2323
var assert = require('assert');
2424

25-
var Script = require('vm').Script;
25+
var vm = require('vm');
26+
var Script = vm.Script;
2627
var script = new Script('"passed";');
2728

2829
common.debug('run in a new empty context');
@@ -44,3 +45,13 @@ assert.equal('lala', context.thing);
4445

4546
// Issue GH-227:
4647
Script.runInNewContext('', null, 'some.js');
48+
49+
// GH-558, non-context argument segfaults / raises assertion
50+
function isTypeError(o) {
51+
return o instanceof TypeError;
52+
}
53+
54+
[undefined, null, 0, 0.0, '', {}, []].forEach(function(e) {
55+
assert.throws(function() { script.runInContext(e); }, isTypeError);
56+
assert.throws(function() { vm.runInContext('', e); }, isTypeError);
57+
});

0 commit comments

Comments
 (0)