Skip to content

Commit

Permalink
Fix crash on invalid date object (#317)
Browse files Browse the repository at this point in the history 
mini_racer converts JS objects to Ruby objects. Ruby's C API functions
call rb_raise() on invalid inputs, and rb_raise() in turn is a glorified
longjmp() that jumps up the stack.

Longjmp and C++ destructors do not play well together. It left V8 in an
undefined state, resulting in random "Disposing the isolate that is
entered by a thread" fatal errors.

Fixes: #316
  • Loading branch information
@bnoordhuis
bnoordhuis authored Sep 21, 2024
1 parent 4f552cf commit 5a03255
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 72 deletions.
166 changes: 94 additions & 72 deletions ext/mini_racer_extension/mini_racer_extension.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,6 @@ static VALUE convert_v8_to_ruby(Isolate* isolate, Local<Context> context,
return INT2FIX(value->Int32Value(context).ToChecked());
}

if (value->IsNumber()) {
return rb_float_new(value->NumberValue(context).ToChecked());
}

if (value->IsTrue()) {
return Qtrue;
}
Expand All @@ -598,81 +594,107 @@ static VALUE convert_v8_to_ruby(Isolate* isolate, Local<Context> context,
return Qfalse;
}

if (value->IsArray()) {
VALUE rb_array = rb_ary_new();
Local<Array> arr = Local<Array>::Cast(value);
for(uint32_t i=0; i < arr->Length(); i++) {
MaybeLocal<Value> element = arr->Get(context, i);
if (element.IsEmpty()) {
continue;
}
VALUE rb_elem = convert_v8_to_ruby(isolate, context, element.ToLocalChecked());
if (rb_funcall(rb_elem, rb_intern("class"), 0) == rb_cFailedV8Conversion) {
return rb_elem;
}
rb_ary_push(rb_array, rb_elem);
}
return rb_array;
}

if (value->IsFunction()){
return rb_funcall(rb_cJavaScriptFunction, rb_intern("new"), 0);
}

if (value->IsDate()){
double ts = Local<Date>::Cast(value)->ValueOf();
double secs = ts/1000;
long nanos = round((secs - floor(secs)) * 1000000);

return rb_time_new(secs, nanos);
}

if (value->IsObject()) {
VALUE rb_hash = rb_hash_new();
TryCatch trycatch(isolate);

Local<Object> object = value->ToObject(context).ToLocalChecked();
auto maybe_props = object->GetOwnPropertyNames(context);
if (!maybe_props.IsEmpty()) {
Local<Array> props = maybe_props.ToLocalChecked();
for(uint32_t i=0; i < props->Length(); i++) {
MaybeLocal<Value> key = props->Get(context, i);
if (key.IsEmpty()) {
return rb_funcall(rb_cFailedV8Conversion, rb_intern("new"), 1, rb_str_new2(""));
}
VALUE rb_key = convert_v8_to_ruby(isolate, context, key.ToLocalChecked());

MaybeLocal<Value> prop_value = object->Get(context, key.ToLocalChecked());
// this may have failed due to Get raising
if (prop_value.IsEmpty() || trycatch.HasCaught()) {
return new_empty_failed_conv_obj();
}

VALUE rb_value = convert_v8_to_ruby(
isolate, context, prop_value.ToLocalChecked());
rb_hash_aset(rb_hash, rb_key, rb_value);
struct State {
Isolate* isolate;
Local<Value> value;
Local<Context> context;
} state = {isolate, value, context};

// calls to rb_*() functions can raise exceptions and longjmp,
// and therefore must be inside this protected block
auto can_raise = [](VALUE arg) -> VALUE {
State* state =
reinterpret_cast<State*>(static_cast<uintptr_t>(RB_NUM2ULL(arg)));
Isolate* isolate = state->isolate;
Local<Value> value = state->value;
Local<Context> context = state->context;

if (value->IsNumber()) {
return rb_float_new(value->NumberValue(context).ToChecked());
}

if (value->IsArray()) {
VALUE rb_array = rb_ary_new();
Local<Array> arr = Local<Array>::Cast(value);
for(uint32_t i = 0; i < arr->Length(); i++) {
MaybeLocal<Value> element = arr->Get(context, i);
if (element.IsEmpty()) {
continue;
}
VALUE rb_elem = convert_v8_to_ruby(isolate, context, element.ToLocalChecked());
if (rb_funcall(rb_elem, rb_intern("class"), 0) == rb_cFailedV8Conversion) {
return rb_elem;
}
rb_ary_push(rb_array, rb_elem);
}
return rb_array;
}
return rb_hash;
}

if (value->IsSymbol()) {
v8::String::Utf8Value symbol_name(isolate,
Local<Symbol>::Cast(value)->Description(isolate));
if (value->IsFunction()){
return rb_funcall(rb_cJavaScriptFunction, rb_intern("new"), 0);
}

VALUE str_symbol = rb_utf8_str_new(*symbol_name, symbol_name.length());
if (value->IsDate()){
double ts = Local<Date>::Cast(value)->ValueOf();
double secs = ts/1000;
long nanos = round((secs - floor(secs)) * 1000000);

return rb_str_intern(str_symbol);
}
return rb_time_new(secs, nanos);
}

MaybeLocal<String> rstr_maybe = value->ToString(context);
if (value->IsObject()) {
VALUE rb_hash = rb_hash_new();
TryCatch trycatch(isolate);

Local<Object> object = value->ToObject(context).ToLocalChecked();
auto maybe_props = object->GetOwnPropertyNames(context);
if (!maybe_props.IsEmpty()) {
Local<Array> props = maybe_props.ToLocalChecked();
for (uint32_t i = 0; i < props->Length(); i++) {
MaybeLocal<Value> key = props->Get(context, i);
if (key.IsEmpty()) {
return rb_funcall(rb_cFailedV8Conversion, rb_intern("new"), 1, rb_str_new2(""));
}
VALUE rb_key = convert_v8_to_ruby(isolate, context, key.ToLocalChecked());

if (rstr_maybe.IsEmpty()) {
return Qnil;
} else {
Local<String> rstr = rstr_maybe.ToLocalChecked();
return rb_utf8_str_new(*String::Utf8Value(isolate, rstr), rstr->Utf8Length(isolate));
}
MaybeLocal<Value> prop_value = object->Get(context, key.ToLocalChecked());
// this may have failed due to Get raising
if (prop_value.IsEmpty() || trycatch.HasCaught()) {
return new_empty_failed_conv_obj();
}

VALUE rb_value = convert_v8_to_ruby(
isolate, context, prop_value.ToLocalChecked());
rb_hash_aset(rb_hash, rb_key, rb_value);
}
}
return rb_hash;
}

if (value->IsSymbol()) {
v8::String::Utf8Value symbol_name(isolate,
Local<Symbol>::Cast(value)->Description(isolate));

VALUE str_symbol = rb_utf8_str_new(*symbol_name, symbol_name.length());

return rb_str_intern(str_symbol);
}

MaybeLocal<String> rstr_maybe = value->ToString(context);

if (rstr_maybe.IsEmpty()) {
return Qnil;
} else {
Local<String> rstr = rstr_maybe.ToLocalChecked();
return rb_utf8_str_new(*String::Utf8Value(isolate, rstr), rstr->Utf8Length(isolate));
}
};

// this is kind of slow because RB_ULL2NUM allocs when the pointer
// doesn't fit in a fixnum but yolo'ing and reinterpret_casting the
// pointer to a VALUE is probably not very sound
VALUE arg = RB_ULL2NUM(reinterpret_cast<uintptr_t>(&state));
return rb_protect(can_raise, arg, nullptr);
}

static VALUE convert_v8_to_ruby(Isolate* isolate,
Expand Down
10 changes: 10 additions & 0 deletions test/mini_racer_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,16 @@ def test_return_hash
)
end

def test_date_nan
# NoMethodError: undefined method `source_location' for "<internal:core>
# core/float.rb:114:in `to_i'":Thread::Backtrace::Location
if RUBY_ENGINE == "truffleruby"
skip "TruffleRuby bug"
end
context = MiniRacer::Context.new
context.eval("new Date(NaN)") # should not crash process
end

def test_return_date
context = MiniRacer::Context.new
test_time = Time.new
Expand Down

0 comments on commit 5a03255

Please sign in to comment.