fix(NAPI): return detached buffer from napi_create_external_buffer if empty (#24297)

### What does this PR do?
When `napi_create_external_buffer` receives empty input, the returned
buffer should be detached.

This fixes the remaining tests in `ref-napi` other than three that use a
few uv symbols
<img width="329" height="159" alt="Screenshot 2025-11-01 at 8 38 01 PM"
src="https://github.com/user-attachments/assets/2c75f937-79c5-467a-bde3-44e45e05d9a0"
/>

### How did you verify your code works?
Added tests for correct values from `napi_get_buffer_info`,
`napi_get_arraybuffer_info`, and `napi_is_detached_arraybuffer` when
given an empty buffer from `napi_create_external_buffer`

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
Dylan Conway
2025-11-01 22:21:41 -07:00
committed by GitHub
parent 60c0fd7786
commit 42543fb544
8 changed files with 149 additions and 26 deletions

View File

@@ -1136,9 +1136,6 @@ pub const JSValue = enum(i64) {
pub fn asArrayBuffer(this: JSValue, global: *JSGlobalObject) ?ArrayBuffer {
var out: ArrayBuffer = undefined;
// `ptr` might not get set if the ArrayBuffer is empty, so make sure it starts out with a
// defined value.
out.ptr = &.{};
if (JSC__JSValue__asArrayBuffer(this, global, &out)) {
return out;
}

View File

@@ -3086,11 +3086,7 @@ bool JSC__JSValue__asArrayBuffer(
}
}
out->_value = JSValue::encode(value);
if (data) {
// Avoid setting `ptr` to null; the corresponding Zig field is a non-optional pointer.
// The caller should have already set `ptr` to a zero-length array.
out->ptr = static_cast<char*>(data);
}
out->ptr = static_cast<char*>(data);
return true;
}
@@ -6071,10 +6067,7 @@ extern "C" void JSC__ArrayBuffer__deref(JSC::ArrayBuffer* self) { self->deref();
extern "C" void JSC__ArrayBuffer__asBunArrayBuffer(JSC::ArrayBuffer* self, Bun__ArrayBuffer* out)
{
const std::size_t byteLength = self->byteLength();
if (void* data = self->data()) {
// Avoid setting `ptr` to null; it's a non-optional pointer in Zig.
out->ptr = static_cast<char*>(data);
}
out->ptr = static_cast<char*>(self->data());
out->len = byteLength;
out->byte_len = byteLength;
out->_value = 0;

View File

@@ -1994,8 +1994,12 @@ extern "C" napi_status napi_create_external_buffer(napi_env env, size_t length,
auto* subclassStructure = globalObject->JSBufferSubclassStructure();
if (data == nullptr || length == 0) {
auto* buffer = JSC::JSUint8Array::createUninitialized(globalObject, subclassStructure, 0);
// TODO: is there a way to create a detached uint8 array?
auto arrayBuffer = JSC::ArrayBuffer::createUninitialized(0, 1);
auto* buffer = JSC::JSUint8Array::create(globalObject, subclassStructure, WTFMove(arrayBuffer), 0, 0);
NAPI_RETURN_IF_EXCEPTION(env);
buffer->existingBuffer()->detach(vm);
vm.heap.addFinalizer(buffer, [env = WTF::Ref<NapiEnv>(*env), finalize_cb, data, finalize_hint](JSCell* cell) -> void {
NAPI_LOG("external buffer finalizer (empty buffer)");

View File

@@ -1,11 +1,15 @@
pub const ArrayBuffer = extern struct {
ptr: [*]u8 = &[0]u8{},
ptr: ?[*]u8 = null,
len: usize = 0,
byte_len: usize = 0,
value: jsc.JSValue = jsc.JSValue.zero,
typed_array_type: jsc.JSValue.JSType = .Cell,
shared: bool = false,
pub fn isDetached(this: *const ArrayBuffer) bool {
return this.ptr == null;
}
// require('buffer').kMaxLength.
// keep in sync with Bun::Buffer::kMaxLength
pub const max_size = std.math.maxInt(c_uint);
@@ -315,7 +319,10 @@ pub const ArrayBuffer = extern struct {
/// new ArrayBuffer(view.buffer, view.byteOffset, view.byteLength)
/// ```
pub inline fn byteSlice(this: *const @This()) []u8 {
return this.ptr[0..this.byte_len];
if (this.isDetached()) {
return &.{};
}
return this.ptr.?[0..this.byte_len];
}
/// The equivalent of
@@ -330,7 +337,10 @@ pub const ArrayBuffer = extern struct {
}
pub inline fn asU16Unaligned(this: *const @This()) []align(1) u16 {
return @ptrCast(this.ptr[0 .. this.byte_len / @sizeOf(u16) * @sizeOf(u16)]);
if (this.isDetached()) {
return &.{};
}
return @ptrCast(this.ptr.?[0 .. this.byte_len / @sizeOf(u16) * @sizeOf(u16)]);
}
pub inline fn asU32(this: *const @This()) []u32 {
@@ -338,7 +348,10 @@ pub const ArrayBuffer = extern struct {
}
pub inline fn asU32Unaligned(this: *const @This()) []align(1) u32 {
return @ptrCast(this.ptr[0 .. this.byte_len / @sizeOf(u32) * @sizeOf(u32)]);
if (this.isDetached()) {
return &.{};
}
return @ptrCast(this.ptr.?[0 .. this.byte_len / @sizeOf(u32) * @sizeOf(u32)]);
}
pub const BinaryType = enum(u4) {
@@ -668,7 +681,6 @@ pub const JSCArrayBuffer = opaque {
pub fn asArrayBuffer(self: *Self) ArrayBuffer {
var out: ArrayBuffer = undefined;
out.ptr = &.{}; // `ptr` might not get set if the ArrayBuffer is empty
JSC__ArrayBuffer__asBunArrayBuffer(self, &out);
return out;
}

View File

@@ -206,7 +206,7 @@ pub export fn TextEncoder__encodeRopeString(
if (array == .zero) {
array = jsc.JSValue.createUninitializedUint8Array(globalThis, length) catch return .zero;
array.ensureStillAlive();
@memcpy(array.asArrayBuffer(globalThis).?.ptr[0..length], buf_to_use[0..length]);
@memcpy(array.asArrayBuffer(globalThis).?.byteSlice(), buf_to_use[0..length]);
}
return array;

View File

@@ -813,7 +813,7 @@ pub extern fn napi_create_arraybuffer(env: napi_env, byte_length: usize, data: [
pub extern fn napi_create_external_arraybuffer(env: napi_env, external_data: ?*anyopaque, byte_length: usize, finalize_cb: napi_finalize, finalize_hint: ?*anyopaque, result: *napi_value) napi_status;
pub export fn napi_get_arraybuffer_info(env_: napi_env, arraybuffer_: napi_value, data: ?*[*]u8, byte_length: ?*usize) napi_status {
pub export fn napi_get_arraybuffer_info(env_: napi_env, arraybuffer_: napi_value, data: ?*?[*]u8, byte_length: ?*usize) napi_status {
log("napi_get_arraybuffer_info", .{});
const env = env_ orelse {
return envIsNull();
@@ -825,11 +825,10 @@ pub export fn napi_get_arraybuffer_info(env_: napi_env, arraybuffer_: napi_value
return env.setLastError(.invalid_arg);
}
const slice = array_buffer.slice();
if (data) |dat|
dat.* = slice.ptr;
dat.* = array_buffer.ptr;
if (byte_length) |len|
len.* = slice.len;
len.* = array_buffer.byte_len;
return env.ok();
}
@@ -840,7 +839,7 @@ pub export fn napi_get_typedarray_info(
typedarray_: napi_value,
maybe_type: ?*napi_typedarray_type,
maybe_length: ?*usize,
maybe_data: ?*[*]u8,
maybe_data: ?*?[*]u8,
maybe_arraybuffer: ?*napi_value,
maybe_byte_offset: ?*usize, // note: this is always 0
) napi_status {
@@ -892,7 +891,7 @@ pub export fn napi_get_dataview_info(
env_: napi_env,
dataview_: napi_value,
maybe_bytelength: ?*usize,
maybe_data: ?*[*]u8,
maybe_data: ?*?[*]u8,
maybe_arraybuffer: ?*napi_value,
maybe_byte_offset: ?*usize, // note: this is always 0
) napi_status {
@@ -1223,7 +1222,7 @@ pub export fn napi_create_buffer_copy(env_: napi_env, length: usize, data: [*]u8
return env.ok();
}
extern fn napi_is_buffer(napi_env, napi_value, *bool) napi_status;
pub export fn napi_get_buffer_info(env_: napi_env, value_: napi_value, data: ?*[*]u8, length: ?*usize) napi_status {
pub export fn napi_get_buffer_info(env_: napi_env, value_: napi_value, data: ?*?[*]u8, length: ?*usize) napi_status {
log("napi_get_buffer_info", .{});
const env = env_ orelse {
return envIsNull();

View File

@@ -1336,6 +1336,115 @@ test_napi_create_external_buffer_empty(const Napi::CallbackInfo &info) {
return ok(env);
}
static napi_value test_napi_empty_buffer_info(const Napi::CallbackInfo &info) {
Napi::Env env = info.Env();
// Test: Create an empty external buffer and verify napi_get_buffer_info and
// napi_get_typedarray_info
{
napi_value buffer;
napi_status status =
napi_create_external_buffer(env, 0, nullptr, nullptr, nullptr, &buffer);
if (status != napi_ok) {
printf("FAIL: napi_create_external_buffer with nullptr and zero length "
"failed with status %d\n",
status);
return env.Undefined();
}
// Test napi_get_buffer_info
void *buffer_data = reinterpret_cast<void *>(
0xDEADBEEF); // Initialize to non-null to ensure it's set to null
size_t buffer_length =
999; // Initialize to non-zero to ensure it's set to 0
status = napi_get_buffer_info(env, buffer, &buffer_data, &buffer_length);
if (status != napi_ok) {
printf("FAIL: napi_get_buffer_info failed with status %d\n", status);
return env.Undefined();
}
if (buffer_data != nullptr) {
printf("FAIL: napi_get_buffer_info returned non-null data pointer: %p\n",
buffer_data);
return env.Undefined();
}
if (buffer_length != 0) {
printf("FAIL: napi_get_buffer_info returned non-zero length: %zu\n",
buffer_length);
return env.Undefined();
}
printf("PASS: napi_get_buffer_info returns null pointer and 0 length for "
"empty buffer\n");
// Test napi_get_typedarray_info
napi_typedarray_type type;
size_t typedarray_length = 999; // Initialize to non-zero
void *typedarray_data =
reinterpret_cast<void *>(0xDEADBEEF); // Initialize to non-null
napi_value arraybuffer;
size_t byte_offset;
status =
napi_get_typedarray_info(env, buffer, &type, &typedarray_length,
&typedarray_data, &arraybuffer, &byte_offset);
if (status != napi_ok) {
printf("FAIL: napi_get_typedarray_info failed with status %d\n", status);
return env.Undefined();
}
if (typedarray_data != nullptr) {
printf(
"FAIL: napi_get_typedarray_info returned non-null data pointer: %p\n",
typedarray_data);
return env.Undefined();
}
if (typedarray_length != 0) {
printf("FAIL: napi_get_typedarray_info returned non-zero length: %zu\n",
typedarray_length);
return env.Undefined();
}
printf("PASS: napi_get_typedarray_info returns null pointer and 0 length "
"for empty buffer\n");
// Test napi_is_detached_arraybuffer
// First get the underlying arraybuffer from the buffer
napi_value arraybuffer_from_buffer;
status = napi_get_typedarray_info(env, buffer, nullptr, nullptr, nullptr,
&arraybuffer_from_buffer, nullptr);
if (status != napi_ok) {
printf("FAIL: Could not get arraybuffer from buffer, status %d\n",
status);
return env.Undefined();
}
bool is_detached = false;
status = napi_is_detached_arraybuffer(env, arraybuffer_from_buffer,
&is_detached);
if (status != napi_ok) {
printf("FAIL: napi_is_detached_arraybuffer failed with status %d\n",
status);
return env.Undefined();
}
if (!is_detached) {
printf("FAIL: napi_is_detached_arraybuffer returned false for empty "
"buffer's arraybuffer, expected true\n");
return env.Undefined();
}
printf("PASS: napi_is_detached_arraybuffer returns true for empty buffer's "
"arraybuffer\n");
}
return ok(env);
}
void register_standalone_tests(Napi::Env env, Napi::Object exports) {
REGISTER_FUNCTION(env, exports, test_issue_7685);
REGISTER_FUNCTION(env, exports, test_issue_11949);
@@ -1365,6 +1474,7 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) {
REGISTER_FUNCTION(env, exports, test_napi_typeof_empty_value);
REGISTER_FUNCTION(env, exports, test_napi_freeze_seal_indexed);
REGISTER_FUNCTION(env, exports, test_napi_create_external_buffer_empty);
REGISTER_FUNCTION(env, exports, test_napi_empty_buffer_info);
}
} // namespace napitests

View File

@@ -274,6 +274,14 @@ describe.concurrent("napi", () => {
expect(result).toContain("PASS: napi_create_external_buffer with nullptr finalizer");
expect(result).not.toContain("FAIL");
});
it("empty buffer returns null pointer and 0 length from napi_get_buffer_info and napi_get_typedarray_info", async () => {
const result = await checkSameOutput("test_napi_empty_buffer_info", []);
expect(result).toContain("PASS: napi_get_buffer_info returns null pointer and 0 length for empty buffer");
expect(result).toContain("PASS: napi_get_typedarray_info returns null pointer and 0 length for empty buffer");
expect(result).toContain("PASS: napi_is_detached_arraybuffer returns true for empty buffer's arraybuffer");
expect(result).not.toContain("FAIL");
});
});
describe("napi_async_work", () => {