Fix panic in JsArrayBufferValue as_ref/as_mut with Rust 1.78 by tronical · Pull Request #2083 · napi-rs/napi-rs

tronical · 2024-05-03T09:19:14Z

In Rust 1.78, when debug assertions are enabled, slice::from_raw_parts panics if the provided data is a null pointer. This is possible through JsArrayBufferValue::new() as well as through JsArrayBuffer::into_value, when napi_get_arraybuffer_info returns a null pointer due to a zero length buffer.

tronical · 2024-05-03T09:20:02Z

I tried adding a test with this patch, but unfortunately I could not figure out how to update the snapshots:

diff --git a/examples/napi/__tests__/values.spec.ts b/examples/napi/__tests__/values.spec.ts
index 47a710a6..adfdd36f 100644
--- a/examples/napi/__tests__/values.spec.ts
+++ b/examples/napi/__tests__/values.spec.ts
@@ -89,6 +89,7 @@ import {
   xxh3,
   xxh64Alias,
   tsRename,
+  acceptArraybuffer,
   acceptSlice,
   u8ArrayToArray,
   i8ArrayToArray,
@@ -740,6 +741,11 @@ test('buffer', (t) => {
   t.true(Array.isArray(asyncBufferToArray(Buffer.from([1, 2, 3]).buffer)))
 })

+test('emptybuffer', (t) => {
+  let buf = new ArrayBuffer(0);
+  t.is(acceptArraybuffer(buf), 0n);
+})
+
 test('TypedArray', (t) => {
   t.is(acceptSlice(new Uint8Array([1, 2, 3])), 3n)
   t.deepEqual(u8ArrayToArray(new Uint8Array([1, 2, 3])), [1, 2, 3])
diff --git a/examples/napi/index.cjs b/examples/napi/index.cjs
index def2f34e..84adc5f4 100644
--- a/examples/napi/index.cjs
+++ b/examples/napi/index.cjs
@@ -400,6 +400,7 @@ module.exports.Optional = nativeBinding.Optional
 module.exports.Selector = nativeBinding.Selector
 module.exports.UseNullableClass = nativeBinding.UseNullableClass
 module.exports.Width = nativeBinding.Width
+module.exports.acceptArraybuffer = nativeBinding.acceptArraybuffer
 module.exports.acceptSlice = nativeBinding.acceptSlice
 module.exports.acceptThreadsafeFunction = nativeBinding.acceptThreadsafeFunction
 module.exports.acceptThreadsafeFunctionFatal = nativeBinding.acceptThreadsafeFunctionFatal
diff --git a/examples/napi/index.d.cts b/examples/napi/index.d.cts
index b62656eb..5f19bc83 100644
--- a/examples/napi/index.d.cts
+++ b/examples/napi/index.d.cts
@@ -238,6 +238,8 @@ export interface A {
   foo: number
 }

+export function acceptArraybuffer(fixture: ArrayBuffer): bigint
+
 export function acceptSlice(fixture: Uint8Array): bigint

 export function acceptThreadsafeFunction(func: (err: Error | null, arg: number) => any): void
diff --git a/examples/napi/src/typed_array.rs b/examples/napi/src/typed_array.rs
index 620e250b..899af63d 100644
--- a/examples/napi/src/typed_array.rs
+++ b/examples/napi/src/typed_array.rs
@@ -49,6 +49,11 @@ async fn array_buffer_pass_through(buf: Uint8Array) -> Result<Uint8Array> {
   Ok(buf)
 }

+#[napi]
+fn accept_arraybuffer(fixture: JsArrayBuffer) -> Result<usize> {
+  Ok(fixture.into_value()?.as_ref().len())
+}
+
 #[napi]
 fn accept_slice(fixture: &[u8]) -> usize {
   fixture.len()

But this reproduces the issue.

tronical · 2024-05-03T09:20:50Z

Given that this affects all existing napi-rs users it would be awesome if there's a chance of back-porting this into 2.16.x.

After the Rust 1.78 release, napi-rs panics on zero-sized array buffers. Work around it by disabling the debug assertions in the core library that trigger this. Upstream PR: napi-rs/napi-rs#2083

After the Rust 1.78 release, napi-rs panics on zero-sized array buffers. Work around it by not allocating a zero-sized array buffer for this specific test. Upstream PR: napi-rs/napi-rs#2083

We can revert this change when napi-rs/napi-rs#2083 is merged and a new `napi` version is released.

* Pin rust to v1.77 We can revert this change when napi-rs/napi-rs#2083 is merged and a new `napi` version is released. * Update rust to use 1.77 rather than 1.77.0

…2083) In Rust 1.78, when debug assertions are enabled, slice::from_raw_parts panics if the provided data is a null pointer. This is possible through JsArrayBufferValue::new() as well as through JsArrayBuffer::into_value, when napi_get_arraybuffer_info returns a null pointer due to a zero length buffer.

* Add test for #2083 * Add test for serde_byte with empty buffer * Fix using serde_bytes with empty buffers * skip wasi test * fix test * clippy fix * avoid ub * use safer impl --------- Co-authored-by: Simon Hausmann <simon.hausmann@slint.dev> Co-authored-by: LongYinan <lynweklm@gmail.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | | | lockFileMaintenance | All locks refreshed | | [napi](https://togithub.com/napi-rs/napi-rs) | workspace.dependencies | patch | `2.16.4` -> `2.16.6` | | [napi-derive](https://togithub.com/napi-rs/napi-rs) | workspace.dependencies | patch | `2.16.3` -> `2.16.4` | | [num-bigint](https://togithub.com/rust-num/num-bigint) | workspace.dependencies | patch | `0.4.4` -> `0.4.5` | | [petgraph](https://togithub.com/petgraph/petgraph) | workspace.dependencies | patch | `0.6.4` -> `0.6.5` | | [proc-macro2](https://togithub.com/dtolnay/proc-macro2) | workspace.dependencies | patch | `1.0.81` -> `1.0.82` | | [serde](https://serde.rs) ([source](https://togithub.com/serde-rs/serde)) | workspace.dependencies | patch | `1.0.200` -> `1.0.201` | | [serde_json](https://togithub.com/serde-rs/json) | workspace.dependencies | patch | `1.0.116` -> `1.0.117` | | [trybuild](https://togithub.com/dtolnay/trybuild) | workspace.dependencies | patch | `1.0.93` -> `1.0.95` | 🔧 This Pull Request updates lock files to use the latest dependency versions. --- ### Release Notes <details> <summary>napi-rs/napi-rs (napi)</summary> ### [`v2.16.6`](https://togithub.com/napi-rs/napi-rs/releases/tag/napi%402.16.6) [Compare Source](https://togithub.com/napi-rs/napi-rs/compare/napi@2.16.5...napi@2.16.6) #### What's Changed - fix(napi): panic when deserializing empty buffer with Rust 1.78 by [@mischnic](https://togithub.com/mischnic) in [https://github.com/napi-rs/napi-rs/pull/2094](https://togithub.com/napi-rs/napi-rs/pull/2094) **Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.16.5...napi@2.16.6 ### [`v2.16.5`](https://togithub.com/napi-rs/napi-rs/releases/tag/napi%402.16.5) [Compare Source](https://togithub.com/napi-rs/napi-rs/compare/napi@2.16.4...napi@2.16.5) ##### What's Changed - Fix panic in JsArrayBufferValue as_ref/as_mut with Rust 1.78 by [@tronical](https://togithub.com/tronical) in [https://github.com/napi-rs/napi-rs/pull/2083](https://togithub.com/napi-rs/napi-rs/pull/2083) - fix(napi): make sure env without exception pending before throw error by [@southorange0929](https://togithub.com/southorange0929) in [https://github.com/napi-rs/napi-rs/pull/2092](https://togithub.com/napi-rs/napi-rs/pull/2092) **Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.16.4...napi@2.16.5 </details> <details> <summary>rust-num/num-bigint (num-bigint)</summary> ### [`v0.4.5`](https://togithub.com/rust-num/num-bigint/blob/HEAD/RELEASES.md#Release-045-2024-05-06) [Compare Source](https://togithub.com/rust-num/num-bigint/compare/num-bigint-0.4.4...num-bigint-0.4.5) - [Upgrade to 2021 edition, **MSRV 1.60**][292] - [Add `const ZERO` and implement `num_traits::ConstZero`][298] - [Add `modinv` methods for the modular inverse][288] - [Optimize multiplication with imbalanced operands][295] - [Optimize scalar division on x86 and x86-64][236] **Contributors**: [@cuviper](https://togithub.com/cuviper), [@joelonsql](https://togithub.com/joelonsql), [@waywardmonkeys](https://togithub.com/waywardmonkeys) [236]: https://togithub.com/rust-num/num-bigint/pull/236 [288]: https://togithub.com/rust-num/num-bigint/pull/288 [292]: https://togithub.com/rust-num/num-bigint/pull/292 [295]: https://togithub.com/rust-num/num-bigint/pull/295 [298]: https://togithub.com/rust-num/num-bigint/pull/298 </details> <details> <summary>petgraph/petgraph (petgraph)</summary> ### [`v0.6.5`](https://togithub.com/petgraph/petgraph/blob/HEAD/RELEASES.rst#Version-065-2024-05-06) [Compare Source](https://togithub.com/petgraph/petgraph/compare/petgraph@v0.6.4...petgraph@v0.6.5) \========================== - Add rayon support for `GraphMap` (`#573`*, `#615`*) - Add `Topo::with_initials` method (`#585`\_) - Add logo to the project (`#598`\_) - Add Ford-Fulkerson algorithm (`#640`\_) - Update `itertools` to 0.12.1 (`#628`\_) - Update `GraphMap` to allow custom hash functions (`#623`\_) - Fix documentation (`#630`\_) - Fix clippy warnings (`#627`\_) - (internal) Fix remove old `copyclone` macro (`#601`\_) - (internal) Move minimum spanning tree into own module (`#624`\_) .. \_`#573`: [https://github.com/petgraph/petgraph/pull/573](https://togithub.com/petgraph/petgraph/pull/573) .. \_`#615`: [https://github.com/petgraph/petgraph/pull/615](https://togithub.com/petgraph/petgraph/pull/615) .. \_`#585`: [https://github.com/petgraph/petgraph/pull/585](https://togithub.com/petgraph/petgraph/pull/585) .. \_`#598`: [https://github.com/petgraph/petgraph/pull/598](https://togithub.com/petgraph/petgraph/pull/598) .. \_`#640`: [https://github.com/petgraph/petgraph/pull/640](https://togithub.com/petgraph/petgraph/pull/640) .. \_`#628`: [https://github.com/petgraph/petgraph/pull/628](https://togithub.com/petgraph/petgraph/pull/628) .. \_`#623`: [https://github.com/petgraph/petgraph/pull/623](https://togithub.com/petgraph/petgraph/pull/623) .. \_`#630`: [https://github.com/petgraph/petgraph/pull/630](https://togithub.com/petgraph/petgraph/pull/630) .. \_`#627`: [https://github.com/petgraph/petgraph/pull/627](https://togithub.com/petgraph/petgraph/pull/627) .. \_`#601`: [https://github.com/petgraph/petgraph/pull/601](https://togithub.com/petgraph/petgraph/pull/601) .. \_`#624`: [https://github.com/petgraph/petgraph/pull/624](https://togithub.com/petgraph/petgraph/pull/624) </details> <details> <summary>dtolnay/proc-macro2 (proc-macro2)</summary> ### [`v1.0.82`](https://togithub.com/dtolnay/proc-macro2/releases/tag/1.0.82) [Compare Source](https://togithub.com/dtolnay/proc-macro2/compare/1.0.81...1.0.82) - Resolve unexpected_cfgs warning ([#456](https://togithub.com/dtolnay/proc-macro2/issues/456)) </details> <details> <summary>serde-rs/serde (serde)</summary> ### [`v1.0.201`](https://togithub.com/serde-rs/serde/releases/tag/v1.0.201) [Compare Source](https://togithub.com/serde-rs/serde/compare/v1.0.200...v1.0.201) - Resolve unexpected_cfgs warning ([#2737](https://togithub.com/serde-rs/serde/issues/2737)) </details> <details> <summary>serde-rs/json (serde_json)</summary> ### [`v1.0.117`](https://togithub.com/serde-rs/json/releases/tag/v1.0.117) [Compare Source](https://togithub.com/serde-rs/json/compare/v1.0.116...v1.0.117) - Resolve unexpected_cfgs warning ([#1130](https://togithub.com/serde-rs/json/issues/1130)) </details> <details> <summary>dtolnay/trybuild (trybuild)</summary> ### [`v1.0.95`](https://togithub.com/dtolnay/trybuild/releases/tag/1.0.95) [Compare Source](https://togithub.com/dtolnay/trybuild/compare/1.0.94...1.0.95) - Keep long type names in diagnostics so that test output does not vary depending on the length of the absolute filepath of the crate ([#269](https://togithub.com/dtolnay/trybuild/issues/269)) ### [`v1.0.94`](https://togithub.com/dtolnay/trybuild/releases/tag/1.0.94) [Compare Source](https://togithub.com/dtolnay/trybuild/compare/1.0.93...1.0.94) - Resolve unexpected_cfgs warning ([#268](https://togithub.com/dtolnay/trybuild/issues/268)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on monday" in timezone Asia/Shanghai, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/oxc-project/oxc).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

tronical mentioned this pull request May 3, 2024

Fix Node.js CI slint-ui/slint#5169

Merged

mischnic mentioned this pull request May 5, 2024

panic with empty buffer as argument: NonNull::new_unchecked requires that the pointer is non-null #2085

Closed

yamadapc added a commit to parcel-bundler/parcel that referenced this pull request May 5, 2024

Pin rust to v1.77

db87015

We can revert this change when napi-rs/napi-rs#2083 is merged and a new `napi` version is released.

yamadapc mentioned this pull request May 5, 2024

Pin rust to v1.77 parcel-bundler/parcel#9696

Merged

Brooooooklyn merged commit 67ee2d0 into napi-rs:main May 6, 2024

mischnic mentioned this pull request May 6, 2024

Bump napi-rs parcel-bundler/parcel#9699

Closed

2 tasks

mischnic added a commit to mischnic/napi-rs that referenced this pull request May 6, 2024

Add test for napi-rs#2083

d138481

mischnic pushed a commit to mischnic/napi-rs that referenced this pull request May 6, 2024

Add test for napi-rs#2083

aa17381

mischnic mentioned this pull request May 6, 2024

fix(napi): panic when deserializing empty buffer with Rust 1.78 #2094

Merged

Xanewok mentioned this pull request Sep 17, 2024

fix(napi): nullptr handling in TypedArrays #2258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix panic in JsArrayBufferValue as_ref/as_mut with Rust 1.78#2083

Fix panic in JsArrayBufferValue as_ref/as_mut with Rust 1.78#2083
Brooooooklyn merged 1 commit intonapi-rs:mainfrom
slint-ui:simon/nullptr-fix

tronical commented May 3, 2024

Uh oh!

tronical commented May 3, 2024

Uh oh!

tronical commented May 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tronical commented May 3, 2024

Uh oh!

tronical commented May 3, 2024

Uh oh!

tronical commented May 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants