From bedaacf77d921b252c02f268a932c0f74d8c9b1b Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Thu, 5 Sep 2024 10:35:30 +0800 Subject: [PATCH 1/2] add napi_create_buffer_from_arraybuffer (nodejs/node#54505) --- packages/emnapi/include/node/node_api.h | 12 +++++ packages/emnapi/src/value/create.ts | 57 +++++++++++++++++++++++ packages/test/buffer/binding.c | 60 +++++++++++++++++++++++++ packages/test/buffer/buffer.test.js | 2 + 4 files changed, 131 insertions(+) diff --git a/packages/emnapi/include/node/node_api.h b/packages/emnapi/include/node/node_api.h index 7fa08bf4..aa258d4e 100644 --- a/packages/emnapi/include/node/node_api.h +++ b/packages/emnapi/include/node/node_api.h @@ -145,6 +145,18 @@ napi_create_external_buffer(napi_env env, void* finalize_hint, napi_value* result); #endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + +#ifdef NAPI_EXPERIMENTAL +#define NODE_API_EXPERIMENTAL_HAS_CREATE_BUFFER_FROM_ARRAYBUFFER + +NAPI_EXTERN napi_status NAPI_CDECL +node_api_create_buffer_from_arraybuffer(napi_env env, + napi_value arraybuffer, + size_t byte_offset, + size_t byte_length, + napi_value* result); +#endif // NAPI_EXPERIMENTAL + NAPI_EXTERN napi_status NAPI_CDECL napi_create_buffer_copy(napi_env env, size_t length, const void* data, diff --git a/packages/emnapi/src/value/create.ts b/packages/emnapi/src/value/create.ts index 520bb59f..ccb7995e 100644 --- a/packages/emnapi/src/value/create.ts +++ b/packages/emnapi/src/value/create.ts @@ -412,6 +412,63 @@ export function napi_create_external_buffer ( ) } +/** + * @__sig ippppp + */ +export function node_api_create_buffer_from_arraybuffer ( + env: napi_env, + arraybuffer: napi_value, + byte_offset: size_t, + byte_length: size_t, + result: Pointer +): napi_status { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + let value: number + + return $PREAMBLE!(env, (envObject) => { + $CHECK_ARG!(envObject, arraybuffer) + $CHECK_ARG!(envObject, result) + from64('byte_offset') + from64('byte_length') + byte_offset = byte_offset >>> 0 + byte_length = byte_length >>> 0 + const handle = emnapiCtx.handleStore.get(arraybuffer)! + const buffer = handle.value + if (!(buffer instanceof ArrayBuffer)) { + return envObject.setLastError(napi_status.napi_invalid_arg) + } + + if ((byte_length + byte_offset) > buffer.byteLength) { + const err: RangeError & { code?: string } = new RangeError('The byte offset + length is out of range') + err.code = 'ERR_OUT_OF_RANGE' + throw err + } + + const Buffer = emnapiCtx.feature.Buffer! + if (!Buffer) { + throw emnapiCtx.createNotSupportBufferError('node_api_create_buffer_from_arraybuffer', '') + } + const out = Buffer.from(buffer, byte_offset, byte_length) + if (buffer === wasmMemory.buffer) { + if (!emnapiExternalMemory.wasmMemoryViewTable.has(out)) { + emnapiExternalMemory.wasmMemoryViewTable.set(out, { + Ctor: Buffer, + address: byte_offset, + length: byte_length, + ownership: ReferenceOwnership.kUserland, + runtimeAllocated: 0 + }) + } + } + from64('result') + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + value = emnapiCtx.addToCurrentScope(out).id + makeSetValue('result', 0, 'value', '*') + return envObject.getReturnStatus() + }) +} + /** * @__sig ippppp */ diff --git a/packages/test/buffer/binding.c b/packages/test/buffer/binding.c index 54735849..37f1f6aa 100644 --- a/packages/test/buffer/binding.c +++ b/packages/test/buffer/binding.c @@ -190,6 +190,64 @@ static napi_value getMemoryDataAsArray(napi_env env, napi_callback_info info) { return ret; } +NAPI_EXTERN napi_status NAPI_CDECL +node_api_create_buffer_from_arraybuffer(napi_env env, + napi_value arraybuffer, + size_t byte_offset, + size_t byte_length, + napi_value* result); + +static napi_value testBufferFromArrayBuffer(napi_env env, + napi_callback_info info) { + napi_status status; + napi_value arraybuffer; + void* data; + napi_value buffer; + size_t byte_length = 1024; + size_t byte_offset = 0; + + status = napi_create_arraybuffer(env, byte_length, &data, &arraybuffer); + NODE_API_ASSERT(env, status == napi_ok, "Failed to create arraybuffer"); + + status = node_api_create_buffer_from_arraybuffer( + env, arraybuffer, byte_offset, byte_length, &buffer); + NODE_API_ASSERT( + env, status == napi_ok, "Failed to create buffer from arraybuffer"); + + void* buffer_data; + size_t buffer_length; + status = napi_get_buffer_info(env, buffer, &buffer_data, &buffer_length); + NODE_API_ASSERT(env, status == napi_ok, "Failed to get buffer info"); + NODE_API_ASSERT(env, buffer_length == byte_length, "Buffer length mismatch"); + + bool is_buffer; + status = napi_is_buffer(env, buffer, &is_buffer); + NODE_API_ASSERT(env, status == napi_ok, "Failed to check if value is buffer"); + NODE_API_ASSERT(env, is_buffer, "Expected a Buffer but did not get one"); + + status = node_api_create_buffer_from_arraybuffer( + env, arraybuffer, byte_length, byte_length + 1, &buffer); + NODE_API_ASSERT(env, + status == 10, + "Expected range error for invalid byte offset"); + napi_value last_error; + status = napi_get_and_clear_last_exception(env, &last_error); + NODE_API_ASSERT(env, status == napi_ok, "Failed to call napi_get_and_clear_last_exception"); + + napi_value non_arraybuffer; + status = napi_create_uint32(env, 123, &non_arraybuffer); + NODE_API_ASSERT( + env, status == napi_ok, "Failed to create non-arraybuffer value"); + + status = node_api_create_buffer_from_arraybuffer( + env, non_arraybuffer, byte_offset, byte_length, &buffer); + NODE_API_ASSERT(env, + status == napi_invalid_arg, + "Expected invalid arg error for non-arraybuffer input"); + + return arraybuffer; +} + static napi_value Init(napi_env env, napi_value exports) { napi_value theValue; @@ -208,6 +266,8 @@ static napi_value Init(napi_env env, napi_value exports) { DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer), DECLARE_NODE_API_PROPERTY("invalidObjectAsBuffer", invalidObjectAsBuffer), DECLARE_NODE_API_PROPERTY("getMemoryDataAsArray", getMemoryDataAsArray), + DECLARE_NODE_API_PROPERTY("testBufferFromArrayBuffer", + testBufferFromArrayBuffer), }; NODE_API_CALL(env, napi_define_properties( diff --git a/packages/test/buffer/buffer.test.js b/packages/test/buffer/buffer.test.js index 4f3f13e9..48bb1875 100644 --- a/packages/test/buffer/buffer.test.js +++ b/packages/test/buffer/buffer.test.js @@ -58,4 +58,6 @@ module.exports = load('buffer').then(async binding => { assert.deepStrictEqual(binding.getMemoryDataAsArray(buffer), Array(6).fill(99)) assert.deepStrictEqual(binding.getMemoryDataAsArray(typedArray), Array(6).fill(99)) assert.deepStrictEqual(binding.getMemoryDataAsArray(dataView), Array(6).fill(99)) + + binding.testBufferFromArrayBuffer() }) From c3fbe0209489e1493b4ab845d60cc3a3a92dbc5a Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Sat, 12 Oct 2024 00:10:30 +0800 Subject: [PATCH 2/2] fix --- packages/emnapi/src/value/create.ts | 12 ++-- packages/test/CMakeLists.txt | 1 + packages/test/buffer/binding.c | 87 +++++++++++------------------ packages/test/buffer/buffer.test.js | 5 +- 4 files changed, 43 insertions(+), 62 deletions(-) diff --git a/packages/emnapi/src/value/create.ts b/packages/emnapi/src/value/create.ts index ccb7995e..2bb57457 100644 --- a/packages/emnapi/src/value/create.ts +++ b/packages/emnapi/src/value/create.ts @@ -229,10 +229,10 @@ export function napi_create_typedarray ( $CHECK_ARG!(envObject, result) const handle = emnapiCtx.handleStore.get(arraybuffer)! - const buffer = handle.value - if (!(buffer instanceof ArrayBuffer)) { + if (!handle.isArrayBuffer()) { return envObject.setLastError(napi_status.napi_invalid_arg) } + const buffer = handle.value from64('byte_offset') from64('length') @@ -433,10 +433,10 @@ export function node_api_create_buffer_from_arraybuffer ( byte_offset = byte_offset >>> 0 byte_length = byte_length >>> 0 const handle = emnapiCtx.handleStore.get(arraybuffer)! - const buffer = handle.value - if (!(buffer instanceof ArrayBuffer)) { + if (!handle.isArrayBuffer()) { return envObject.setLastError(napi_status.napi_invalid_arg) } + const buffer = handle.value if ((byte_length + byte_offset) > buffer.byteLength) { const err: RangeError & { code?: string } = new RangeError('The byte offset + length is out of range') @@ -490,10 +490,10 @@ export function napi_create_dataview ( byte_length = byte_length >>> 0 byte_offset = byte_offset >>> 0 const handle = emnapiCtx.handleStore.get(arraybuffer)! - const buffer = handle.value - if (!(buffer instanceof ArrayBuffer)) { + if (!handle.isArrayBuffer()) { return envObject.setLastError(napi_status.napi_invalid_arg) } + const buffer = handle.value if ((byte_length + byte_offset) > buffer.byteLength) { const err: RangeError & { code?: string } = new RangeError('byte_offset + byte_length should be less than or equal to the size in bytes of the array passed in') diff --git a/packages/test/CMakeLists.txt b/packages/test/CMakeLists.txt index 1f55be23..08153968 100644 --- a/packages/test/CMakeLists.txt +++ b/packages/test/CMakeLists.txt @@ -294,6 +294,7 @@ add_test("number" "./number/binding.c;./number/test_null.c" OFF) add_test("symbol" "./symbol/binding.c" OFF) add_test("typedarray" "./typedarray/binding.c" OFF) add_test("buffer" "./buffer/binding.c" OFF) +target_compile_definitions("buffer" PRIVATE "NAPI_EXPERIMENTAL") add_test("buffer_finalizer" "./buffer_finalizer/binding.c" OFF) add_test("fatal_exception" "./fatal_exception/binding.c" OFF) add_test("cleanup_hook" "./cleanup_hook/binding.c" OFF) diff --git a/packages/test/buffer/binding.c b/packages/test/buffer/binding.c index 37f1f6aa..ebc337aa 100644 --- a/packages/test/buffer/binding.c +++ b/packages/test/buffer/binding.c @@ -38,17 +38,23 @@ static const char theText[] = const unsigned int theTextSize = sizeof(theText); static int deleterCallCount = 0; -static void deleteTheText(napi_env env, void* data, void* finalize_hint) { - NODE_API_ASSERT_RETURN_VOID( - env, data != NULL && strcmp(data, theText) == 0, "invalid data"); + +static void deleteTheText(node_api_basic_env env, + void* data, + void* finalize_hint) { + NODE_API_BASIC_ASSERT_RETURN_VOID(data != NULL && strcmp(data, theText) == 0, + "invalid data"); + (void)finalize_hint; free(data); deleterCallCount++; } -static void noopDeleter(napi_env env, void* data, void* finalize_hint) { - NODE_API_ASSERT_RETURN_VOID( - env, data != NULL && strcmp(data, theText) == 0, "invalid data"); +static void noopDeleter(node_api_basic_env env, + void* data, + void* finalize_hint) { + NODE_API_BASIC_ASSERT_RETURN_VOID(data != NULL && strcmp(data, theText) == 0, + "invalid data"); (void)finalize_hint; deleterCallCount++; } @@ -75,9 +81,12 @@ static napi_value newExternalBuffer(napi_env env, napi_callback_info info) { NODE_API_ASSERT( env, theCopy, "Failed to copy static text for newExternalBuffer"); NODE_API_CALL(env, - napi_create_external_buffer( - env, theTextSize, theCopy, deleteTheText, - NULL /* finalize_hint */, &theBuffer)); + napi_create_external_buffer(env, + sizeof(theText), + theCopy, + deleteTheText, + NULL /* finalize_hint */, + &theBuffer)); return theBuffer; } @@ -134,9 +143,12 @@ static napi_value bufferInfo(napi_env env, napi_callback_info info) { static napi_value staticBuffer(napi_env env, napi_callback_info info) { napi_value theBuffer; NODE_API_CALL(env, - napi_create_external_buffer( - env, sizeof(theText), (void*)theText, noopDeleter, - NULL /* finalize_hint */, &theBuffer)); + napi_create_external_buffer(env, + sizeof(theText), + (void*)theText, + noopDeleter, + NULL /* finalize_hint */, + &theBuffer)); return theBuffer; } @@ -190,62 +202,30 @@ static napi_value getMemoryDataAsArray(napi_env env, napi_callback_info info) { return ret; } -NAPI_EXTERN napi_status NAPI_CDECL -node_api_create_buffer_from_arraybuffer(napi_env env, - napi_value arraybuffer, - size_t byte_offset, - size_t byte_length, - napi_value* result); - -static napi_value testBufferFromArrayBuffer(napi_env env, - napi_callback_info info) { +static napi_value bufferFromArrayBuffer(napi_env env, + napi_callback_info info) { napi_status status; napi_value arraybuffer; - void* data; napi_value buffer; size_t byte_length = 1024; - size_t byte_offset = 0; + void* data = NULL; + size_t buffer_length = 0; + void* buffer_data = NULL; status = napi_create_arraybuffer(env, byte_length, &data, &arraybuffer); NODE_API_ASSERT(env, status == napi_ok, "Failed to create arraybuffer"); status = node_api_create_buffer_from_arraybuffer( - env, arraybuffer, byte_offset, byte_length, &buffer); + env, arraybuffer, 0, byte_length, &buffer); NODE_API_ASSERT( env, status == napi_ok, "Failed to create buffer from arraybuffer"); - void* buffer_data; - size_t buffer_length; status = napi_get_buffer_info(env, buffer, &buffer_data, &buffer_length); NODE_API_ASSERT(env, status == napi_ok, "Failed to get buffer info"); - NODE_API_ASSERT(env, buffer_length == byte_length, "Buffer length mismatch"); - - bool is_buffer; - status = napi_is_buffer(env, buffer, &is_buffer); - NODE_API_ASSERT(env, status == napi_ok, "Failed to check if value is buffer"); - NODE_API_ASSERT(env, is_buffer, "Expected a Buffer but did not get one"); - - status = node_api_create_buffer_from_arraybuffer( - env, arraybuffer, byte_length, byte_length + 1, &buffer); - NODE_API_ASSERT(env, - status == 10, - "Expected range error for invalid byte offset"); - napi_value last_error; - status = napi_get_and_clear_last_exception(env, &last_error); - NODE_API_ASSERT(env, status == napi_ok, "Failed to call napi_get_and_clear_last_exception"); - - napi_value non_arraybuffer; - status = napi_create_uint32(env, 123, &non_arraybuffer); - NODE_API_ASSERT( - env, status == napi_ok, "Failed to create non-arraybuffer value"); - status = node_api_create_buffer_from_arraybuffer( - env, non_arraybuffer, byte_offset, byte_length, &buffer); - NODE_API_ASSERT(env, - status == napi_invalid_arg, - "Expected invalid arg error for non-arraybuffer input"); + NODE_API_ASSERT(env, buffer_length == byte_length, "Buffer length mismatch"); - return arraybuffer; + return buffer; } static napi_value Init(napi_env env, napi_value exports) { @@ -266,8 +246,7 @@ static napi_value Init(napi_env env, napi_value exports) { DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer), DECLARE_NODE_API_PROPERTY("invalidObjectAsBuffer", invalidObjectAsBuffer), DECLARE_NODE_API_PROPERTY("getMemoryDataAsArray", getMemoryDataAsArray), - DECLARE_NODE_API_PROPERTY("testBufferFromArrayBuffer", - testBufferFromArrayBuffer), + DECLARE_NODE_API_PROPERTY("bufferFromArrayBuffer", bufferFromArrayBuffer), }; NODE_API_CALL(env, napi_define_properties( diff --git a/packages/test/buffer/buffer.test.js b/packages/test/buffer/buffer.test.js index 48bb1875..1d7f95dd 100644 --- a/packages/test/buffer/buffer.test.js +++ b/packages/test/buffer/buffer.test.js @@ -21,6 +21,9 @@ module.exports = load('buffer').then(async binding => { // To test this doesn't crash binding.invalidObjectAsBuffer({}) + + const testBuffer = binding.bufferFromArrayBuffer() + assert(testBuffer instanceof Buffer, 'Expected a Buffer') })().then(common.mustCall()) process.externalBuffer = binding.newExternalBuffer() @@ -58,6 +61,4 @@ module.exports = load('buffer').then(async binding => { assert.deepStrictEqual(binding.getMemoryDataAsArray(buffer), Array(6).fill(99)) assert.deepStrictEqual(binding.getMemoryDataAsArray(typedArray), Array(6).fill(99)) assert.deepStrictEqual(binding.getMemoryDataAsArray(dataView), Array(6).fill(99)) - - binding.testBufferFromArrayBuffer() })