From 5a85d4f2c68e207d3868e9c8e8b511a6aaba2144 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 30 Sep 2020 11:55:59 +0200 Subject: [PATCH] src: make MakeCallback() check can_call_into_js before getting method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/35424 Reviewed-By: Joyee Cheung Reviewed-By: Denys Otrishko Reviewed-By: James M Snell Reviewed-By: Andrey Pechkurov Reviewed-By: Gerhard Stöbich Reviewed-By: Rich Trott --- src/api/callback.cc | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index 84664c089594eb..3d4f91a866ea39 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -223,10 +223,19 @@ MaybeLocal MakeCallback(Isolate* isolate, int argc, Local argv[], async_context asyncContext) { - Local callback_v = - recv->Get(isolate->GetCurrentContext(), symbol).ToLocalChecked(); - if (callback_v.IsEmpty()) return Local(); - if (!callback_v->IsFunction()) return Local(); + // 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(); + + Local callback_v; + if (!recv->Get(isolate->GetCurrentContext(), symbol).ToLocal(&callback_v)) + return Local(); + 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 callback = callback_v.As(); return MakeCallback(isolate, recv, callback, argc, argv, asyncContext); }