-
Notifications
You must be signed in to change notification settings - Fork 15k
/
cherry-pick-09ae62b.patch
236 lines (221 loc) · 10.9 KB
/
cherry-pick-09ae62b.patch
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
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"