napi: report the real byteOffset from napi_get_typedarray_info/get_dataview_info

[alii]

napi_get_typedarray_info and napi_get_dataview_info always wrote byte_offset = 0, so a native addon doing the canonical zero-copy reconstruction arraybuffer.Data() + byte_offset read the wrong region for any view created at a non-zero offset (e.g. new Uint8Array(buf, 4) → the addon read byte 0 instead of byte 4). The returned {arraybuffer, byte_offset, data} triple was self-inconsistent: data = base+offset, but byte_offset = 0 and arraybuffer = base, so base + byte_offset != data.

data already has the offset folded in, so report the view's real byteOffset (via JSC's existing JSObjectGetTypedArrayByteOffset) instead of 0 — making base + byte_offset == data, matching Node. Covers both typedarray and dataview, null-guarded; zero-offset views still report 0; length/data/arraybuffer unchanged.

Adds an N-API regression test (offset views at 0/4/8 for Uint8Array + DataView, asserting byte_offset and the base + byte_offset == data reconstruction match Node). Not a port regression — 1.3.14 also returns 0.

[alii]

@robobun adopt

[robobun]

Superseded by work that landed on main while this was in review: real byteOffset for both functions (#33731), fast-mode typed arrays (#37154), and Float16Array mapping (#34144), all with Node-compared tests. No functional delta remains; recommending close (details in my comment below). Only leftover is the minor type==NULL subtype-mismatch check, which I can send as a small separate PR if wanted.

[coderabbitai]

Actionable comments posted: 0

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ea95d29-2485-4a50-9ddc-0647ac3bf1b0

📥 Commits

Reviewing files that changed from the base of the PR and between 20f4a3a and fea749b.

📒 Files selected for processing (5)

• src/jsc/array_buffer.rs
• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/standalone_tests.cpp
• test/napi/napi.test.ts

💤 Files with no reviewable changes (1)

• src/jsc/array_buffer.rs

---

Walkthrough

This PR moves the implementation of napi_get_typedarray_info and napi_get_dataview_info from Rust to C++ to correctly report actual byte_offset values. The Rust side simplifies napi_typedarray_type to a u32 FFI pass-through and declares the functions as extern C bindings. C++ now handles type validation, mapping, and real byte-offset computation. Comprehensive tests validate corrected behavior across multiple offsets, fast-mode arrays, and edge cases.

Changes

N-API typed array byte_offset correction

Layer / File(s)	Summary
Rust FFI update - extern declarations and type refactoring src/runtime/napi/napi_body.rs	Adds JSObjectGetTypedArrayByteOffset extern declaration, simplifies napi_typedarray_type from a Rust enum to u32, removes Rust implementations, and introduces extern "C" bindings for both napi_get_typedarray_info and napi_get_dataview_info to delegate to C++.
C++ implementation - typed array and DataView introspection src/jsc/bindings/napi.cpp	Implements napi_get_typedarray_info and napi_get_dataview_info in C++ with a JSType-to-napi_typedarray_type mapping helper. Both functions validate input type, materialize backing ArrayBuffer, and conditionally populate length, data, arraybuffer, and byte_offset using the actual view offset.
Comprehensive tests for byte_offset behavior test/napi/napi-app/standalone_tests.cpp, test/napi/napi.test.ts	Adds C++ standalone tests validating correct byte_offset for multiple offsets, pointer reconstruction consistency, fast-mode typed arrays, type mismatch rejection, Float16Array edge cases, and data-only pointer stability. Includes TypeScript test harness integration and Bun-specific Float16Array rejection test.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: reporting the real byteOffset from two N-API functions instead of always returning 0.
Description check	✅ Passed	The description covers the problem, solution, and testing, but is missing explicit sections matching the repository's template structure (What does this PR do? / How did you verify your code works?).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Found 1 issue this PR may fix:

1. Unknown Error (napi) #15972 - User passes Buffer.from(...).subarray(4) (byteOffset=4) to node-dpapi native addon; the addon calls napi_get_typedarray_info, gets byte_offset=0, and reads from the wrong memory region, causing a crash

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #15972

🤖 Generated with Claude Code

[claude]

LGTM — straightforward fix replacing the hardcoded byte_offset = 0 with JSC's JSObjectGetTypedArrayByteOffset, mirroring the adjacent JSObjectGetTypedArrayBuffer call pattern, with a Node-comparison regression test.

Extended reasoning...

Overview

This PR fixes napi_get_typedarray_info and napi_get_dataview_info in both the Zig (src/runtime/napi/napi.zig) and Rust (src/runtime/napi/napi_body.rs) implementations. Previously these functions hardcoded byte_offset = 0, breaking the N-API invariant that arraybuffer_base + byte_offset == data for views created at a non-zero offset. The fix swaps the hardcoded 0 for a call to JSC's existing JSObjectGetTypedArrayByteOffset C API. A regression test in test/napi/napi-app/standalone_tests.cpp + test/napi/napi.test.ts exercises offsets 0/4/8 for both Uint8Array and DataView, asserting the reconstructed pointer matches and comparing output against Node via checkSameOutput.

Security risks

None. The change reads a metadata field (byte offset) from an already-validated JS typed-array/DataView object via an existing JSC C API. No new untrusted input parsing, no allocation, no pointer arithmetic beyond what callers already perform with the returned values. If anything, this reduces risk: addons that compute base + byte_offset will now read the correct region instead of the wrong one.

Level of scrutiny

Low-to-medium. The core change is ~4 lines per function across two mirrored implementations, replacing a literal 0 with an FFI call that uses the identical calling convention (env.to_js(), asObjectRef(), null exception ref) as the JSObjectGetTypedArrayBuffer call two lines above it in each function. The Zig extern was already declared in javascript_core_c_api.zig:127; the Rust side adds the matching extern declaration alongside the existing JSObjectGetTypedArrayBuffer declaration. The null-pointer guard in Rust is preserved (and made explicit before the FFI call); Zig's optional unwrap already handles it.

Other factors

• The regression test uses checkSameOutput, which runs the same N-API test under both Node and Bun and asserts identical stdout — so the test itself validates Node-compat, not just internal consistency.
• No CODEOWNERS entry covers src/runtime/napi/.
• No prior reviewer comments to address.
• The bug-hunting system found no issues.
• The PR description correctly notes data/length/arraybuffer outputs are unchanged, so existing addons that read data directly are unaffected; only addons reconstructing via base + offset see the (correct) behavior change.

[Jarred-Sumner]

I suggest moving this one to .cpp. It'll be simpler.

[alii]

@robobun — two things from Jarred's review:

1. Delete the slop comment he flagged in src/jsc/array_buffer.rs ("Delete this slop comment.").
2. He suggested moving the byteOffset retrieval to C++ ("I suggest moving this one to .cpp. It'll be simpler.") — instead of the Rust extern "C" JSObjectGetTypedArrayByteOffset declaration + call in napi_body.rs, do the byteOffset lookup in C++ (a small shim in the napi .cpp, or compute it where the napi function's C++ side already has the view) and call that. Implement his suggested approach — it removes the FFI decl and is simpler.

Keep the behavior and the N-API regression test unchanged: napi_get_typedarray_info / napi_get_dataview_info still report the real byteOffset so arraybuffer.Data() + byteOffset == data, matching Node.

[alii]

@robobun Get this mergable

[robobun]

@alii I went to rebase this (it's 1377 commits behind and conflicts in 4 files) and found it's been superseded on main. Everything this PR fixes has already landed, in the Rust impl, with Node-compared tests:

• Real byteOffset from both napi_get_typedarray_info and napi_get_dataview_info (the bug this PR is for): landed in jsc: delete javascript_core_c_api.rs and remove all JSC C API usage from Rust #33731, which replaced the JSC C API with get_array_buffer_view_byte_offset(). Tested by test_typedarray_info_byte_offset / test_dataview_info_byte_offset at offsets 0/16/24/32/48, asserting data == arraybuffer_data + byte_offset.
• Fast-mode typed arrays (materialize before handing out data): landed in napi: fix dropped writes through data pointers for small typed arrays #37154 (materialize_array_buffer_view_buffer()), tested by test_typedarray_info_write_visibility.
• No JSC C API: jsc: delete javascript_core_c_api.rs and remove all JSC C API usage from Rust #33731 removed it from Rust entirely, so Jarred's original motivation for moving this to C++ no longer applies.
• Float16Array: napi: map Float16Array in napi_get_typedarray_info/napi_create_typedarray #34144 added napi_float16_array and maps it. That also settles the thread with Dylan, and it's a better answer than the napi_invalid_arg this branch currently returns, so rebasing this as-is would regress that.

So the functional delta left in this PR is zero, and merging it would only move working Rust into ~400 lines of C++. I'd close this rather than rebase it. The one tiny thing not on main is rejecting a mismatched view subtype when type == NULL (DataView passed to typedarray_info, or a typed array to dataview_info); if you want that, it's a couple of lines in the Rust and I can open a small separate PR.

Happy to do the rebase instead if you still want the C++ move for its own sake, but I don't think it's worth it.

[alii]

Want to just close this then if it's already fixed?

[robobun]

Yep, closing. Everything here landed via #33731, #37154, and #34144.