vm: add isContext; prevent double-contextifying

Previously, calling `vm.createContext(o)` repeatedly on the same `o`
would cause new C++ `ContextifyContext`s to be created and stored on
`o`, while the previous resident went off into leaked-memory limbo.
Now, repeatedly trying to contextify a sandbox will do nothing after
the first time.

To detect this, an independently-useful `vm.isContext(sandbox)` export
was added.

diff --git a/lib/vm.js b/lib/vm.js
index c71bb81..8fee40a 100644 (file)
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -28,6 +28,7 @@ var util = require('util');
 //   - runInThisContext()
 //   - runInContext(sandbox, [timeout])
 // - makeContext(sandbox)
+// - isContext(sandbox)
 // From this we build the entire documented API.
 
 Script.prototype.runInNewContext = function(initSandbox, timeout, disp) {
@@ -44,10 +45,11 @@ exports.createScript = function(code, filename, disp) {
 exports.createContext = function(initSandbox) {
   if (util.isUndefined(initSandbox)) {
     initSandbox = {};
+  } else if (binding.isContext(initSandbox)) {
+    return initSandbox;
   }
 
   binding.makeContext(initSandbox);
-
   return initSandbox;
 };
 
@@ -65,3 +67,5 @@ exports.runInThisContext = function(code, filename, timeout, disp) {
   var script = exports.createScript(code, filename, disp);
   return script.runInThisContext(timeout, disp);
 };
+
+exports.isContext = binding.isContext;

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 3f47f52..8d3b531 100644 (file)
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -119,6 +119,7 @@ class ContextifyContext {
     data_wrapper_ctor.Reset(node_isolate, function_template->GetFunction());
 
     NODE_SET_METHOD(target, "makeContext", MakeContext);
+    NODE_SET_METHOD(target, "isContext", IsContext);
   }
 
 
@@ -130,11 +131,31 @@ class ContextifyContext {
     }
     Local<Object> sandbox = args[0].As<Object>();
 
+    Local<String> hidden_name =
+        FIXED_ONE_BYTE_STRING(node_isolate, "_contextifyHidden");
+
+    // Don't allow contextifying a sandbox multiple times.
+    assert(sandbox->GetHiddenValue(hidden_name).IsEmpty());
+
     ContextifyContext* context = new ContextifyContext(sandbox);
     Local<External> hidden_context = External::New(context);
+    sandbox->SetHiddenValue(hidden_name, hidden_context);
+  }
+
+
+  static void IsContext(const FunctionCallbackInfo<Value>& args) {
+    HandleScope scope(node_isolate);
+
+    if (!args[0]->IsObject()) {
+      ThrowTypeError("sandbox must be an object");
+      return;
+    }
+    Local<Object> sandbox = args[0].As<Object>();
+
     Local<String> hidden_name =
         FIXED_ONE_BYTE_STRING(node_isolate, "_contextifyHidden");
-    sandbox->SetHiddenValue(hidden_name, hidden_context);
+
+    args.GetReturnValue().Set(!sandbox->GetHiddenValue(hidden_name).IsEmpty());
   }
 
 

diff --git a/test/simple/test-vm-create-context-arg.js b/test/simple/test-vm-create-context-arg.js
index 27cd5e1..8c49a37 100644 (file)
--- a/test/simple/test-vm-create-context-arg.js
+++ b/test/simple/test-vm-create-context-arg.js
@@ -25,9 +25,15 @@ var vm = require('vm');
 
 assert.throws(function() {
   var ctx = vm.createContext('string is not supported');
-});
+}, TypeError);
 
 assert.doesNotThrow(function() {
   var ctx = vm.createContext({ a: 1 });
   ctx = vm.createContext([0, 1, 2, 3]);
 });
+
+assert.doesNotThrow(function() {
+    var sandbox = {};
+    vm.createContext(sandbox);
+    vm.createContext(sandbox);
+});

diff --git a/test/simple/test-vm-is-context.js b/test/simple/test-vm-is-context.js
new file mode 100644 (file)
index 0000000..9ef2037
--- /dev/null
+++ b/test/simple/test-vm-is-context.js
@@ -0,0 +1,38 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var vm = require('vm');
+
+assert.throws(function() {
+  vm.isContext('string is not supported');
+}, TypeError);
+
+assert.strictEqual(vm.isContext({}), false);
+assert.strictEqual(vm.isContext([]), false);
+
+assert.strictEqual(vm.isContext(vm.createContext()), true);
+assert.strictEqual(vm.isContext(vm.createContext([])), true);
+
+var sandbox = { foo: 'bar' };
+vm.createContext(sandbox);
+assert.strictEqual(vm.isContext(sandbox), true);