Skip to content

napi: report the real byteOffset from napi_get_typedarray_info/get_dataview_info #31524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alii wants to merge 1 commit into from
Closed

napi: report the real byteOffset from napi_get_typedarray_info/get_dataview_info #31524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/jsc/array_buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,12 +884,6 @@ impl TypedArrayType {
TypedArrayType::TypeDataView => C::kJSTypedArrayTypeNone,
}
}

// LAYERING: Zig's `toNapi` (array_buffer.zig:524) maps to
// `napi_typedarray_type`, which is defined in `bun_runtime` (a higher-tier
// crate that depends on `bun_jsc`). The conversion lives next to its target
// type as `napi_typedarray_type::from_typed_array_type` in
// `bun_runtime::napi` to avoid the dep cycle.
}

// ──────────────────────────────────────────────────────────────────────────
Expand Down
139 changes: 139 additions & 0 deletions src/jsc/bindings/napi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1786,6 +1786,145 @@ extern "C" napi_status napi_create_typedarray(
NAPI_RETURN_SUCCESS(env);
}

// Inverse of getTypedArrayTypeFromNAPI: maps a JSC typed-array cell type to the
// napi enum. Returns false when there is no corresponding `napi_typedarray_type`
// — i.e. for `Float16Array` (a typed array, but N-API has no value for it), for
// DataView, and for any non-typed-array type.
static bool getNAPITypeFromJSType(JSC::JSType type, napi_typedarray_type* result)
{
switch (type) {
case JSC::JSType::Int8ArrayType:
*result = napi_int8_array;
return true;
case JSC::JSType::Uint8ArrayType:
*result = napi_uint8_array;
return true;
case JSC::JSType::Uint8ClampedArrayType:
*result = napi_uint8_clamped_array;
return true;
case JSC::JSType::Int16ArrayType:
*result = napi_int16_array;
return true;
case JSC::JSType::Uint16ArrayType:
*result = napi_uint16_array;
return true;
case JSC::JSType::Int32ArrayType:
*result = napi_int32_array;
return true;
case JSC::JSType::Uint32ArrayType:
*result = napi_uint32_array;
return true;
case JSC::JSType::Float32ArrayType:
Comment thread
robobun marked this conversation as resolved.
*result = napi_float32_array;
return true;
case JSC::JSType::Float64ArrayType:
*result = napi_float64_array;
return true;
case JSC::JSType::BigInt64ArrayType:
*result = napi_bigint64_array;
return true;
case JSC::JSType::BigUint64ArrayType:
*result = napi_biguint64_array;
return true;
default:
return false;
}
}

extern "C" napi_status napi_get_typedarray_info(
napi_env env,
napi_value typedarray,
napi_typedarray_type* type,
size_t* length,
void** data,
napi_value* arraybuffer,
size_t* byte_offset)
{
NAPI_PREAMBLE(env);
NAPI_CHECK_ENV_NOT_IN_GC(env);
NAPI_CHECK_ARG(env, typedarray);
Zig::GlobalObject* globalObject = toJS(env);

JSC::JSArrayBufferView* view = dynamicDowncast<JSC::JSArrayBufferView>(toJS(typedarray));
NAPI_RETURN_EARLY_IF_FALSE(env, view, napi_invalid_arg);

// Reject a DataView unconditionally (Node gates on `value->IsTypedArray()` before reading
// any field, so this must run even when `type == nullptr`). `isTypedArrayType` is true for
// every typed-array kind and false for `DataViewType`.
NAPI_RETURN_EARLY_IF_FALSE(env, JSC::isTypedArrayType(view->type()), napi_invalid_arg);

// Materialize the backing ArrayBuffer *before* reading `vector()` whenever `data` or
// `arraybuffer` is requested (Node does the same). For a `FastTypedArray` (GC-managed
// storage, no ArrayBuffer yet) `possiblySharedBuffer()` copies the bytes into a
// malloc-backed buffer and repoints `m_vector`; reading `vector()` first would leave
// `data` pointing at storage that a later `.buffer` access orphans (dangling), and
// breaks `arraybuffer + byte_offset == data`.
JSC::JSArrayBuffer* jsBuffer = (arraybuffer || data) ? view->possiblySharedJSBuffer(globalObject) : nullptr;

if (type) {
// `Float16Array` is a typed array but has no `napi_typedarray_type` value (the enum
// stops at `napi_biguint64_array`). If the caller asked for the type we can't report
// one, so fail with `napi_invalid_arg` instead of returning success with `*type`
// uninitialized. Callers that don't request the type (`type == nullptr`) still succeed.
NAPI_RETURN_EARLY_IF_FALSE(env, getNAPITypeFromJSType(view->type(), type), napi_invalid_arg);
}
Comment thread
robobun marked this conversation as resolved.
if (length) {
*length = view->length();
}
if (data) {
*data = view->vector();
}
if (arraybuffer) {
*arraybuffer = toNapi(jsBuffer, globalObject);
}
Comment thread
robobun marked this conversation as resolved.
if (byte_offset) {
// `data` is `view->vector()`, which already has the offset folded in. The N-API
// contract is that `arraybuffer` (the base) plus `byte_offset` reconstructs `data`,
// so report the view's real byteOffset here — not 0.
*byte_offset = view->byteOffset();
}
NAPI_RETURN_SUCCESS(env);
}

