Skip to content

Commit

Permalink
Handle DataCloneError serialization errors (#330)
Browse files Browse the repository at this point in the history 
Instead of trying to detect upfront if an object can be serialized,
just do it and see if it throws a DataCloneError.

Unexpected DataCloneErrors were of course already being handled but
the new approach does it in a centralized and generic manner.

Failed serialization now passes an object with a single "error" string
property back to Ruby land.

Fixes the failing test from #329
  • Loading branch information
@bnoordhuis
bnoordhuis authored Jan 9, 2025
1 parent 5fad3b6 commit 394980b
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 87 deletions.
118 changes: 50 additions & 68 deletions ext/mini_racer_extension/mini_racer_v8.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ struct State
v8::Local<v8::Context> safe_context;
v8::Persistent<v8::Context> persistent_context; // single-thread mode only
v8::Persistent<v8::Context> persistent_safe_context; // single-thread mode only
v8::Persistent<v8::Object> persistent_webassembly_instance;
v8::Local<v8::Object> webassembly_instance;
Context *ruby_context;
int64_t max_memory;
int err_reason;
Expand Down Expand Up @@ -81,21 +79,54 @@ bool reply(State& st, v8::Local<v8::Value> v)
return serialized.data != nullptr; // exception pending if false
}

bool reply(State& st, v8::Local<v8::Value> result, v8::Local<v8::Value> err)
{
v8::TryCatch try_catch(st.isolate);
try_catch.SetVerbose(st.verbose_exceptions);
v8::Local<v8::Array> response;
{
v8::Context::Scope context_scope(st.safe_context);
response = v8::Array::New(st.isolate, 2);
}
response->Set(st.context, 0, result).Check();
response->Set(st.context, 1, err).Check();
if (reply(st, response)) return true;
if (!try_catch.CanContinue()) { // termination exception?
try_catch.ReThrow();
return false;
}
v8::String::Utf8Value s(st.isolate, try_catch.Exception());
const char *message = *s ? *s : "unexpected failure";
// most serialization errors will be DataCloneErrors but not always
// DataCloneErrors are not directly detectable so use a heuristic
if (!strstr(message, "could not be cloned")) {
try_catch.ReThrow();
return false;
}
// return an {"error": "foo could not be cloned"} object
v8::Local<v8::Object> error;
{
v8::Context::Scope context_scope(st.safe_context);
error = v8::Object::New(st.isolate);
}
auto key = v8::String::NewFromUtf8Literal(st.isolate, "error");
v8::Local<v8::String> val;
if (!v8::String::NewFromUtf8(st.isolate, message).ToLocal(&val)) {
val = v8::String::NewFromUtf8Literal(st.isolate, "unexpected error");
}
error->Set(st.context, key, val).Check();
response->Set(st.context, 0, error).Check();
if (!reply(st, response)) {
try_catch.ReThrow();
return false;
}
return true;
}

v8::Local<v8::Value> sanitize(State& st, v8::Local<v8::Value> v)
{
// punch through proxies
while (v->IsProxy()) v = v8::Proxy::Cast(*v)->GetTarget();
// things that cannot be serialized
if (v->IsArgumentsObject() ||
v->IsPromise() ||
v->IsModule() ||
v->IsModuleNamespaceObject() ||
v->IsWasmMemoryObject() ||
v->IsWasmModuleObject() ||
v->IsWasmNull() ||
v->IsWeakRef()) {
return v8::Object::New(st.isolate);
}
// V8's serializer doesn't accept symbols
if (v->IsSymbol()) return v8::Symbol::Cast(*v)->Description(st.isolate);
// TODO(bnoordhuis) replace this hack with something more principled
Expand All @@ -111,17 +142,10 @@ v8::Local<v8::Value> sanitize(State& st, v8::Local<v8::Value> v)
return array;
}
}
// WebAssembly.Instance objects are not serializable but there
// is no direct way to detect them through the V8 C++ API
if (!st.webassembly_instance.IsEmpty() &&
v->IsObject() &&
v->InstanceOf(st.context, st.webassembly_instance).FromMaybe(false)) {
return v8::Object::New(st.isolate);
}
return v;
}

