Skip to content

Commit 55aab6b

Browse files
addaleaxMylesBorins
authored andcommittedApr 16, 2018
n-api: check against invalid handle scope usage
Fixes: #16175 Backport-PR-URL: #19447 PR-URL: #16201 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 169b53e commit 55aab6b

File tree

4 files changed

+22
-8
lines changed

4 files changed

+22
-8
lines changed
 

‎src/node_api.cc

+15
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ struct napi_env__ {
4848
v8::Persistent<v8::ObjectTemplate> accessor_data_template;
4949
bool has_instance_available;
5050
napi_extended_error_info last_error;
51+
int open_handle_scopes = 0;
5152
};
5253

5354
#define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \
@@ -508,12 +509,16 @@ class CallbackWrapperBase : public CallbackWrapper {
508509
// Make sure any errors encountered last time we were in N-API are gone.
509510
napi_clear_last_error(env);
510511

512+
int open_handle_scopes = env->open_handle_scopes;
513+
511514
napi_value result = cb(env, cbinfo_wrapper);
512515

513516
if (result != nullptr) {
514517
this->SetReturnValue(result);
515518
}
516519

520+
CHECK_EQ(env->open_handle_scopes, open_handle_scopes);
521+
517522
if (!env->last_exception.IsEmpty()) {
518523
isolate->ThrowException(
519524
v8::Local<v8::Value>::New(isolate, env->last_exception));
@@ -2587,6 +2592,7 @@ napi_status napi_open_handle_scope(napi_env env, napi_handle_scope* result) {
25872592

25882593
*result = v8impl::JsHandleScopeFromV8HandleScope(
25892594
new v8impl::HandleScopeWrapper(env->isolate));
2595+
env->open_handle_scopes++;
25902596
return napi_clear_last_error(env);
25912597
}
25922598

@@ -2595,7 +2601,11 @@ napi_status napi_close_handle_scope(napi_env env, napi_handle_scope scope) {
25952601
// JS exceptions.
25962602
CHECK_ENV(env);
25972603
CHECK_ARG(env, scope);
2604+
if (env->open_handle_scopes == 0) {
2605+
return napi_handle_scope_mismatch;
2606+
}
25982607

2608+
env->open_handle_scopes--;
25992609
delete v8impl::V8HandleScopeFromJsHandleScope(scope);
26002610
return napi_clear_last_error(env);
26012611
}
@@ -2610,6 +2620,7 @@ napi_status napi_open_escapable_handle_scope(
26102620

26112621
*result = v8impl::JsEscapableHandleScopeFromV8EscapableHandleScope(
26122622
new v8impl::EscapableHandleScopeWrapper(env->isolate));
2623+
env->open_handle_scopes++;
26132624
return napi_clear_last_error(env);
26142625
}
26152626

@@ -2620,8 +2631,12 @@ napi_status napi_close_escapable_handle_scope(
26202631
// JS exceptions.
26212632
CHECK_ENV(env);
26222633
CHECK_ARG(env, scope);
2634+
if (env->open_handle_scopes == 0) {
2635+
return napi_handle_scope_mismatch;
2636+
}
26232637

26242638
delete v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
2639+
env->open_handle_scopes--;
26252640
return napi_clear_last_error(env);
26262641
}
26272642

‎src/node_api_types.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ typedef enum {
6969
napi_generic_failure,
7070
napi_pending_exception,
7171
napi_cancelled,
72-
napi_escape_called_twice
72+
napi_escape_called_twice,
73+
napi_handle_scope_mismatch
7374
} napi_status;
7475

7576
typedef napi_value (*napi_callback)(napi_env env,

‎test/addons-napi/test_handle_scope/test.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ testHandleScope.NewScope();
1010

1111
assert.ok(testHandleScope.NewScopeEscape() instanceof Object);
1212

13-
assert.throws(
14-
() => {
15-
testHandleScope.NewScopeEscapeTwice();
16-
},
17-
Error);
13+
testHandleScope.NewScopeEscapeTwice();
1814

1915
assert.throws(
2016
() => {

‎test/addons-napi/test_handle_scope/test_handle_scope.c

+4-2
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,15 @@ napi_value NewScopeEscapeTwice(napi_env env, napi_callback_info info) {
3333
napi_escapable_handle_scope scope;
3434
napi_value output = NULL;
3535
napi_value escapee = NULL;
36+
napi_status status;
3637

3738
NAPI_CALL(env, napi_open_escapable_handle_scope(env, &scope));
3839
NAPI_CALL(env, napi_create_object(env, &output));
3940
NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
40-
NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
41+
status = napi_escape_handle(env, scope, output, &escapee);
42+
NAPI_ASSERT(env, status == napi_escape_called_twice, "Escaping twice fails");
4143
NAPI_CALL(env, napi_close_escapable_handle_scope(env, scope));
42-
return escapee;
44+
return NULL;
4345
}
4446

4547
napi_value NewScopeWithException(napi_env env, napi_callback_info info) {

0 commit comments

Comments
 (0)
Please sign in to comment.