Permalink
Browse files

Verify that the argument passed to vm.runInContext() is a context obj…

…ect.

Fixes #558.
  • Loading branch information...
1 parent b74d119 commit f08720606757577d95bd09b48697c7decbf17f00 @bnoordhuis bnoordhuis committed Jul 5, 2011
Showing with 21 additions and 2 deletions.
  1. +9 −1 src/node_script.cc
  2. +12 −1 test/simple/test-script-context.js
View
@@ -50,6 +50,7 @@ class WrappedContext : ObjectWrap {
Persistent<Context> GetV8Context();
static Local<Object> NewInstance();
+ static bool InstanceOf(Handle<Value> value);
protected:
@@ -110,6 +111,11 @@ void WrappedContext::Initialize(Handle<Object> target) {
}
+bool WrappedContext::InstanceOf(Handle<Value> value) {
+ return !value.IsEmpty() && constructor_template->HasInstance(value);
+}
+
+
Handle<Value> WrappedContext::New(const Arguments& args) {
HandleScope scope;
@@ -282,7 +288,9 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
}
const int sandbox_index = input_flag == compileCode ? 1 : 0;
- if (context_flag == userContext && args.Length() < (sandbox_index + 1)) {
+ if (context_flag == userContext
+ && !WrappedContext::InstanceOf(args[sandbox_index]))
+ {
return ThrowException(Exception::TypeError(
String::New("needs a 'context' argument.")));
}
@@ -22,7 +22,8 @@
var common = require('../common');
var assert = require('assert');
-var Script = require('vm').Script;
+var vm = require('vm');
+var Script = vm.Script;
var script = new Script('"passed";');
common.debug('run in a new empty context');
@@ -44,3 +45,13 @@ assert.equal('lala', context.thing);
// Issue GH-227:
Script.runInNewContext('', null, 'some.js');
+
+// GH-558, non-context argument segfaults / raises assertion
+function isTypeError(o) {
+ return o instanceof TypeError;
+}
+
+[undefined, null, 0, 0.0, '', {}, []].forEach(function(e) {
+ assert.throws(function() { script.runInContext(e); }, isTypeError);
+ assert.throws(function() { vm.runInContext('', e); }, isTypeError);
+});

0 comments on commit f087206

Please sign in to comment.