From 9aaeff3b56fd81f7f745e63a8c34e37bf1679afe Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 9 Dec 2022 03:25:07 -0800 Subject: [PATCH 1/2] chore: cherry-pick 09ae62b from node --- patches/node/.patches | 1 + patches/node/cherry-pick-09ae62b.patch | 201 +++++++++++++++++++++++++ 2 files changed, 202 insertions(+) create mode 100644 patches/node/cherry-pick-09ae62b.patch diff --git a/patches/node/.patches b/patches/node/.patches index 2897a14ae0757..ba016afe7ea1d 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -47,3 +47,4 @@ build_ensure_native_module_compilation_fails_if_not_using_a_new.patch buffer_fix_atob_input_validation.patch fix_expose_the_built-in_electron_module_via_the_esm_loader.patch chore_enable_c_17_for_native_modules.patch +cherry-pick-09ae62b.patch diff --git a/patches/node/cherry-pick-09ae62b.patch b/patches/node/cherry-pick-09ae62b.patch new file mode 100644 index 0000000000000..9dd50d00380ee --- /dev/null +++ b/patches/node/cherry-pick-09ae62b.patch @@ -0,0 +1,201 @@ +From 09ae62b9a869ff19d634b0fd1f5a17f198cd1ae7 Mon Sep 17 00:00:00 2001 +From: Michael Dawson +Date: Tue, 25 Oct 2022 17:39:41 -0400 +Subject: [PATCH] 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 + +PR-URL: https://github.com/nodejs/node/pull/45181 +Reviewed-By: Chengzhong Wu +Reviewed-By: Minwoo Jung +--- + doc/api/n-api.md | 27 +++++++++++++++++++ + src/js_native_api.h | 2 ++ + src/js_native_api_types.h | 3 ++- + src/js_native_api_v8.cc | 3 ++- + src/node_api.cc | 4 +++ + src/node_api.h | 2 ++ + .../js-native-api/test_general/test_general.c | 5 ++++ + test/node-api/test_general/test_general.c | 5 ++++ + 8 files changed, 49 insertions(+), 2 deletions(-) + +diff --git a/doc/api/n-api.md b/doc/api/n-api.md +index 3a2b9bceb2ab..b0b89de1c75b 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; + ``` + +@@ -2403,6 +2404,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 +@@ -2447,6 +2461,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 220d140d4bfe..d11a2c5a18cf 100644 +--- a/src/js_native_api.h ++++ b/src/js_native_api.h +@@ -401,6 +401,7 @@ NAPI_EXTERN napi_status NAPI_CDECL 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_CDECL + napi_create_external_arraybuffer(napi_env env, + void* external_data, +@@ -408,6 +409,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_CDECL napi_get_arraybuffer_info( + napi_env env, napi_value arraybuffer, void** data, size_t* byte_length); + NAPI_EXTERN napi_status NAPI_CDECL napi_is_typedarray(napi_env env, +diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h +index 376930ba4a32..517614950a90 100644 +--- a/src/js_native_api_types.h ++++ b/src/js_native_api_types.h +@@ -98,7 +98,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 58567c5e44a9..9000175d9d86 100644 +--- a/src/js_native_api_v8.cc ++++ b/src/js_native_api_v8.cc +@@ -746,6 +746,7 @@ static const char* error_messages[] = { + "An arraybuffer was expected", + "A detachable arraybuffer was expected", + "Main thread would deadlock", ++ "External buffers are not allowed", + }; + + napi_status NAPI_CDECL napi_get_last_error_info( +@@ -757,7 +758,7 @@ napi_status NAPI_CDECL napi_get_last_error_info( + // 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, + "Count of error messages must match count of error values"); +diff --git a/src/node_api.cc b/src/node_api.cc +index a37d354eaf0e..15fe8a07f705 100644 +--- a/src/node_api.cc ++++ b/src/node_api.cc +@@ -950,6 +950,10 @@ napi_status NAPI_CDECL 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 3dc17f31f687..8e6af2cf284e 100644 +--- a/src/node_api.h ++++ b/src/node_api.h +@@ -153,6 +153,7 @@ NAPI_EXTERN napi_status NAPI_CDECL 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_CDECL + napi_create_external_buffer(napi_env env, + size_t length, +@@ -160,6 +161,7 @@ napi_create_external_buffer(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_CDECL 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 7b755ce9a9f2..b474ab442cb7 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 + #include + #include +diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c +index d430e2df4f35..b8d837d5e456 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 + #include + #include "../../js-native-api/common.h" From 82962c1c37fc7789f7fcd200571076e27381c13f Mon Sep 17 00:00:00 2001 From: VerteDinde Date: Mon, 12 Dec 2022 21:05:25 -0800 Subject: [PATCH 2/2] chore: fixup patch --- patches/node/cherry-pick-09ae62b.patch | 145 +++++++++++++++---------- 1 file changed, 90 insertions(+), 55 deletions(-) diff --git a/patches/node/cherry-pick-09ae62b.patch b/patches/node/cherry-pick-09ae62b.patch index 9dd50d00380ee..d41a1ac1b6477 100644 --- a/patches/node/cherry-pick-09ae62b.patch +++ b/patches/node/cherry-pick-09ae62b.patch @@ -1,7 +1,7 @@ -From 09ae62b9a869ff19d634b0fd1f5a17f198cd1ae7 Mon Sep 17 00:00:00 2001 +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 25 Oct 2022 17:39:41 -0400 -Subject: [PATCH] node-api: handle no support for external buffers +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 @@ -19,19 +19,9 @@ Signed-off-by: Michael Dawson PR-URL: https://github.com/nodejs/node/pull/45181 Reviewed-By: Chengzhong Wu Reviewed-By: Minwoo Jung ---- - doc/api/n-api.md | 27 +++++++++++++++++++ - src/js_native_api.h | 2 ++ - src/js_native_api_types.h | 3 ++- - src/js_native_api_v8.cc | 3 ++- - src/node_api.cc | 4 +++ - src/node_api.h | 2 ++ - .../js-native-api/test_general/test_general.c | 5 ++++ - test/node-api/test_general/test_general.c | 5 ++++ - 8 files changed, 49 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md -index 3a2b9bceb2ab..b0b89de1c75b 100644 +index 3d1741bad82359af6a258fb2a059656f1528bba0..d64f80e2635074df7f8ecd72d1fde7d1747786fa 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -579,6 +579,7 @@ typedef enum { @@ -42,7 +32,7 @@ index 3a2b9bceb2ab..b0b89de1c75b 100644 } napi_status; ``` -@@ -2403,6 +2404,19 @@ napi_create_external_arraybuffer(napi_env env, +@@ -2394,6 +2395,19 @@ napi_create_external_arraybuffer(napi_env env, Returns `napi_ok` if the API succeeded. @@ -62,7 +52,7 @@ index 3a2b9bceb2ab..b0b89de1c75b 100644 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 -@@ -2447,6 +2461,19 @@ napi_status napi_create_external_buffer(napi_env env, +@@ -2438,6 +2452,19 @@ napi_status napi_create_external_buffer(napi_env env, Returns `napi_ok` if the API succeeded. @@ -83,30 +73,30 @@ index 3a2b9bceb2ab..b0b89de1c75b 100644 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 220d140d4bfe..d11a2c5a18cf 100644 +index 50ccf11e2405802f1c48764067b4010e8b9a0815..2ddf3780e8bde8df59b202c0913cf6434a6581ce 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h -@@ -401,6 +401,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_arraybuffer(napi_env env, - size_t byte_length, - void** data, - napi_value* result); +@@ -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_CDECL + NAPI_EXTERN napi_status napi_create_external_arraybuffer(napi_env env, void* external_data, -@@ -408,6 +409,7 @@ napi_create_external_arraybuffer(napi_env env, +@@ -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_CDECL napi_get_arraybuffer_info( - napi_env env, napi_value arraybuffer, void** data, size_t* byte_length); - NAPI_EXTERN napi_status NAPI_CDECL napi_is_typedarray(napi_env env, + 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 376930ba4a32..517614950a90 100644 +index 6aba06629b31543c13698dbb02b82db309587c4a..7529b853655a25dd6f945df77c9dd024a0403b26 100644 --- a/src/js_native_api_types.h +++ b/src/js_native_api_types.h -@@ -98,7 +98,8 @@ typedef enum { +@@ -92,7 +92,8 @@ typedef enum { napi_date_expected, napi_arraybuffer_expected, napi_detachable_arraybuffer_expected, @@ -117,31 +107,77 @@ index 376930ba4a32..517614950a90 100644 // 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 58567c5e44a9..9000175d9d86 100644 +index 1c29c43836a0c35a832e494f8eaebbbe1eee8ea4..108ba4bedbd1684b9a69834ae12347faf4670a27 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc -@@ -746,6 +746,7 @@ static const char* error_messages[] = { - "An arraybuffer was expected", - "A detachable arraybuffer was expected", - "Main thread would deadlock", +@@ -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_CDECL napi_get_last_error_info( -@@ -757,7 +758,7 @@ napi_status NAPI_CDECL napi_get_last_error_info( + 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, - "Count of error messages must match count of error values"); + static_assert( + NAPI_ARRAYSIZE(error_messages) == last_status + 1, diff --git a/src/node_api.cc b/src/node_api.cc -index a37d354eaf0e..15fe8a07f705 100644 +index 60fbe96b8ef272768736ce029bac3ff72f3c84a5..6f8575bb8f289aab041bc126b2fd5f53b1116af5 100644 --- a/src/node_api.cc +++ b/src/node_api.cc -@@ -950,6 +950,10 @@ napi_status NAPI_CDECL napi_create_external_buffer(napi_env env, +@@ -929,6 +929,10 @@ napi_status napi_create_external_buffer(napi_env env, NAPI_PREAMBLE(env); CHECK_ARG(env, result); @@ -153,27 +189,26 @@ index a37d354eaf0e..15fe8a07f705 100644 // The finalizer object will delete itself after invoking the callback. diff --git a/src/node_api.h b/src/node_api.h -index 3dc17f31f687..8e6af2cf284e 100644 +index 1772c67c15afb2d2712b1900a584f627852e3d7e..47703198fed09a61c3e9e06fa5781d340cc39cf9 100644 --- a/src/node_api.h +++ b/src/node_api.h -@@ -153,6 +153,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer(napi_env env, - size_t length, - void** data, - napi_value* result); +@@ -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_CDECL - napi_create_external_buffer(napi_env env, - size_t length, -@@ -160,6 +161,7 @@ napi_create_external_buffer(napi_env env, - napi_finalize finalize_cb, - void* finalize_hint, - napi_value* result); + 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_CDECL napi_create_buffer_copy(napi_env env, - size_t length, - const void* data, + 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 7b755ce9a9f2..b474ab442cb7 100644 +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 @@ @@ -186,7 +221,7 @@ index 7b755ce9a9f2..b474ab442cb7 100644 #include #include diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c -index d430e2df4f35..b8d837d5e456 100644 +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 @@