extern "C" napi_status napi_get_dataview_info(
napi_env env,
napi_value dataview,
size_t* bytelength,
void** data,
napi_value* arraybuffer,
size_t* byte_offset)
{
NAPI_PREAMBLE(env);
NAPI_CHECK_ENV_NOT_IN_GC(env);
NAPI_CHECK_ARG(env, dataview);
Zig::GlobalObject* globalObject = toJS(env);

JSC::JSArrayBufferView* view = dynamicDowncast<JSC::JSArrayBufferView>(toJS(dataview));
NAPI_RETURN_EARLY_IF_FALSE(env, view, napi_object_expected);
// Node checks `IsDataView()` — reject a typed array passed here.
NAPI_RETURN_EARLY_IF_FALSE(env, view->type() == JSC::DataViewType, napi_invalid_arg);

// A DataView is always ArrayBuffer-backed (never a FastTypedArray), so the ordering
// below doesn't matter for correctness — but keep it symmetric with
// napi_get_typedarray_info: materialize before reading `vector()`.
JSC::JSArrayBuffer* jsBuffer = (arraybuffer || data) ? view->possiblySharedJSBuffer(globalObject) : nullptr;

if (bytelength) {
*bytelength = view->byteLength();
}
if (data) {
*data = view->vector();
}
if (arraybuffer) {
*arraybuffer = toNapi(jsBuffer, globalObject);
}
if (byte_offset) {
// See napi_get_typedarray_info: report the view's real byteOffset, not 0.
*byte_offset = view->byteOffset();
}
NAPI_RETURN_SUCCESS(env);
}

