mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
4c2f91198b
* chore: cherry-pick 09ae62b from node * chore: fixup patch Co-authored-by: VerteDinde <vertedinde@electronjs.org>
237 lines
11 KiB
Diff
237 lines
11 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Michael Dawson <mdawson@devrus.com>
|
|
Date: Tue, 25 Oct 2022 17:39:41 -0400
|
|
Subject: node-api: handle no support for external buffers
|
|
|
|
Refs: https://github.com/electron/electron/issues/35801
|
|
Refs: https://github.com/nodejs/abi-stable-node/issues/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: https://github.com/nodejs/node/pull/45181
|
|
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
|
|
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
|
|
|
|
diff --git a/doc/api/n-api.md b/doc/api/n-api.md
|
|
index 3d1741bad82359af6a258fb2a059656f1528bba0..d64f80e2635074df7f8ecd72d1fde7d1747786fa 100644
|
|
--- a/doc/api/n-api.md
|
|
+++ b/doc/api/n-api.md
|
|
@@ -579,6 +579,7 @@ typedef enum {
|
|
napi_arraybuffer_expected,
|
|
napi_detachable_arraybuffer_expected,
|
|
napi_would_deadlock, /* unused */
|
|
+ napi_no_external_buffers_allowed
|
|
} napi_status;
|
|
```
|
|
|
|
@@ -2394,6 +2395,19 @@ napi_create_external_arraybuffer(napi_env env,
|
|
|
|
Returns `napi_ok` if the API succeeded.
|
|
|
|
+**Some runtimes other than Node.js have dropped support for external buffers**.
|
|
+On runtimes other than Node.js this method may return
|
|
+`napi_no_external_buffers_allowed` to indicate that external
|
|
+buffers are not supported. One such runtime is Electron as
|
|
+described in this issue
|
|
+[electron/issues/35801](https://github.com/electron/electron/issues/35801).
|
|
+
|
|
+In order to maintain broadest compatibility with all runtimes
|
|
+you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
|
|
+includes for the node-api headers. Doing so will hide the 2 functions
|
|
+that create external buffers. This will ensure a compilation error
|
|
+occurs if you accidentally use one of these methods.
|
|
+
|
|
This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`.
|
|
The underlying byte buffer of the `ArrayBuffer` is externally allocated and
|
|
managed. The caller must ensure that the byte buffer remains valid until the
|
|
@@ -2438,6 +2452,19 @@ napi_status napi_create_external_buffer(napi_env env,
|
|
|
|
Returns `napi_ok` if the API succeeded.
|
|
|
|
+**Some runtimes other than Node.js have dropped support for external buffers**.
|
|
+On runtimes other than Node.js this method may return
|
|
+`napi_no_external_buffers_allowed` to indicate that external
|
|
+buffers are not supported. One such runtime is Electron as
|
|
+described in this issue
|
|
+[electron/issues/35801](https://github.com/electron/electron/issues/35801).
|
|
+
|
|
+In order to maintain broadest compatibility with all runtimes
|
|
+you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
|
|
+includes for the node-api headers. Doing so will hide the 2 functions
|
|
+that create external buffers. This will ensure a compilation error
|
|
+occurs if you accidentally use one of these methods.
|
|
+
|
|
This API allocates a `node::Buffer` object and initializes it with data
|
|
backed by the passed in buffer. While this is still a fully-supported data
|
|
structure, in most cases using a `TypedArray` will suffice.
|
|
diff --git a/src/js_native_api.h b/src/js_native_api.h
|
|
index 50ccf11e2405802f1c48764067b4010e8b9a0815..2ddf3780e8bde8df59b202c0913cf6434a6581ce 100644
|
|
--- a/src/js_native_api.h
|
|
+++ b/src/js_native_api.h
|
|
@@ -404,6 +404,7 @@ NAPI_EXTERN napi_status napi_create_arraybuffer(napi_env env,
|
|
size_t byte_length,
|
|
void** data,
|
|
napi_value* result);
|
|
+#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
NAPI_EXTERN napi_status
|
|
napi_create_external_arraybuffer(napi_env env,
|
|
void* external_data,
|
|
@@ -411,6 +412,7 @@ napi_create_external_arraybuffer(napi_env env,
|
|
napi_finalize finalize_cb,
|
|
void* finalize_hint,
|
|
napi_value* result);
|
|
+#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
NAPI_EXTERN napi_status napi_get_arraybuffer_info(napi_env env,
|
|
napi_value arraybuffer,
|
|
void** data,
|
|
diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
|
|
index 6aba06629b31543c13698dbb02b82db309587c4a..7529b853655a25dd6f945df77c9dd024a0403b26 100644
|
|
--- a/src/js_native_api_types.h
|
|
+++ b/src/js_native_api_types.h
|
|
@@ -92,7 +92,8 @@ typedef enum {
|
|
napi_date_expected,
|
|
napi_arraybuffer_expected,
|
|
napi_detachable_arraybuffer_expected,
|
|
- napi_would_deadlock // unused
|
|
+ napi_would_deadlock, // unused
|
|
+ napi_no_external_buffers_allowed
|
|
} napi_status;
|
|
// Note: when adding a new enum value to `napi_status`, please also update
|
|
// * `const int last_status` in the definition of `napi_get_last_error_info()'
|
|
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
|
|
index 1c29c43836a0c35a832e494f8eaebbbe1eee8ea4..108ba4bedbd1684b9a69834ae12347faf4670a27 100644
|
|
--- a/src/js_native_api_v8.cc
|
|
+++ b/src/js_native_api_v8.cc
|
|
@@ -721,29 +721,30 @@ void Reference::SecondPassCallback(
|
|
} // end of namespace v8impl
|
|
|
|
// Warning: Keep in-sync with napi_status enum
|
|
-static
|
|
-const char* error_messages[] = {nullptr,
|
|
- "Invalid argument",
|
|
- "An object was expected",
|
|
- "A string was expected",
|
|
- "A string or symbol was expected",
|
|
- "A function was expected",
|
|
- "A number was expected",
|
|
- "A boolean was expected",
|
|
- "An array was expected",
|
|
- "Unknown failure",
|
|
- "An exception is pending",
|
|
- "The async work item was cancelled",
|
|
- "napi_escape_handle already called on scope",
|
|
- "Invalid handle scope usage",
|
|
- "Invalid callback scope usage",
|
|
- "Thread-safe function queue is full",
|
|
- "Thread-safe function handle is closing",
|
|
- "A bigint was expected",
|
|
- "A date was expected",
|
|
- "An arraybuffer was expected",
|
|
- "A detachable arraybuffer was expected",
|
|
- "Main thread would deadlock",
|
|
+static const char* error_messages[] = {
|
|
+ nullptr,
|
|
+ "Invalid argument",
|
|
+ "An object was expected",
|
|
+ "A string was expected",
|
|
+ "A string or symbol was expected",
|
|
+ "A function was expected",
|
|
+ "A number was expected",
|
|
+ "A boolean was expected",
|
|
+ "An array was expected",
|
|
+ "Unknown failure",
|
|
+ "An exception is pending",
|
|
+ "The async work item was cancelled",
|
|
+ "napi_escape_handle already called on scope",
|
|
+ "Invalid handle scope usage",
|
|
+ "Invalid callback scope usage",
|
|
+ "Thread-safe function queue is full",
|
|
+ "Thread-safe function handle is closing",
|
|
+ "A bigint was expected",
|
|
+ "A date was expected",
|
|
+ "An arraybuffer was expected",
|
|
+ "A detachable arraybuffer was expected",
|
|
+ "Main thread would deadlock",
|
|
+ "External buffers are not allowed",
|
|
};
|
|
|
|
napi_status napi_get_last_error_info(napi_env env,
|
|
@@ -755,7 +756,7 @@ napi_status napi_get_last_error_info(napi_env env,
|
|
// message in the `napi_status` enum each time a new error message is added.
|
|
// We don't have a napi_status_last as this would result in an ABI
|
|
// change each time a message was added.
|
|
- const int last_status = napi_would_deadlock;
|
|
+ const int last_status = napi_no_external_buffers_allowed;
|
|
|
|
static_assert(
|
|
NAPI_ARRAYSIZE(error_messages) == last_status + 1,
|
|
diff --git a/src/node_api.cc b/src/node_api.cc
|
|
index 60fbe96b8ef272768736ce029bac3ff72f3c84a5..6f8575bb8f289aab041bc126b2fd5f53b1116af5 100644
|
|
--- a/src/node_api.cc
|
|
+++ b/src/node_api.cc
|
|
@@ -929,6 +929,10 @@ napi_status napi_create_external_buffer(napi_env env,
|
|
NAPI_PREAMBLE(env);
|
|
CHECK_ARG(env, result);
|
|
|
|
+#if defined(V8_ENABLE_SANDBOX)
|
|
+ return napi_set_last_error(env, napi_no_external_buffers_allowed);
|
|
+#endif
|
|
+
|
|
v8::Isolate* isolate = env->isolate;
|
|
|
|
// The finalizer object will delete itself after invoking the callback.
|
|
diff --git a/src/node_api.h b/src/node_api.h
|
|
index 1772c67c15afb2d2712b1900a584f627852e3d7e..47703198fed09a61c3e9e06fa5781d340cc39cf9 100644
|
|
--- a/src/node_api.h
|
|
+++ b/src/node_api.h
|
|
@@ -138,12 +138,14 @@ NAPI_EXTERN napi_status napi_create_buffer(napi_env env,
|
|
size_t length,
|
|
void** data,
|
|
napi_value* result);
|
|
+#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
NAPI_EXTERN napi_status napi_create_external_buffer(napi_env env,
|
|
size_t length,
|
|
void* data,
|
|
napi_finalize finalize_cb,
|
|
void* finalize_hint,
|
|
napi_value* result);
|
|
+#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
NAPI_EXTERN napi_status napi_create_buffer_copy(napi_env env,
|
|
size_t length,
|
|
const void* data,
|
|
diff --git a/test/js-native-api/test_general/test_general.c b/test/js-native-api/test_general/test_general.c
|
|
index 7b755ce9a9f202eaf91e5103d6c0204925f99cac..b474ab442cb763cb84ec77901da91a6f1471c946 100644
|
|
--- a/test/js-native-api/test_general/test_general.c
|
|
+++ b/test/js-native-api/test_general/test_general.c
|
|
@@ -1,3 +1,8 @@
|
|
+// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to
|
|
+// validate that it can be used as a form of test itself. It is
|
|
+// not related to any of the other tests
|
|
+// defined in the file
|
|
+#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <stdint.h>
|
|
diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c
|
|
index d430e2df4f3520fddbc5ce8d260adba565e6c3c9..b8d837d5e45650fcb9ba04030721b0f51377f078 100644
|
|
--- a/test/node-api/test_general/test_general.c
|
|
+++ b/test/node-api/test_general/test_general.c
|
|
@@ -1,4 +1,9 @@
|
|
#define NAPI_EXPERIMENTAL
|
|
+// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to validate that it can
|
|
+// be used as a form of test itself. It is
|
|
+// not related to any of the other tests
|
|
+// defined in the file
|
|
+#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
|
|
#include <node_api.h>
|
|
#include <stdlib.h>
|
|
#include "../../js-native-api/common.h"
|