From 1cf32631bf2c118fd261fe217feedebbaf988b39 Mon Sep 17 00:00:00 2001
From: nihohit <nihohit@gmail.com>
Date: Wed, 30 Nov 2022 14:54:13 +0200
Subject: [PATCH] fix(napi): typed arrays ref shouldn't use offset. (#1376)

Notice from the n-api docs that the data returned from
`napi_get_typedarray_info` is already adjusted by the byte offset.
https://nodejs.org/api/n-api.html#napi_get_typedarray_info

This means that when `as_ref`/`as_mut` apply the byte offset, the
offset is in practice applied twice.
This wasn't caught in tests because no test tried to modify a typed
array with a byte offset, and the test didn't us the typed array
structs, only `JsTypedArray`. If you want, I can modify the rest of the
functions in examples/napi-compt-mode/src/arraybuffers.rs
and the matching tests, to test all typed arrays.

IMO the `byte_offset` field can be removed entirely from the struct,
but I wanted to submit a minimal PR.
---
 crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs | 4 ----
 examples/napi-compat-mode/__test__/arraybuffer.spec.ts   | 7 +++++++
 examples/napi-compat-mode/src/arraybuffer.rs             | 7 +++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
index 3a07ef24..c7363b4c 100644
--- a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
+++ b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
@@ -207,16 +207,12 @@ macro_rules! impl_typed_array {
     impl AsRef<[$rust_type]> for $name {
       fn as_ref(&self) -> &[$rust_type] {
         unsafe { std::slice::from_raw_parts(self.data, self.length) }
-          .split_at(self.byte_offset)
-          .1
       }
     }
 
     impl AsMut<[$rust_type]> for $name {
       fn as_mut(&mut self) -> &mut [$rust_type] {
         unsafe { std::slice::from_raw_parts_mut(self.data, self.length) }
-          .split_at_mut(self.byte_offset)
-          .1
       }
     }
 
diff --git a/examples/napi-compat-mode/__test__/arraybuffer.spec.ts b/examples/napi-compat-mode/__test__/arraybuffer.spec.ts
index 911d2037..f9608451 100644
--- a/examples/napi-compat-mode/__test__/arraybuffer.spec.ts
+++ b/examples/napi-compat-mode/__test__/arraybuffer.spec.ts
@@ -17,6 +17,13 @@ test('should be able to mutate Uint8Array', (t) => {
   t.is(fixture[0], 42)
 })
 
+test('should be able to mutate Uint8Array in its middle', (t) => {
+  const fixture = new Uint8Array([0, 1, 2])
+  const view = new Uint8Array(fixture.buffer, 1, 1)
+  bindings.mutateUint8Array(view)
+  t.is(fixture[1], 42)
+})
+
 test('should be able to mutate Uint16Array', (t) => {
   const fixture = new Uint16Array([0, 1, 2])
   bindings.mutateUint16Array(fixture)
diff --git a/examples/napi-compat-mode/src/arraybuffer.rs b/examples/napi-compat-mode/src/arraybuffer.rs
index 2227d467..ffc57624 100644
--- a/examples/napi-compat-mode/src/arraybuffer.rs
+++ b/examples/napi-compat-mode/src/arraybuffer.rs
@@ -1,7 +1,10 @@
 use std::f64::consts::PI;
 use std::str;
 
-use napi::{CallContext, JsArrayBuffer, JsNumber, JsObject, JsTypedArray, JsUndefined, Result};
+use napi::{
+  bindgen_prelude::Uint8Array, CallContext, JsArrayBuffer, JsNumber, JsObject, JsTypedArray,
+  JsUndefined, Result,
+};
 
 #[js_function(1)]
 pub fn get_arraybuffer_length(ctx: CallContext) -> Result<JsNumber> {
@@ -11,7 +14,7 @@ pub fn get_arraybuffer_length(ctx: CallContext) -> Result<JsNumber> {
 
 #[js_function(1)]
 pub fn mutate_uint8_array(ctx: CallContext) -> Result<JsUndefined> {
-  let mut buffer = ctx.get::<JsTypedArray>(0)?.into_value()?;
+  let mut buffer = ctx.get::<Uint8Array>(0)?;
   let buffer_mut_ref: &mut [u8] = buffer.as_mut();
   buffer_mut_ref[0] = 42;
   ctx.env.get_undefined()