cherry-pick-09ae62b.patch 9.0 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: Michael Dawson <[email protected]>
  3. Date: Tue, 25 Oct 2022 17:39:41 -0400
  4. Subject: node-api: handle no support for external buffers
  5. Refs: https://github.com/electron/electron/issues/35801
  6. Refs: https://github.com/nodejs/abi-stable-node/issues/441
  7. Electron recently dropped support for external
  8. buffers. Provide a way for addon authors to:
  9. - hide the methods to create external buffers so they can
  10. avoid using them if they want the broadest compatibility.
  11. - call the methods that create external buffers at runtime
  12. to check if external buffers are supported and either
  13. use them or not based on the return code.
  14. Signed-off-by: Michael Dawson <[email protected]>
  15. PR-URL: https://github.com/nodejs/node/pull/45181
  16. Reviewed-By: Chengzhong Wu <[email protected]>
  17. Reviewed-By: Minwoo Jung <[email protected]>
  18. diff --git a/doc/api/n-api.md b/doc/api/n-api.md
  19. index 71a0081352d3497b9d0c12666b2c6e3ef6da650a..6ba74e5dc7a358ea3dc1e1ce6cd9636a6143c371 100644
  20. --- a/doc/api/n-api.md
  21. +++ b/doc/api/n-api.md
  22. @@ -579,6 +579,7 @@ typedef enum {
  23. napi_arraybuffer_expected,
  24. napi_detachable_arraybuffer_expected,
  25. napi_would_deadlock, /* unused */
  26. + napi_no_external_buffers_allowed
  27. } napi_status;
  28. ```
  29. @@ -2399,6 +2400,19 @@ napi_create_external_arraybuffer(napi_env env,
  30. Returns `napi_ok` if the API succeeded.
  31. +**Some runtimes other than Node.js have dropped support for external buffers**.
  32. +On runtimes other than Node.js this method may return
  33. +`napi_no_external_buffers_allowed` to indicate that external
  34. +buffers are not supported. One such runtime is Electron as
  35. +described in this issue
  36. +[electron/issues/35801](https://github.com/electron/electron/issues/35801).
  37. +
  38. +In order to maintain broadest compatibility with all runtimes
  39. +you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
  40. +includes for the node-api headers. Doing so will hide the 2 functions
  41. +that create external buffers. This will ensure a compilation error
  42. +occurs if you accidentally use one of these methods.
  43. +
  44. This API returns a Node-API value corresponding to a JavaScript `ArrayBuffer`.
  45. The underlying byte buffer of the `ArrayBuffer` is externally allocated and
  46. managed. The caller must ensure that the byte buffer remains valid until the
  47. @@ -2443,6 +2457,19 @@ napi_status napi_create_external_buffer(napi_env env,
  48. Returns `napi_ok` if the API succeeded.
  49. +**Some runtimes other than Node.js have dropped support for external buffers**.
  50. +On runtimes other than Node.js this method may return
  51. +`napi_no_external_buffers_allowed` to indicate that external
  52. +buffers are not supported. One such runtime is Electron as
  53. +described in this issue
  54. +[electron/issues/35801](https://github.com/electron/electron/issues/35801).
  55. +
  56. +In order to maintain broadest compatibility with all runtimes
  57. +you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before
  58. +includes for the node-api headers. Doing so will hide the 2 functions
  59. +that create external buffers. This will ensure a compilation error
  60. +occurs if you accidentally use one of these methods.
  61. +
  62. This API allocates a `node::Buffer` object and initializes it with data
  63. backed by the passed in buffer. While this is still a fully-supported data
  64. structure, in most cases using a `TypedArray` will suffice.
  65. diff --git a/src/js_native_api.h b/src/js_native_api.h
  66. index 220d140d4bfe9a076c739cdc2b03ea22963aa704..d11a2c5a18cf16ece1c74b735886b5ee0c7ec009 100644
  67. --- a/src/js_native_api.h
  68. +++ b/src/js_native_api.h
  69. @@ -401,6 +401,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_arraybuffer(napi_env env,
  70. size_t byte_length,
  71. void** data,
  72. napi_value* result);
  73. +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  74. NAPI_EXTERN napi_status NAPI_CDECL
  75. napi_create_external_arraybuffer(napi_env env,
  76. void* external_data,
  77. @@ -408,6 +409,7 @@ napi_create_external_arraybuffer(napi_env env,
  78. napi_finalize finalize_cb,
  79. void* finalize_hint,
  80. napi_value* result);
  81. +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  82. NAPI_EXTERN napi_status NAPI_CDECL napi_get_arraybuffer_info(
  83. napi_env env, napi_value arraybuffer, void** data, size_t* byte_length);
  84. NAPI_EXTERN napi_status NAPI_CDECL napi_is_typedarray(napi_env env,
  85. diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
  86. index 376930ba4a32206097bc2f20f4c04c9910c231e0..517614950a90f75330e7afeafcc4ed180308c871 100644
  87. --- a/src/js_native_api_types.h
  88. +++ b/src/js_native_api_types.h
  89. @@ -98,7 +98,8 @@ typedef enum {
  90. napi_date_expected,
  91. napi_arraybuffer_expected,
  92. napi_detachable_arraybuffer_expected,
  93. - napi_would_deadlock // unused
  94. + napi_would_deadlock, // unused
  95. + napi_no_external_buffers_allowed
  96. } napi_status;
  97. // Note: when adding a new enum value to `napi_status`, please also update
  98. // * `const int last_status` in the definition of `napi_get_last_error_info()'
  99. diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
  100. index 5f8e21e0b58c8a8fea1e3ee2e0eecf5b892e4683..bafcb9bec5bafdb775b467476c94f44ca8c4b64e 100644
  101. --- a/src/js_native_api_v8.cc
  102. +++ b/src/js_native_api_v8.cc
  103. @@ -748,6 +748,7 @@ static const char* error_messages[] = {
  104. "An arraybuffer was expected",
  105. "A detachable arraybuffer was expected",
  106. "Main thread would deadlock",
  107. + "External buffers are not allowed",
  108. };
  109. napi_status NAPI_CDECL napi_get_last_error_info(
  110. @@ -759,7 +760,7 @@ napi_status NAPI_CDECL napi_get_last_error_info(
  111. // message in the `napi_status` enum each time a new error message is added.
  112. // We don't have a napi_status_last as this would result in an ABI
  113. // change each time a message was added.
  114. - const int last_status = napi_would_deadlock;
  115. + const int last_status = napi_no_external_buffers_allowed;
  116. static_assert(NAPI_ARRAYSIZE(error_messages) == last_status + 1,
  117. "Count of error messages must match count of error values");
  118. diff --git a/src/node_api.cc b/src/node_api.cc
  119. index 08352b0a584b543cf1ad2947f820d8a5dc7fe0db..26e0292083cd760b7f6aeddca3527631652c5d03 100644
  120. --- a/src/node_api.cc
  121. +++ b/src/node_api.cc
  122. @@ -949,6 +949,10 @@ napi_status NAPI_CDECL napi_create_external_buffer(napi_env env,
  123. NAPI_PREAMBLE(env);
  124. CHECK_ARG(env, result);
  125. +#if defined(V8_ENABLE_SANDBOX)
  126. + return napi_set_last_error(env, napi_no_external_buffers_allowed);
  127. +#endif
  128. +
  129. v8::Isolate* isolate = env->isolate;
  130. // The finalizer object will delete itself after invoking the callback.
  131. diff --git a/src/node_api.h b/src/node_api.h
  132. index 982b41cadf20aed89c3c61feeec8e74ed875e25c..07af926cafb536a25c99ada43cdf0fc1c2f72896 100644
  133. --- a/src/node_api.h
  134. +++ b/src/node_api.h
  135. @@ -153,6 +153,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer(napi_env env,
  136. size_t length,
  137. void** data,
  138. napi_value* result);
  139. +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  140. NAPI_EXTERN napi_status NAPI_CDECL
  141. napi_create_external_buffer(napi_env env,
  142. size_t length,
  143. @@ -160,6 +161,7 @@ napi_create_external_buffer(napi_env env,
  144. napi_finalize finalize_cb,
  145. void* finalize_hint,
  146. napi_value* result);
  147. +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  148. NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer_copy(napi_env env,
  149. size_t length,
  150. const void* data,
  151. diff --git a/test/js-native-api/test_general/test_general.c b/test/js-native-api/test_general/test_general.c
  152. index 7b755ce9a9f202eaf91e5103d6c0204925f99cac..b474ab442cb763cb84ec77901da91a6f1471c946 100644
  153. --- a/test/js-native-api/test_general/test_general.c
  154. +++ b/test/js-native-api/test_general/test_general.c
  155. @@ -1,3 +1,8 @@
  156. +// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to
  157. +// validate that it can be used as a form of test itself. It is
  158. +// not related to any of the other tests
  159. +// defined in the file
  160. +#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  161. #include <stdio.h>
  162. #include <stdlib.h>
  163. #include <stdint.h>
  164. diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c
  165. index d430e2df4f3520fddbc5ce8d260adba565e6c3c9..b8d837d5e45650fcb9ba04030721b0f51377f078 100644
  166. --- a/test/node-api/test_general/test_general.c
  167. +++ b/test/node-api/test_general/test_general.c
  168. @@ -1,4 +1,9 @@
  169. #define NAPI_EXPERIMENTAL
  170. +// we define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED here to validate that it can
  171. +// be used as a form of test itself. It is
  172. +// not related to any of the other tests
  173. +// defined in the file
  174. +#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
  175. #include <node_api.h>
  176. #include <stdlib.h>
  177. #include "../../js-native-api/common.h"