GeezerVison is showing github.com — node-api: handle no support for external buffers · nodejs/node@2dbeb88
File treeExpand file treeCollapse file tree Open diff view settings js-native-api/test_general
Expand file treeCollapse file tree Open diff view settings Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -573,6 +573,7 @@ typedef enum { | 573 | 573 | napi_arraybuffer_expected, | 574 | 574 | napi_detachable_arraybuffer_expected, | 575 | 575 | napi_would_deadlock, /* unused */ | | 576 | + napi_no_external_buffers_allowed | 576 | 577 | } napi_status; | 577 | 578 | ``` | 578 | 579 |
| @@ -2252,6 +2253,19 @@ napi_create_external_arraybuffer(napi_env env, | 2252 | 2253 |
| 2253 | 2254 | Returns `napi_ok` if the API succeeded. | 2254 | 2255 |
| | 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 | + | 2255 | 2269 | This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`. | 2256 | 2270 | The underlying byte buffer of the `ArrayBuffer` is externally allocated and | 2257 | 2271 | 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, | 2295 | 2309 |
| 2296 | 2310 | Returns `napi_ok` if the API succeeded. | 2297 | 2311 |
| | 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 | + | 2298 | 2325 | This API allocates a `node::Buffer` object and initializes it with data | 2299 | 2326 | backed by the passed in buffer. While this is still a fully-supported data | 2300 | 2327 | structure, in most cases using a `TypedArray` will suffice. | |
Collapse file Copy file name to clipboardExpand all lines: src/js_native_api.h +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -387,13 +387,15 @@ NAPI_EXTERN napi_status napi_create_arraybuffer(napi_env env, | 387 | 387 | size_t byte_length, | 388 | 388 | void** data, | 389 | 389 | napi_value* result); | | 390 | +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 390 | 391 | NAPI_EXTERN napi_status | 391 | 392 | napi_create_external_arraybuffer(napi_env env, | 392 | 393 | void* external_data, | 393 | 394 | size_t byte_length, | 394 | 395 | napi_finalize finalize_cb, | 395 | 396 | void* finalize_hint, | 396 | 397 | napi_value* result); | | 398 | +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 397 | 399 | NAPI_EXTERN napi_status napi_get_arraybuffer_info(napi_env env, | 398 | 400 | napi_value arraybuffer, | 399 | 401 | void** data, | |
Collapse file Copy file name to clipboardExpand all lines: src/js_native_api_types.h +2-1Lines changed: 2 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -92,7 +92,8 @@ typedef enum { | 92 | 92 | napi_date_expected, | 93 | 93 | napi_arraybuffer_expected, | 94 | 94 | napi_detachable_arraybuffer_expected, | 95 | | - napi_would_deadlock // unused | | 95 | + napi_would_deadlock, // unused | | 96 | + napi_no_external_buffers_allowed | 96 | 97 | } napi_status; | 97 | 98 | // Note: when adding a new enum value to `napi_status`, please also update | 98 | 99 | // * `const int last_status` in the definition of `napi_get_last_error_info()' | |
Collapse file Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc +2-1Lines changed: 2 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -744,6 +744,7 @@ const char* error_messages[] = {nullptr, | 744 | 744 | "An arraybuffer was expected", | 745 | 745 | "A detachable arraybuffer was expected", | 746 | 746 | "Main thread would deadlock", | | 747 | + "External buffers are not allowed", | 747 | 748 | }; | 748 | 749 |
| 749 | 750 | napi_status napi_get_last_error_info(napi_env env, | @@ -755,7 +756,7 @@ napi_status napi_get_last_error_info(napi_env env, | 755 | 756 | // message in the `napi_status` enum each time a new error message is added. | 756 | 757 | // We don't have a napi_status_last as this would result in an ABI | 757 | 758 | // 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; | 759 | 760 |
| 760 | 761 | static_assert( | 761 | 762 | NAPI_ARRAYSIZE(error_messages) == last_status + 1, | |
Collapse file Copy file name to clipboardExpand all lines: src/node_api.cc +4Lines changed: 4 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -928,6 +928,10 @@ napi_status napi_create_external_buffer(napi_env env, | 928 | 928 | NAPI_PREAMBLE(env); | 929 | 929 | CHECK_ARG(env, result); | 930 | 930 |
| | 931 | +#if defined(V8_ENABLE_SANDBOX) | | 932 | + return napi_set_last_error(env, napi_no_external_buffers_allowed); | | 933 | +#endif | | 934 | + | 931 | 935 | v8::Isolate* isolate = env->isolate; | 932 | 936 |
| 933 | 937 | // The finalizer object will delete itself after invoking the callback. | |
Collapse file Copy file name to clipboardExpand all lines: src/node_api.h +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -138,12 +138,14 @@ NAPI_EXTERN napi_status napi_create_buffer(napi_env env, | 138 | 138 | size_t length, | 139 | 139 | void** data, | 140 | 140 | napi_value* result); | | 141 | +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 141 | 142 | NAPI_EXTERN napi_status napi_create_external_buffer(napi_env env, | 142 | 143 | size_t length, | 143 | 144 | void* data, | 144 | 145 | napi_finalize finalize_cb, | 145 | 146 | void* finalize_hint, | 146 | 147 | napi_value* result); | | 148 | +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 147 | 149 | NAPI_EXTERN napi_status napi_create_buffer_copy(napi_env env, | 148 | 150 | size_t length, | 149 | 151 | const void* data, | |
Collapse file 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 number | Diff line number | Diff line change |
|---|
| | 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 | 1 | 6 | #include <stdio.h> | 2 | 7 | #include <stdlib.h> | 3 | 8 | #include <stdint.h> | |
Collapse file Copy file name to clipboardExpand all lines: test/node-api/test_general/test_general.c +5Lines changed: 5 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | #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 | 2 | 7 | #include <node_api.h> | 3 | 8 | #include <stdlib.h> | 4 | 9 | #include "../../js-native-api/common.h" | |
|