Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2dbeb88

Browse filesBrowse files
mhdawsonrichardlau
mhdawson
authored and
richardlau
committed
node-api: handle no support for external buffers
Refs: electron/electron#35801 Refs: nodejs/abi-stable-node#441 Electron recently dropped support for external buffers. Provide a way for addon authors to: - hide the methods to create external buffers so they can avoid using them if they want the broadest compatibility. - call the methods that create external buffers at runtime to check if external buffers are supported and either use them or not based on the return code. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #45181 Backport-PR-URL: #45616 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 785dc3e commit 2dbeb88
Copy full SHA for 2dbeb88

File tree

Expand file treeCollapse file tree

8 files changed

+49
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+49
-2
lines changed
Open diff view settings
Collapse file

‎doc/api/n-api.md‎

Copy file name to clipboardExpand all lines: doc/api/n-api.md
+27Lines changed: 27 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ typedef enum {
573573
napi_arraybuffer_expected,
574574
napi_detachable_arraybuffer_expected,
575575
napi_would_deadlock, /* unused */
576+
napi_no_external_buffers_allowed
576577
} napi_status;
577578
```
578579

@@ -2252,6 +2253,19 @@ napi_create_external_arraybuffer(napi_env env,
22522253

22532254
Returns `napi_ok` if the API succeeded.
22542255

2256+
**Some runtimes other than Node.js have dropped support for external buffers**.
2257+
On runtimes other than Node.js this method may return
2258+
`napi_no_external_buffers_allowed` to indicate that external
2259+
buffers are not supported. One such runtime is Electron as
2260+
described in this issue
2261+
[electron/issues/35801](https://github.com/electron/electron/issues/35801).
2262+
2263+
In order to maintain broadest compatibility with all runtimes
2264+
you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
2265+
includes for the node-api headers. Doing so will hide the 2 functions
2266+
that create external buffers. This will ensure a compilation error
2267+
occurs if you accidentally use one of these methods.
2268+
22552269
This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`.
22562270
The underlying byte buffer of the `ArrayBuffer` is externally allocated and
22572271
managed. The caller must ensure that the byte buffer remains valid until the
@@ -2295,6 +2309,19 @@ napi_status napi_create_external_buffer(napi_env env,
22952309

22962310
Returns `napi_ok` if the API succeeded.
22972311

2312+
**Some runtimes other than Node.js have dropped support for external buffers**.
2313+
On runtimes other than Node.js this method may return
2314+
`napi_no_external_buffers_allowed` to indicate that external
2315+
buffers are not supported. One such runtime is Electron as
2316+
described in this issue
2317+
[electron/issues/35801](https://github.com/electron/electron/issues/35801).
2318+
2319+
In order to maintain broadest compatibility with all runtimes
2320+
you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
2321+
includes for the node-api headers. Doing so will hide the 2 functions
2322+
that create external buffers. This will ensure a compilation error
2323+
occurs if you accidentally use one of these methods.
2324+
22982325
This API allocates a `node::Buffer` object and initializes it with data
22992326
backed by the passed in buffer. While this is still a fully-supported data
23002327
structure, in most cases using a `TypedArray` will suffice.
Collapse file

‎src/js_native_api.h‎

Copy file name to clipboardExpand all lines: src/js_native_api.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,15 @@ NAPI_EXTERN napi_status napi_create_arraybuffer(napi_env env,
387387
size_t byte_length,
388388
void** data,
389389
napi_value* result);
390+
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
390391
NAPI_EXTERN napi_status
391392
napi_create_external_arraybuffer(napi_env env,
392393
void* external_data,
393394
size_t byte_length,
394395
napi_finalize finalize_cb,
395396
void* finalize_hint,
396397
napi_value* result);
398+
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
397399
NAPI_EXTERN napi_status napi_get_arraybuffer_info(napi_env env,
398400
napi_value arraybuffer,
399401
void** data,
Collapse file

‎src/js_native_api_types.h‎

Copy file name to clipboardExpand all lines: src/js_native_api_types.h
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ typedef enum {
9292
napi_date_expected,
9393
napi_arraybuffer_expected,
9494
napi_detachable_arraybuffer_expected,
95-
napi_would_deadlock // unused
95+
napi_would_deadlock, // unused
96+
napi_no_external_buffers_allowed
9697
} napi_status;
9798
// Note: when adding a new enum value to `napi_status`, please also update
9899
// * `const int last_status` in the definition of `napi_get_last_error_info()'
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ const char* error_messages[] = {nullptr,
744744
"An arraybuffer was expected",
745745
"A detachable arraybuffer was expected",
746746
"Main thread would deadlock",
747+
"External buffers are not allowed",
747748
};
748749

749750
napi_status napi_get_last_error_info(napi_env env,
@@ -755,7 +756,7 @@ napi_status napi_get_last_error_info(napi_env env,
755756
// message in the `napi_status` enum each time a new error message is added.
756757
// We don't have a napi_status_last as this would result in an ABI
757758
// change each time a message was added.
758-
const int last_status = napi_would_deadlock;
759+
const int last_status = napi_no_external_buffers_allowed;
759760

760761
static_assert(
761762
NAPI_ARRAYSIZE(error_messages) == last_status + 1,
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,10 @@ napi_status napi_create_external_buffer(napi_env env,
928928
NAPI_PREAMBLE(env);
929929
CHECK_ARG(env, result);
930930

931+
#if defined(V8_ENABLE_SANDBOX)
932+
return napi_set_last_error(env, napi_no_external_buffers_allowed);
933+
#endif
934+
931935
v8::Isolate* isolate = env->isolate;
932936

933937
// The finalizer object will delete itself after invoking the callback.
Collapse file

‎src/node_api.h‎

Copy file name to clipboardExpand all lines: src/node_api.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,14 @@ NAPI_EXTERN napi_status napi_create_buffer(napi_env env,
138138
size_t length,
139139
void** data,
140140
napi_value* result);
141+
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
141142
NAPI_EXTERN napi_status napi_create_external_buffer(napi_env env,
142143
size_t length,
143144
void* data,
144145
napi_finalize finalize_cb,
145146
void* finalize_hint,
146147
napi_value* result);
148+
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
147149
NAPI_EXTERN napi_status napi_create_buffer_copy(napi_env env,
148150
size_t length,
149151
const void* data,
Collapse file

‎test/js-native-api/test_general/test_general.c‎

Copy file name to clipboardExpand all lines: test/js-native-api/test_general/test_general.c
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to
2+
// validate that it can be used as a form of test itself. It is
3+
// not related to any of the other tests
4+
// defined in the file
5+
#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
16
#include <stdio.h>
27
#include <stdlib.h>
38
#include <stdint.h>
Collapse file

‎test/node-api/test_general/test_general.c‎

Copy file name to clipboardExpand all lines: test/node-api/test_general/test_general.c
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
#define NAPI_EXPERIMENTAL
2+
// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to validate that it can
3+
// be used as a form of test itself. It is
4+
// not related to any of the other tests
5+
// defined in the file
6+
#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
27
#include <node_api.h>
38
#include <stdlib.h>
49
#include "../../js-native-api/common.h"

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.