Skip to content

Commit

Permalink
src: make MakeCallback() check can_call_into_js before getting method
Browse files Browse the repository at this point in the history
There is a check for this in the inner `MakeCallback()` function
called by it, but since the `Get()` call here can also result in a
call into JS, we should ideally check the flag before that.

PR-URL: nodejs#35424
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and joesepi committed Oct 22, 2020
1 parent 56cabf1 commit 9c56134
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,19 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
int argc,
Local<Value> argv[],
async_context asyncContext) {
Local<Value> callback_v =
recv->Get(isolate->GetCurrentContext(), symbol).ToLocalChecked();
if (callback_v.IsEmpty()) return Local<Value>();
if (!callback_v->IsFunction()) return Local<Value>();
// Check can_call_into_js() first because calling Get() might do so.
Environment* env = Environment::GetCurrent(recv->CreationContext());
CHECK_NOT_NULL(env);
if (!env->can_call_into_js()) return Local<Value>();

Local<Value> callback_v;
if (!recv->Get(isolate->GetCurrentContext(), symbol).ToLocal(&callback_v))
return Local<Value>();
if (!callback_v->IsFunction()) {
// This used to return an empty value, but Undefined() makes more sense
// since no exception is pending here.
return Undefined(isolate);
}
Local<Function> callback = callback_v.As<Function>();
return MakeCallback(isolate, recv, callback, argc, argv, asyncContext);
}
Expand Down

0 comments on commit 9c56134

Please sign in to comment.