contextify: handle infinite recursion errors

Try to be consistent with v0.10 and emit "Maximum call stack size
reached", even if it happens when allocating context or doing other
stuff.

fix #7045

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index d17899e..40f2c17 100644 (file)
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -69,15 +69,23 @@ class ContextifyContext {
       : env_(env),
         sandbox_(env->isolate(), sandbox),
         context_(env->isolate(), CreateV8Context(env)),
       : env_(env),
         sandbox_(env->isolate(), sandbox),
         context_(env->isolate(), CreateV8Context(env)),

-        proxy_global_(env->isolate(), context()->Global()),

         // Wait for sandbox_, proxy_global_, and context_ to die
         // Wait for sandbox_, proxy_global_, and context_ to die

-        references_(3) {
+        references_(0) {

     sandbox_.MakeWeak(this, WeakCallback);
     sandbox_.MarkIndependent();
     sandbox_.MakeWeak(this, WeakCallback);
     sandbox_.MarkIndependent();

+    references_++;
+
+    // Allocation failure or maximum call stack size reached
+    if (context_.IsEmpty())
+      return;

     context_.MakeWeak(this, WeakCallback);
     context_.MarkIndependent();
     context_.MakeWeak(this, WeakCallback);
     context_.MarkIndependent();

+    references_++;
+
+    proxy_global_.Reset(env->isolate(), context()->Global());

     proxy_global_.MakeWeak(this, WeakCallback);
     proxy_global_.MarkIndependent();
     proxy_global_.MakeWeak(this, WeakCallback);
     proxy_global_.MarkIndependent();

+    references_++;

   }
 
 
   }
 
 


@@ -180,6 +188,9 @@ class ContextifyContext {
     HandleScope scope(node_isolate);
     Local<Object> wrapper =
         env->script_data_constructor_function()->NewInstance();
     HandleScope scope(node_isolate);
     Local<Object> wrapper =
         env->script_data_constructor_function()->NewInstance();

+    if (wrapper.IsEmpty())
+      return scope.Close(Handle<Value>());
+

     Wrap<ContextifyContext>(wrapper, this);
     return scope.Close(wrapper);
   }
     Wrap<ContextifyContext>(wrapper, this);
     return scope.Close(wrapper);
   }


@@ -232,7 +243,17 @@ class ContextifyContext {
     // Don't allow contextifying a sandbox multiple times.
     assert(sandbox->GetHiddenValue(hidden_name).IsEmpty());
 
     // Don't allow contextifying a sandbox multiple times.
     assert(sandbox->GetHiddenValue(hidden_name).IsEmpty());
 

+    TryCatch try_catch;

     ContextifyContext* context = new ContextifyContext(env, sandbox);
     ContextifyContext* context = new ContextifyContext(env, sandbox);

+
+    if (try_catch.HasCaught()) {
+      try_catch.ReThrow();
+      return;
+    }
+
+    if (context->context().IsEmpty())
+      return;
+

     Local<External> hidden_context = External::New(context);
     sandbox->SetHiddenValue(hidden_name, hidden_context);
   }
     Local<External> hidden_context = External::New(context);
     sandbox->SetHiddenValue(hidden_name, hidden_context);
   }


@@ -490,14 +511,18 @@ class ContextifyScript : public BaseObject {
           "sandbox argument must have been converted to a context.");
     }
 
           "sandbox argument must have been converted to a context.");
     }
 

+    if (contextify_context->context().IsEmpty())
+      return;
+

     // Do the eval within the context
     Context::Scope context_scope(contextify_context->context());
     // Do the eval within the context
     Context::Scope context_scope(contextify_context->context());

-    EvalMachine(contextify_context->env(),
-                timeout,
-                display_errors,
-                args,
-                try_catch);
-    contextify_context->CopyProperties();
+    if (EvalMachine(contextify_context->env(),
+                    timeout,
+                    display_errors,
+                    args,
+                    try_catch)) {
+      contextify_context->CopyProperties();
+    }

   }
 
   static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
   }
 
   static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,


@@ -565,14 +590,14 @@ class ContextifyScript : public BaseObject {
   }
 
 
   }
 
 

-  static void EvalMachine(Environment* env,
+  static bool EvalMachine(Environment* env,

                           const int64_t timeout,
                           const bool display_errors,
                           const FunctionCallbackInfo<Value>& args,
                           TryCatch& try_catch) {
     if (!ContextifyScript::InstanceOf(env, args.This())) {
                           const int64_t timeout,
                           const bool display_errors,
                           const FunctionCallbackInfo<Value>& args,
                           TryCatch& try_catch) {
     if (!ContextifyScript::InstanceOf(env, args.This())) {

-      return ThrowTypeError(
-          "Script methods can only be called on script instances.");
+      ThrowTypeError("Script methods can only be called on script instances.");
+      return false;

     }
 
     ContextifyScript* wrapped_script =
     }
 
     ContextifyScript* wrapped_script =


@@ -590,7 +615,8 @@ class ContextifyScript : public BaseObject {
 
     if (try_catch.HasCaught() && try_catch.HasTerminated()) {
       V8::CancelTerminateExecution(args.GetIsolate());
 
     if (try_catch.HasCaught() && try_catch.HasTerminated()) {
       V8::CancelTerminateExecution(args.GetIsolate());

-      return ThrowError("Script execution timed out.");
+      ThrowError("Script execution timed out.");
+      return false;

     }
 
     if (result.IsEmpty()) {
     }
 
     if (result.IsEmpty()) {


@@ -599,10 +625,11 @@ class ContextifyScript : public BaseObject {
         DisplayExceptionLine(try_catch.Message());
       }
       try_catch.ReThrow();
         DisplayExceptionLine(try_catch.Message());
       }
       try_catch.ReThrow();

-      return;
+      return false;

     }
 
     args.GetReturnValue().Set(result);
     }
 
     args.GetReturnValue().Set(result);

+    return true;

   }
 
 
   }
 
 

diff --git a/test/simple/test-vm-infinite-recursion.js b/test/simple/test-vm-infinite-recursion.js
new file mode 100644 (file)
index 0000000..29ad029
--- /dev/null
+++ b/test/simple/test-vm-infinite-recursion.js
@@ -0,0 +1,30 @@
+// 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.
+
+// Flags: --stack-size=128
+
+var assert = require('assert');
+var vm = require('vm');
+var s = 'vm.runInNewContext(s, { vm: vm, s: s })';
+
+assert.throws(function() {
+  eval(s);
+}, /Maximum call stack/);