namespace Zig {

extern "C" napi_status napi_get_all_property_names(
Expand Down
165 changes: 26 additions & 139 deletions src/runtime/napi/napi_body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ unsafe extern "C" {
ctx: *mut JSGlobalObject,
object: jsc::c_api::JSObjectRef,
) -> jsc::c_api::JSValueRef;
fn JSObjectGetTypedArrayBuffer(
ctx: *mut JSGlobalObject,
object: jsc::c_api::JSObjectRef,
exception: jsc::c_api::ExceptionRef,
) -> jsc::c_api::JSObjectRef;
fn JSObjectMakeDate(
ctx: *mut JSGlobalObject,
argument_count: usize,
Expand Down Expand Up @@ -346,42 +341,11 @@ pub(super) type napi_property_attributes = c_uint;
// constructs or matches variants.
pub(super) type napi_valuetype = u32;

#[repr(u32)]
#[derive(Copy, Clone, PartialEq, Eq)]
pub(super) enum napi_typedarray_type {
int8_array = 0,
uint8_array = 1,
uint8_clamped_array = 2,
int16_array = 3,
uint16_array = 4,
int32_array = 5,
uint32_array = 6,
float32_array = 7,
float64_array = 8,
bigint64_array = 9,
biguint64_array = 10,
}

impl napi_typedarray_type {
pub(super) fn from_js_type(this: jsc::JSType) -> Option<napi_typedarray_type> {
// PORT NOTE: jsc::JSType is a newtype struct with associated consts (not an enum),
// so glob-import is unavailable; match on the qualified const paths instead.
Some(match this {
jsc::JSType::Int8Array => napi_typedarray_type::int8_array,
jsc::JSType::Uint8Array => napi_typedarray_type::uint8_array,
jsc::JSType::Uint8ClampedArray => napi_typedarray_type::uint8_clamped_array,
jsc::JSType::Int16Array => napi_typedarray_type::int16_array,
jsc::JSType::Uint16Array => napi_typedarray_type::uint16_array,
jsc::JSType::Int32Array => napi_typedarray_type::int32_array,
jsc::JSType::Uint32Array => napi_typedarray_type::uint32_array,
jsc::JSType::Float32Array => napi_typedarray_type::float32_array,
jsc::JSType::Float64Array => napi_typedarray_type::float64_array,
jsc::JSType::BigInt64Array => napi_typedarray_type::bigint64_array,
jsc::JSType::BigUint64Array => napi_typedarray_type::biguint64_array,
_ => return None,
})
}
}
// Only passed through FFI — by value into `napi_create_typedarray` and as a
// `*mut napi_typedarray_type` out-param written by C++ `napi_get_typedarray_info`.
// Rust never constructs or matches variants, so (like `napi_valuetype`) it is a
// plain `u32` rather than an enum. The real enum lives in `node_api.h`.
pub(super) type napi_typedarray_type = u32;
Comment thread
robobun marked this conversation as resolved.

#[repr(u32)]
#[derive(Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -1422,65 +1386,17 @@ unsafe extern "C" {
value: napi_value,
result: *mut bool,
) -> napi_status;
}

#[unsafe(no_mangle)]
pub(super) extern "C" fn napi_get_typedarray_info(
env_: napi_env,
typedarray_: napi_value,
maybe_type: *mut napi_typedarray_type,
maybe_length: *mut usize,
maybe_data: *mut *mut u8,
maybe_arraybuffer: *mut napi_value,
maybe_byte_offset: *mut usize, // note: this is always 0
) -> napi_status {
bun_output::scoped_log!(napi, "napi_get_typedarray_info");
let env = get_env!(env_);
env.check_gc();
let typedarray = typedarray_.get();
if typedarray.is_empty_or_undefined_or_null() {
return env.invalid_arg();
}
let _keep = jsc::EnsureStillAlive(typedarray);

let Some(array_buffer) = typedarray.as_array_buffer(env.to_js()) else {
return env.invalid_arg();
};
// SAFETY: `maybe_type` is null or a valid exclusive out-param per N-API contract.
if let Some(ty) = unsafe { maybe_type.as_mut() } {
// Zig: `array_buffer.typed_array_type.toTypedArrayType().toNapi()`. The Rust
// `ArrayBuffer.typed_array_type` field is already a `JSType`, so map it
// straight to `napi_typedarray_type`.
let Some(napi_ty) = napi_typedarray_type::from_js_type(array_buffer.typed_array_type)
else {
return env.invalid_arg();
};
*ty = napi_ty;
}

// TODO: handle detached
write_out(maybe_data, array_buffer.ptr);
write_out(maybe_length, array_buffer.len);

// SAFETY: `maybe_arraybuffer` is null or a valid exclusive out-param per N-API contract.
if let Some(arraybuffer) = unsafe { maybe_arraybuffer.as_mut() } {
arraybuffer.set(
env,
// SAFETY: `typedarray` is a live typed-array object (kept by `_keep`); FFI reads its backing buffer.
JSValue::c(unsafe {
JSObjectGetTypedArrayBuffer(
env.to_js().as_ptr(),
typedarray.as_object_ref(),
ptr::null_mut(),
)
}),
);
}

// `jsc::ArrayBuffer` used to have an `offset` field, but it was always 0 because `ptr`
// already had the offset applied. See <https://github.com/oven-sh/bun/issues/561>.
write_out(maybe_byte_offset, 0);
env.ok()
// Implemented in C++ (`src/jsc/bindings/napi.cpp`) so it can read the view's
// real `byteOffset` directly via `JSArrayBufferView::byteOffset()`.
pub(super) fn napi_get_typedarray_info(
env: napi_env,
typedarray: napi_value,
type_: *mut napi_typedarray_type,
length: *mut usize,
data: *mut *mut u8,
arraybuffer: *mut napi_value,
byte_offset: *mut usize,
) -> napi_status;
}

unsafe extern "C" {
Expand All @@ -1491,6 +1407,16 @@ unsafe extern "C" {
byte_offset: usize,
result: *mut napi_value,
) -> napi_status;
// Implemented in C++ (`src/jsc/bindings/napi.cpp`), alongside
// `napi_get_typedarray_info`.
pub(super) fn napi_get_dataview_info(
env: napi_env,
dataview: napi_value,
bytelength: *mut usize,
data: *mut *mut u8,
arraybuffer: *mut napi_value,
byte_offset: *mut usize,
) -> napi_status;
}

#[unsafe(no_mangle)]
Expand All @@ -1508,45 +1434,6 @@ pub(super) extern "C" fn napi_is_dataview(
env.ok()
}

#[unsafe(no_mangle)]
pub(super) extern "C" fn napi_get_dataview_info(
env_: napi_env,
dataview_: napi_value,
maybe_bytelength: *mut usize,
maybe_data: *mut *mut u8,
maybe_arraybuffer: *mut napi_value,
maybe_byte_offset: *mut usize, // note: this is always 0
) -> napi_status {
bun_output::scoped_log!(napi, "napi_get_dataview_info");
let env = get_env!(env_);
env.check_gc();
let dataview = dataview_.get();
let Some(array_buffer) = dataview.as_array_buffer(env.to_js()) else {
return NapiEnv::set_last_error(Some(env), NapiStatus::object_expected);
};
write_out(maybe_bytelength, array_buffer.byte_len);
write_out(maybe_data, array_buffer.ptr);
// SAFETY: `maybe_arraybuffer` is null or a valid exclusive out-param per N-API contract.
if let Some(arraybuffer) = unsafe { maybe_arraybuffer.as_mut() } {
arraybuffer.set(
env,
// SAFETY: `dataview` is a live DataView object (held in handle scope); FFI reads its backing buffer.
JSValue::c(unsafe {
JSObjectGetTypedArrayBuffer(
env.to_js().as_ptr(),
dataview.as_object_ref(),
ptr::null_mut(),
)
}),
);
}
// `jsc::ArrayBuffer` used to have an `offset` field, but it was always 0 because `ptr`
// already had the offset applied. See <https://github.com/oven-sh/bun/issues/561>.
write_out(maybe_byte_offset, 0);

env.ok()
}

#[unsafe(no_mangle)]
pub(super) extern "C" fn napi_get_version(env_: napi_env, result_: *mut u32) -> napi_status {
bun_output::scoped_log!(napi, "napi_get_version");
Expand Down
Loading
Loading