Skip to content

Commit

Permalink
fix(napi): panic when deserializing empty buffer with Rust 1.78 (#2094)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people committed May 7, 2024
1 parent b883507 commit f3d665d
Show file tree
Hide file tree
Showing 12 changed files with 515 additions and 450 deletions.
17 changes: 0 additions & 17 deletions crates/napi/src/js_values/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,23 +58,6 @@ impl JsBuffer {
}

impl JsBufferValue {
#[cfg(feature = "serde-json")]
pub(crate) fn from_raw(env: sys::napi_env, value: sys::napi_value) -> Result<Self> {
let mut data = ptr::null_mut();
let mut len = 0usize;
check_status!(unsafe {
sys::napi_get_buffer_info(env, value, &mut data, &mut len as *mut usize as *mut _)
})?;
Ok(Self {
value: JsBuffer(Value {
env,
value,
value_type: ValueType::Object,
}),
data: mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(data as *mut _, len, len) }),
})
}

pub fn new(value: JsBuffer, data: mem::ManuallyDrop<Vec<u8>>) -> Self {
JsBufferValue { value, data }
}
Expand Down
13 changes: 6 additions & 7 deletions crates/napi/src/js_values/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ use std::convert::TryInto;
use serde::de::Visitor;
use serde::de::{DeserializeSeed, EnumAccess, MapAccess, SeqAccess, Unexpected, VariantAccess};

use crate::bindgen_runtime::FromNapiValue;
#[cfg(feature = "napi6")]
use crate::JsBigInt;
use crate::{type_of, NapiValue, Value, ValueType};
use crate::{
Error, JsBoolean, JsBufferValue, JsNumber, JsObject, JsString, JsUnknown, Result, Status,
};
use crate::{Error, JsBoolean, JsNumber, JsObject, JsString, JsUnknown, Result, Status};

pub(crate) struct De<'env>(pub(crate) &'env Value);

Expand Down Expand Up @@ -46,8 +45,8 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> {
let mut deserializer =
JsArrayAccess::new(&js_object, js_object.get_array_length_unchecked()?);
visitor.visit_seq(&mut deserializer)
} else if js_object.is_buffer()? {
visitor.visit_bytes(&JsBufferValue::from_raw(self.0.env, self.0.value)?)
} else if js_object.is_typedarray()? {
visitor.visit_bytes(unsafe { FromNapiValue::from_napi_value(self.0.env, self.0.value)? })
} else {
let mut deserializer = JsObjectAccess::new(&js_object)?;
visitor.visit_map(&mut deserializer)
Expand Down Expand Up @@ -79,14 +78,14 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> {
where
V: Visitor<'x>,
{
visitor.visit_bytes(&JsBufferValue::from_raw(self.0.env, self.0.value)?)
visitor.visit_bytes(unsafe { FromNapiValue::from_napi_value(self.0.env, self.0.value)? })
}

fn deserialize_byte_buf<V>(self, visitor: V) -> Result<V::Value>
where
V: Visitor<'x>,
{
visitor.visit_bytes(&JsBufferValue::from_raw(self.0.env, self.0.value)?)
visitor.visit_bytes(unsafe { FromNapiValue::from_napi_value(self.0.env, self.0.value)? })
}

fn deserialize_option<V>(self, visitor: V) -> Result<V::Value>
Expand Down
1 change: 1 addition & 0 deletions examples/napi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ futures = "0.3"
napi-derive = { path = "../../crates/macro", features = ["type-def"] }
napi-shared = { path = "../napi-shared" }
serde = "1"
serde_bytes = "0.11"
serde_derive = "1"
serde_json = "1"
indexmap = "2"
Expand Down
4 changes: 4 additions & 0 deletions examples/napi/__tests__/__snapshots__/typegen.spec.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ Generated by [AVA](https://avajs.dev).
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␊
Expand Down Expand Up @@ -667,6 +669,8 @@ Generated by [AVA](https://avajs.dev).
export function testSerdeBigNumberPrecision(number: string): any␊
export function testSerdeBufferBytes(obj: object): bigint␊
export function testSerdeRoundtrip(data: any): any␊
export function threadsafeFunctionClosureCapture(func: (arg: string) => void): void␊
Expand Down
Binary file modified examples/napi/__tests__/__snapshots__/typegen.spec.ts.snap
Binary file not shown.
15 changes: 15 additions & 0 deletions examples/napi/__tests__/values.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ import {
xxh3,
xxh64Alias,
tsRename,
acceptArraybuffer,
acceptSlice,
u8ArrayToArray,
i8ArrayToArray,
Expand Down Expand Up @@ -123,6 +124,7 @@ import {
returnJsFunction,
testSerdeRoundtrip,
testSerdeBigNumberPrecision,
testSerdeBufferBytes,
createObjWithProperty,
receiveObjectOnlyFromJs,
dateToNumber,
Expand Down Expand Up @@ -725,6 +727,14 @@ test('serde-large-number-precision', (t) => {
)
})

test('serde-buffer-bytes', (t) => {
t.is(testSerdeBufferBytes({ code: new Uint8Array([1, 2, 3]) }), 3n)
t.is(testSerdeBufferBytes({ code: new Uint8Array(0) }), 0n)

t.is(testSerdeBufferBytes({ code: Buffer.from([1, 2, 3]) }), 3n)
t.is(testSerdeBufferBytes({ code: Buffer.alloc(0) }), 0n)
})

test('buffer', (t) => {
let buf = getBuffer()
t.is(buf.toString('utf-8'), 'Hello world')
Expand Down Expand Up @@ -767,6 +777,11 @@ test('TypedArray', (t) => {
)
})

test('emptybuffer', (t) => {
let buf = new ArrayBuffer(0)
t.is(acceptArraybuffer(buf), 0n)
})

test('reset empty buffer', (t) => {
const empty = getEmptyBuffer()

Expand Down

0 comments on commit f3d665d

Please sign in to comment.