Safe constructor for ObjectWrapped classes

New() methods should be invoked as constructors, not regular functions.
Corner cases like Script::New() may cause a SIGSEGV when the GC is run.

More details: http://groups.google.com/group/nodejs/browse_thread/thread/a7e5db68d4cd6356

10 files changed, 56 insertions, 9 deletions

diff --git a/src/node.cc b/src/node.cc
index f4b0820fe..8f13c64b6 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -787,6 +787,22 @@ Local<Value> ErrnoException(int errorno,
 }
 
 
+Handle<Value> FromConstructorTemplate(Persistent<FunctionTemplate>& t,
+                                      const Arguments& args) {
+  HandleScope scope;
+
+  const int argc = args.Length();
+  Local<Value> argv[argc];
+
+  for (int i = 0; i < argc; ++i) {
+    argv[i] = args[i];
+  }
+
+  Local<Object> instance = t->GetFunction()->NewInstance(argc, argv);
+  return scope.Close(instance);
+}
+
+
 enum encoding ParseEncoding(Handle<Value> encoding_v, enum encoding _default) {
   HandleScope scope;
 
diff --git a/src/node.h b/src/node.h
index c73df2abf..07b7f5e17 100644
--- a/src/node.h
+++ b/src/node.h
@@ -65,6 +65,15 @@ ssize_t DecodeWrite(char *buf,
 v8::Local<v8::Object> BuildStatsObject(struct stat * s);
 
 
+/**
+ * Call this when your constructor is invoked as a regular function, e.g. Buffer(10) instead of new Buffer(10).
+ * @param constructorTemplate Constructor template to instantiate from.
+ * @param args The arguments object passed to your constructor.
+ * @see v8::Arguments::IsConstructCall
+ */
+v8::Handle<v8::Value> FromConstructorTemplate(v8::Persistent<v8::FunctionTemplate>& constructorTemplate, const v8::Arguments& args);
+
+
 static inline v8::Persistent<v8::Function>* cb_persist(
     const v8::Local<v8::Value> &v) {
   v8::Persistent<v8::Function> *fn = new v8::Persistent<v8::Function>();
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 10ddbaf72..434bc547a 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -127,18 +127,12 @@ size_t Buffer::Length(Handle<Object> obj) {
 
 
 Handle<Value> Buffer::New(const Arguments &args) {
-  HandleScope scope;
-
   if (!args.IsConstructCall()) {
-    Local<Value> argv[10];
-    for (int i = 0; i < MIN(args.Length(), 10); i++) {
-      argv[i] = args[i];
-    }
-    Local<Object> instance =
-      constructor_template->GetFunction()->NewInstance(args.Length(), argv);
-    return scope.Close(instance);
+    return FromConstructorTemplate(constructor_template, args);
   }
 
+  HandleScope scope;
+
   Buffer *buffer;
   if (args[0]->IsInt32()) {
     // var buffer = new Buffer(1024);
diff --git a/src/node_cares.cc b/src/node_cares.cc
index 4bd07f8d6..ea227a884 100644
--- a/src/node_cares.cc
+++ b/src/node_cares.cc
@@ -459,6 +459,10 @@ void Channel::Initialize(Handle<Object> target) {
 
 
 Handle<Value> Channel::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   struct ares_options options;
diff --git a/src/node_idle_watcher.cc b/src/node_idle_watcher.cc
index 851a6f823..9ae1c4bc5 100644
--- a/src/node_idle_watcher.cc
+++ b/src/node_idle_watcher.cc
@@ -77,6 +77,10 @@ void IdleWatcher::Callback(EV_P_ ev_idle *w, int revents) {
 //  idle.start();
 //
 Handle<Value> IdleWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   IdleWatcher *s = new IdleWatcher();
diff --git a/src/node_io_watcher.cc b/src/node_io_watcher.cc
index 3d519d12f..d0d83a0b1 100644
--- a/src/node_io_watcher.cc
+++ b/src/node_io_watcher.cc
@@ -68,6 +68,10 @@ void IOWatcher::Callback(EV_P_ ev_io *w, int revents) {
 //  io.start();
 //
 Handle<Value> IOWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
   IOWatcher *s = new IOWatcher();
   s->Wrap(args.This());
diff --git a/src/node_script.cc b/src/node_script.cc
index d9631ec2f..cfcb205f2 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -78,6 +78,10 @@ void node::Script::Initialize (Handle<Object> target) {
 
 
 Handle<Value> node::Script::New (const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   node::Script *t = new node::Script();
diff --git a/src/node_signal_watcher.cc b/src/node_signal_watcher.cc
index 305c06a88..d593c945b 100644
--- a/src/node_signal_watcher.cc
+++ b/src/node_signal_watcher.cc
@@ -51,6 +51,10 @@ void SignalWatcher::Callback(EV_P_ ev_signal *watcher, int revents) {
 }
 
 Handle<Value> SignalWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   if (args.Length() != 1 || !args[0]->IsInt32()) {
diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc
index 88ae9c694..bbc4fdfbd 100644
--- a/src/node_stat_watcher.cc
+++ b/src/node_stat_watcher.cc
@@ -46,6 +46,10 @@ void StatWatcher::Callback(EV_P_ ev_stat *watcher, int revents) {
 
 
 Handle<Value> StatWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
   StatWatcher *s = new StatWatcher();
   s->Wrap(args.Holder());
diff --git a/src/node_timer.cc b/src/node_timer.cc
index bee1aace0..5918feb6b 100644
--- a/src/node_timer.cc
+++ b/src/node_timer.cc
@@ -97,6 +97,10 @@ Timer::~Timer ()
 Handle<Value>
 Timer::New (const Arguments& args)
 {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   Timer *t = new Timer();