From 55c65cc2d55b70def061da0c8fd38f08b2929433 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 24 Sep 2010 17:02:08 +0200 Subject: 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 --- src/node.cc | 16 ++++++++++++++++ src/node.h | 9 +++++++++ src/node_buffer.cc | 12 +++--------- src/node_cares.cc | 4 ++++ src/node_idle_watcher.cc | 4 ++++ src/node_io_watcher.cc | 4 ++++ src/node_script.cc | 4 ++++ src/node_signal_watcher.cc | 4 ++++ src/node_stat_watcher.cc | 4 ++++ src/node_timer.cc | 4 ++++ 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 ErrnoException(int errorno, } +Handle FromConstructorTemplate(Persistent& t, + const Arguments& args) { + HandleScope scope; + + const int argc = args.Length(); + Local argv[argc]; + + for (int i = 0; i < argc; ++i) { + argv[i] = args[i]; + } + + Local instance = t->GetFunction()->NewInstance(argc, argv); + return scope.Close(instance); +} + + enum encoding ParseEncoding(Handle 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 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 FromConstructorTemplate(v8::Persistent& constructorTemplate, const v8::Arguments& args); + + static inline v8::Persistent* cb_persist( const v8::Local &v) { v8::Persistent *fn = new v8::Persistent(); 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 obj) { Handle Buffer::New(const Arguments &args) { - HandleScope scope; - if (!args.IsConstructCall()) { - Local argv[10]; - for (int i = 0; i < MIN(args.Length(), 10); i++) { - argv[i] = args[i]; - } - Local 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 target) { Handle 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 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 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 target) { Handle 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 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 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 Timer::New (const Arguments& args) { + if (!args.IsConstructCall()) { + return FromConstructorTemplate(constructor_template, args); + } + HandleScope scope; Timer *t = new Timer(); -- cgit v1.2.1