v8::Local<v8::Value> to_error(State& st, v8::TryCatch *try_catch, int cause)
v8::Local<v8::String> to_error(State& st, v8::TryCatch *try_catch, int cause)
{
v8::Local<v8::Value> t;
char buf[1024];
Expand Down Expand Up @@ -216,18 +240,7 @@ extern "C" State *v8_thread_init(Context *c, const uint8_t *snapshot_buf,
st.safe_context = v8::Context::New(st.isolate);
st.context = v8::Context::New(st.isolate);
v8::Context::Scope context_scope(st.context);
auto global = st.context->Global();
// globalThis.WebAssembly is missing in --jitless mode
auto key = v8::String::NewFromUtf8Literal(st.isolate, "WebAssembly");
v8::Local<v8::Value> wasm_v;
if (global->Get(st.context, key).ToLocal(&wasm_v) && wasm_v->IsObject()) {
auto key = v8::String::NewFromUtf8Literal(st.isolate, "Instance");
st.webassembly_instance =
wasm_v.As<v8::Object>()
->Get(st.context, key).ToLocalChecked().As<v8::Object>();
}
if (single_threaded) {
st.persistent_webassembly_instance.Reset(st.isolate, st.webassembly_instance);
st.persistent_safe_context.Reset(st.isolate, st.safe_context);
st.persistent_context.Reset(st.isolate, st.context);
return pst; // intentionally returning early and keeping alive
Expand All @@ -246,7 +259,7 @@ void v8_api_callback(const v8::FunctionCallbackInfo<v8::Value>& info)
v8::Local<v8::Array> request;
{
v8::Context::Scope context_scope(st.safe_context);
request = v8::Array::New(st.isolate, 2);
request = v8::Array::New(st.isolate, 1 + info.Length());
}
for (int i = 0, n = info.Length(); i < n; i++) {
request->Set(st.context, i, sanitize(st, info[i])).Check();
Expand Down Expand Up @@ -354,11 +367,6 @@ extern "C" void v8_call(State *pst, const uint8_t *p, size_t n)
v8::ValueDeserializer des(st.isolate, p, n);
std::vector<v8::Local<v8::Value>> args;
des.ReadHeader(st.context).Check();
v8::Local<v8::Array> response;
{
v8::Context::Scope context_scope(st.safe_context);
response = v8::Array::New(st.isolate, 2);
}
v8::Local<v8::Value> result;
int cause = INTERNAL_ERROR;
{
Expand Down Expand Up @@ -420,9 +428,7 @@ extern "C" void v8_call(State *pst, const uint8_t *p, size_t n)
if (!cause && try_catch.HasCaught()) cause = RUNTIME_ERROR;
if (cause) result = v8::Undefined(st.isolate);
auto err = to_error(st, &try_catch, cause);
response->Set(st.context, 0, result).Check();
response->Set(st.context, 1, err).Check();
if (!reply(st, response)) {
if (!reply(st, result, err)) {
assert(try_catch.HasCaught());
goto fail; // retry; can be termination exception
}
Expand All @@ -437,11 +443,6 @@ extern "C" void v8_eval(State *pst, const uint8_t *p, size_t n)
v8::HandleScope handle_scope(st.isolate);
v8::ValueDeserializer des(st.isolate, p, n);
des.ReadHeader(st.context).Check();
v8::Local<v8::Array> response;
{
v8::Context::Scope context_scope(st.safe_context);
response = v8::Array::New(st.isolate, 2);
}
v8::Local<v8::Value> result;
int cause = INTERNAL_ERROR;
{
Expand Down Expand Up @@ -475,9 +476,7 @@ extern "C" void v8_eval(State *pst, const uint8_t *p, size_t n)
if (!cause && try_catch.HasCaught()) cause = RUNTIME_ERROR;
if (cause) result = v8::Undefined(st.isolate);
auto err = to_error(st, &try_catch, cause);
response->Set(st.context, 0, result).Check();
response->Set(st.context, 1, err).Check();
if (!reply(st, response)) {
if (!reply(st, result, err)) {
assert(try_catch.HasCaught());
goto fail; // retry; can be termination exception
}
Expand Down Expand Up @@ -638,11 +637,6 @@ extern "C" void v8_snapshot(State *pst, const uint8_t *p, size_t n)
v8::HandleScope handle_scope(st.isolate);
v8::ValueDeserializer des(st.isolate, p, n);
des.ReadHeader(st.context).Check();
v8::Local<v8::Array> response;
{
v8::Context::Scope context_scope(st.safe_context);
response = v8::Array::New(st.isolate, 2);
}
v8::Local<v8::Value> result;
v8::StartupData blob{nullptr, 0};
int cause = INTERNAL_ERROR;
Expand Down Expand Up @@ -682,9 +676,7 @@ extern "C" void v8_snapshot(State *pst, const uint8_t *p, size_t n)
} else {
err = to_error(st, &try_catch, cause);
}
response->Set(st.context, 0, result).Check();
response->Set(st.context, 1, err).Check();
if (!reply(st, response)) {
if (!reply(st, result, err)) {
assert(try_catch.HasCaught());
goto fail; // retry; can be termination exception
}
Expand All @@ -699,11 +691,6 @@ extern "C" void v8_warmup(State *pst, const uint8_t *p, size_t n)
std::vector<uint8_t> storage;
v8::ValueDeserializer des(st.isolate, p, n);
des.ReadHeader(st.context).Check();
v8::Local<v8::Array> response;
{
v8::Context::Scope context_scope(st.safe_context);
response = v8::Array::New(st.isolate, 2);
}
v8::Local<v8::Value> result;
v8::StartupData blob{nullptr, 0};
int cause = INTERNAL_ERROR;
Expand Down Expand Up @@ -761,9 +748,7 @@ extern "C" void v8_warmup(State *pst, const uint8_t *p, size_t n)
} else {
err = to_error(st, &try_catch, cause);
}
response->Set(st.context, 0, result).Check();
response->Set(st.context, 1, err).Check();
if (!reply(st, response)) {
if (!reply(st, result, err)) {
assert(try_catch.HasCaught());
goto fail; // retry; can be termination exception
}
Expand Down Expand Up @@ -807,14 +792,12 @@ extern "C" void v8_single_threaded_enter(State *pst, Context *c, void (*f)(Conte
v8::Isolate::Scope isolate_scope(st.isolate);
v8::HandleScope handle_scope(st.isolate);
{
st.webassembly_instance = v8::Local<v8::Object>::New(st.isolate, st.persistent_webassembly_instance);
st.safe_context = v8::Local<v8::Context>::New(st.isolate, st.persistent_safe_context);
st.context = v8::Local<v8::Context>::New(st.isolate, st.persistent_context);
v8::Context::Scope context_scope(st.context);
f(c);
st.context = v8::Local<v8::Context>();
st.safe_context = v8::Local<v8::Context>();
st.webassembly_instance = v8::Local<v8::Object>();
}
}

Expand All @@ -830,7 +813,6 @@ State::~State()
{
v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
persistent_webassembly_instance.Reset();
persistent_safe_context.Reset();
persistent_context.Reset();
}
Expand Down
38 changes: 19 additions & 19 deletions test/mini_racer_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -893,14 +893,13 @@ def test_wasm_ref
skip "TruffleRuby does not support WebAssembly"
end
context = MiniRacer::Context.new
# Error: [object Object] could not be cloned
assert_raises(MiniRacer::RuntimeError) do
context.eval("
var b = [0,97,115,109,1,0,0,0,1,26,5,80,0,95,0,80,0,95,1,127,0,96,0,1,110,96,1,100,2,1,111,96,0,1,100,3,3,4,3,3,2,4,7,26,2,12,99,114,101,97,116,101,83,116,114,117,99,116,0,1,7,114,101,102,70,117,110,99,0,2,9,5,1,3,0,1,0,10,23,3,8,0,32,0,20,2,251,27,11,7,0,65,12,251,0,1,11,4,0,210,0,11,0,44,4,110,97,109,101,1,37,3,0,11,101,120,112,111,114,116,101,100,65,110,121,1,12,99,114,101,97,116,101,83,116,114,117,99,116,2,7,114,101,102,70,117,110,99]
var o = new WebAssembly.Instance(new WebAssembly.Module(new Uint8Array(b))).exports
o.refFunc()(o.createStruct) // exotic object
")
end
expected = {"error" => "Error: [object Object] could not be cloned."}
actual = context.eval("
var b = [0,97,115,109,1,0,0,0,1,26,5,80,0,95,0,80,0,95,1,127,0,96,0,1,110,96,1,100,2,1,111,96,0,1,100,3,3,4,3,3,2,4,7,26,2,12,99,114,101,97,116,101,83,116,114,117,99,116,0,1,7,114,101,102,70,117,110,99,0,2,9,5,1,3,0,1,0,10,23,3,8,0,32,0,20,2,251,27,11,7,0,65,12,251,0,1,11,4,0,210,0,11,0,44,4,110,97,109,101,1,37,3,0,11,101,120,112,111,114,116,101,100,65,110,121,1,12,99,114,101,97,116,101,83,116,114,117,99,116,2,7,114,101,102,70,117,110,99]
var o = new WebAssembly.Instance(new WebAssembly.Module(new Uint8Array(b))).exports
o.refFunc()(o.createStruct) // exotic object
")
assert_equal expected, actual
end

def test_proxy_support
Expand Down Expand Up @@ -1060,16 +1059,17 @@ def test_map

def test_regexp_string_iterator
context = MiniRacer::Context.new
exc = false
begin
context.eval("'abc'.matchAll(/./g)")
rescue MiniRacer::RuntimeError => e
# TODO(bnoordhuis) maybe detect the iterator object and serialize
# it as an array of strings; problem is there is no V8 API to detect
# regexp string iterator objects
assert_match(/\[object RegExp String Iterator\] could not be cloned/, e.message)
exc = true
end
assert exc
# TODO(bnoordhuis) maybe detect the iterator object and serialize
# it as a string or array of strings; problem is there is no V8 API
# to detect regexp string iterator objects
expected = {"error" => "Error: [object RegExp String Iterator] could not be cloned."}
assert_equal expected, context.eval("'abc'.matchAll(/./g)")
end

def test_function_property
context = MiniRacer::Context.new
# regrettably loses the non-function properties
expected = {"error" => "Error: f() {} could not be cloned."}
assert_equal expected, context.eval("({ x: 42, f() {} })")
end
end

0 comments on commit 394980b

Please sign in to comment.