context::run_script() crash

[VitaminCpp]

Hi,

if I try to compile a script with V8 6.4.388 with an intentional syntax error, my app crashes. This happens because of line 262. v8::Script::Compile() returns a MaybeLocal() and you're using ToLocalChecked() which will crash the current process as stated in v8.h.

To solve this issue just convert it by "ToLocal()" and then check for empty:

v8::Local<v8::Value> context::run_script(std::string const& source,
    std::string const& filename)
{
    v8::EscapableHandleScope scope(isolate_);
    v8::Local<v8::Context> context = isolate_->GetCurrentContext();

    v8::ScriptOrigin origin(to_v8(isolate_, filename));
  v8::Local<v8::Value> result;

  v8::Local<v8::Script> script;
  if (!v8::Script::Compile(context,
    to_v8(isolate_, source), &origin).ToLocal(&script))
  {
    return scope.Escape(result);
  }

  if (!script.IsEmpty())
  {
    script->Run(context).ToLocal(&result);
  }
  return scope.Escape(result);
}

These "ToLocalChecked()" calls are IMHO a bit dangerous for people who run the V8 on the main thread.

[pmed]

Hi @VitaminCpp

Thank you for the issue reporting!

I've fixed it as you proposed. Now the run_script() function returns an empty handle in case of syntax error, but V8 will throw a JavaScript exception. Such an exception could be handled with v8::TryCatch as in the test case.

[VitaminCpp]

Hi @pmed! Thank you for this great library! Great! I think this is a good solution.

[pmed]

Hi @VitaminCpp

Thanks! It seems I have to review all the ToLocalChecked() usage in the library code. So I'd stay this issue open yet.