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 5a0efa0

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 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 3b2b70d commit 5a0efa0
Copy full SHA for 5a0efa0

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
@@ -579,6 +579,7 @@ typedef enum {
579579
napi_arraybuffer_expected,
580580
napi_detachable_arraybuffer_expected,
581581
napi_would_deadlock, /* unused */
582+
napi_no_external_buffers_allowed
582583
} napi_status;
583584
```
584585

@@ -2399,6 +2400,19 @@ napi_create_external_arraybuffer(napi_env env,
23992400

24002401
Returns `napi_ok` if the API succeeded.
24012402

2403+
**Some runtimes other than Node.js have dropped support for external buffers**.
2404+
On runtimes other than Node.js this method may return
2405+
`napi_no_external_buffers_allowed` to indicate that external
2406+
buffers are not supported. One such runtime is Electron as
2407+
described in this issue
2408+
[electron/issues/35801](https://github.com/electron/electron/issues/35801).
2409+
2410+
In order to maintain broadest compatibility with all runtimes
2411+
you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
2412+
includes for the node-api headers. Doing so will hide the 2 functions
2413+
that create external buffers. This will ensure a compilation error
2414+
occurs if you accidentally use one of these methods.
2415+
24022416
This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`.
24032417
The underlying byte buffer of the `ArrayBuffer` is externally allocated and
24042418
managed. The caller must ensure that the byte buffer remains valid until the
@@ -2443,6 +2457,19 @@ napi_status napi_create_external_buffer(napi_env env,
24432457

24442458
Returns `napi_ok` if the API succeeded.
24452459

2460+
**Some runtimes other than Node.js have dropped support for external buffers**.
2461+
On runtimes other than Node.js this method may return
2462+
`napi_no_external_buffers_allowed` to indicate that external
2463+
buffers are not supported. One such runtime is Electron as
2464+
described in this issue
2465+
[electron/issues/35801](https://github.com/electron/electron/issues/35801).
2466+
2467+
In order to maintain broadest compatibility with all runtimes
2468+
you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
2469+
includes for the node-api headers. Doing so will hide the 2 functions
2470+
that create external buffers. This will ensure a compilation error
2471+
occurs if you accidentally use one of these methods.
2472+
24462473
This API allocates a `node::Buffer` object and initializes it with data
24472474
backed by the passed in buffer. While this is still a fully-supported data
24482475
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
@@ -401,13 +401,15 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_arraybuffer(napi_env env,
401401
size_t byte_length,
402402
void** data,
403403
napi_value* result);
404+
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
404405
NAPI_EXTERN napi_status NAPI_CDECL
405406
napi_create_external_arraybuffer(napi_env env,
406407
void* external_data,
407408
size_t byte_length,
408409
napi_finalize finalize_cb,
409410
void* finalize_hint,
410411
napi_value* result);
412+
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
411413
NAPI_EXTERN napi_status NAPI_CDECL napi_get_arraybuffer_info(
412414
napi_env env, napi_value arraybuffer, void** data, size_t* byte_length);
413415
NAPI_EXTERN napi_status NAPI_CDECL napi_is_typedarray(napi_env env,
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
@@ -98,7 +98,8 @@ typedef enum {
9898
napi_date_expected,
9999
napi_arraybuffer_expected,
100100
napi_detachable_arraybuffer_expected,
101-
napi_would_deadlock // unused
101+
napi_would_deadlock, // unused
102+
napi_no_external_buffers_allowed
102103
} napi_status;
103104
// Note: when adding a new enum value to `napi_status`, please also update
104105
// * `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
@@ -746,6 +746,7 @@ static const char* error_messages[] = {
746746
"An arraybuffer was expected",
747747
"A detachable arraybuffer was expected",
748748
"Main thread would deadlock",
749+
"External buffers are not allowed",
749750
};
750751

751752
napi_status NAPI_CDECL napi_get_last_error_info(
@@ -757,7 +758,7 @@ napi_status NAPI_CDECL napi_get_last_error_info(
757758
// message in the `napi_status` enum each time a new error message is added.
758759
// We don't have a napi_status_last as this would result in an ABI
759760
// change each time a message was added.
760-
const int last_status = napi_would_deadlock;
761+
const int last_status = napi_no_external_buffers_allowed;
761762

762763
static_assert(NAPI_ARRAYSIZE(error_messages) == last_status + 1,
763764
"Count of error messages must match count of error values");
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
@@ -950,6 +950,10 @@ napi_status NAPI_CDECL napi_create_external_buffer(napi_env env,
950950
NAPI_PREAMBLE(env);
951951
CHECK_ARG(env, result);
952952

953+
#if defined(V8_ENABLE_SANDBOX)
954+
return napi_set_last_error(env, napi_no_external_buffers_allowed);
955+
#endif
956+
953957
v8::Isolate* isolate = env->isolate;
954958

955959
// 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
@@ -153,13 +153,15 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer(napi_env env,
153153
size_t length,
154154
void** data,
155155
napi_value* result);
156+
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
156157
NAPI_EXTERN napi_status NAPI_CDECL
157158
napi_create_external_buffer(napi_env env,
158159
size_t length,
159160
void* data,
160161
napi_finalize finalize_cb,
161162
void* finalize_hint,
162163
napi_value* result);
164+
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
163165
NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer_copy(napi_env env,
164166
size_t length,
165167
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.