GeezerVison is showing github.com — node-api: handle no support for external buffers · nodejs/node@746175a
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 |
|---|
@@ -579,6 +579,7 @@ typedef enum { | 579 | 579 | napi_arraybuffer_expected, | 580 | 580 | napi_detachable_arraybuffer_expected, | 581 | 581 | napi_would_deadlock, /* unused */ | | 582 | + napi_no_external_buffers_allowed | 582 | 583 | } napi_status; | 583 | 584 | ``` | 584 | 585 |
| @@ -2403,6 +2404,19 @@ napi_create_external_arraybuffer(napi_env env, | 2403 | 2404 |
| 2404 | 2405 | Returns `napi_ok` if the API succeeded. | 2405 | 2406 |
| | 2407 | +**Some runtimes other than Node.js have dropped support for external buffers**. | | 2408 | +On runtimes other than Node.js this method may return | | 2409 | +`napi_no_external_buffers_allowed` to indicate that external | | 2410 | +buffers are not supported. One such runtime is Electron as | | 2411 | +described in this issue | | 2412 | +[electron/issues/35801](https://github.com/electron/electron/issues/35801). | | 2413 | + | | 2414 | +In order to maintain broadest compatibility with all runtimes | | 2415 | +you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before | | 2416 | +includes for the node-api headers. Doing so will hide the 2 functions | | 2417 | +that create external buffers. This will ensure a compilation error | | 2418 | +occurs if you accidentally use one of these methods. | | 2419 | + | 2406 | 2420 | This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`. | 2407 | 2421 | The underlying byte buffer of the `ArrayBuffer` is externally allocated and | 2408 | 2422 | managed. The caller must ensure that the byte buffer remains valid until the | @@ -2447,6 +2461,19 @@ napi_status napi_create_external_buffer(napi_env env, | 2447 | 2461 |
| 2448 | 2462 | Returns `napi_ok` if the API succeeded. | 2449 | 2463 |
| | 2464 | +**Some runtimes other than Node.js have dropped support for external buffers**. | | 2465 | +On runtimes other than Node.js this method may return | | 2466 | +`napi_no_external_buffers_allowed` to indicate that external | | 2467 | +buffers are not supported. One such runtime is Electron as | | 2468 | +described in this issue | | 2469 | +[electron/issues/35801](https://github.com/electron/electron/issues/35801). | | 2470 | + | | 2471 | +In order to maintain broadest compatibility with all runtimes | | 2472 | +you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before | | 2473 | +includes for the node-api headers. Doing so will hide the 2 functions | | 2474 | +that create external buffers. This will ensure a compilation error | | 2475 | +occurs if you accidentally use one of these methods. | | 2476 | + | 2450 | 2477 | This API allocates a `node::Buffer` object and initializes it with data | 2451 | 2478 | backed by the passed in buffer. While this is still a fully-supported data | 2452 | 2479 | 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 |
|---|
@@ -401,13 +401,15 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_arraybuffer(napi_env env, | 401 | 401 | size_t byte_length, | 402 | 402 | void** data, | 403 | 403 | napi_value* result); | | 404 | +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 404 | 405 | NAPI_EXTERN napi_status NAPI_CDECL | 405 | 406 | napi_create_external_arraybuffer(napi_env env, | 406 | 407 | void* external_data, | 407 | 408 | size_t byte_length, | 408 | 409 | napi_finalize finalize_cb, | 409 | 410 | void* finalize_hint, | 410 | 411 | napi_value* result); | | 412 | +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 411 | 413 | NAPI_EXTERN napi_status NAPI_CDECL napi_get_arraybuffer_info( | 412 | 414 | napi_env env, napi_value arraybuffer, void** data, size_t* byte_length); | 413 | 415 | NAPI_EXTERN napi_status NAPI_CDECL napi_is_typedarray(napi_env env, | |
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 |
|---|
@@ -98,7 +98,8 @@ typedef enum { | 98 | 98 | napi_date_expected, | 99 | 99 | napi_arraybuffer_expected, | 100 | 100 | napi_detachable_arraybuffer_expected, | 101 | | - napi_would_deadlock // unused | | 101 | + napi_would_deadlock, // unused | | 102 | + napi_no_external_buffers_allowed | 102 | 103 | } napi_status; | 103 | 104 | // Note: when adding a new enum value to `napi_status`, please also update | 104 | 105 | // * `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 |
|---|
@@ -746,6 +746,7 @@ static const char* error_messages[] = { | 746 | 746 | "An arraybuffer was expected", | 747 | 747 | "A detachable arraybuffer was expected", | 748 | 748 | "Main thread would deadlock", | | 749 | + "External buffers are not allowed", | 749 | 750 | }; | 750 | 751 |
| 751 | 752 | napi_status NAPI_CDECL napi_get_last_error_info( | @@ -757,7 +758,7 @@ napi_status NAPI_CDECL napi_get_last_error_info( | 757 | 758 | // message in the `napi_status` enum each time a new error message is added. | 758 | 759 | // We don't have a napi_status_last as this would result in an ABI | 759 | 760 | // 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; | 761 | 762 |
| 762 | 763 | static_assert(NAPI_ARRAYSIZE(error_messages) == last_status + 1, | 763 | 764 | "Count of error messages must match count of error values"); | |
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 |
|---|
@@ -950,6 +950,10 @@ napi_status NAPI_CDECL napi_create_external_buffer(napi_env env, | 950 | 950 | NAPI_PREAMBLE(env); | 951 | 951 | CHECK_ARG(env, result); | 952 | 952 |
| | 953 | +#if defined(V8_ENABLE_SANDBOX) | | 954 | + return napi_set_last_error(env, napi_no_external_buffers_allowed); | | 955 | +#endif | | 956 | + | 953 | 957 | v8::Isolate* isolate = env->isolate; | 954 | 958 |
| 955 | 959 | // 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 |
|---|
@@ -153,13 +153,15 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer(napi_env env, | 153 | 153 | size_t length, | 154 | 154 | void** data, | 155 | 155 | napi_value* result); | | 156 | +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 156 | 157 | NAPI_EXTERN napi_status NAPI_CDECL | 157 | 158 | napi_create_external_buffer(napi_env env, | 158 | 159 | size_t length, | 159 | 160 | void* data, | 160 | 161 | napi_finalize finalize_cb, | 161 | 162 | void* finalize_hint, | 162 | 163 | napi_value* result); | | 164 | +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED | 163 | 165 | NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer_copy(napi_env env, | 164 | 166 | size_t length, | 165 | 167 | 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" | |
|