Skip to content

Commit

Permalink
Handle non-serializable values in Array & Object (#639)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbourassa authored May 15, 2024
1 parent 30c21d0 commit 162b3da
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 9 deletions.
4 changes: 3 additions & 1 deletion crates/javy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Introduce `rquickjs` to interface with QuickJS instead of `quickjs-wasm-rs`;
this version no longer includes re-exports from `quickjs-wasm-rs`.
this version no longer includes re-exports from `quickjs-wasm-rs`.
- `javy::serde::de::Deserializer` should now match `JSON.stringify`: non-JSON
primitives are skipped in Objects and nullified in Arrays.

## [2.2.0] - 2024-01-31

Expand Down
90 changes: 82 additions & 8 deletions crates/javy/src/serde/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::serde::err::{Error, Result};
use crate::serde::{MAX_SAFE_INTEGER, MIN_SAFE_INTEGER};
use crate::{from_js_error, to_string_lossy};
use anyhow::anyhow;
use rquickjs::Null;
use serde::de::{self, Error as SerError};
use serde::forward_to_deserialize_any;

Expand Down Expand Up @@ -200,14 +201,23 @@ impl<'a, 'de> de::MapAccess<'de> for MapAccess<'a, 'de> {
where
K: de::DeserializeSeed<'de>,
{
if let Some(kv) = self.properties.next() {
let (k, v) = kv.map_err(|e| from_js_error(self.de.value.ctx().clone(), e))?;
self.de.value = k.clone();
self.de.map_key = true;
self.de.current_kv = Some((k, v));
seed.deserialize(&mut *self.de).map(Some)
} else {
Ok(None)
loop {
if let Some(kv) = self.properties.next() {
let (k, v) = kv.map_err(|e| from_js_error(self.de.value.ctx().clone(), e))?;

// Entries with non-JSONable values are skipped to respect JSON.stringify's spec
if !is_jsonable(&v) {
continue;
}

self.de.value = k.clone();
self.de.map_key = true;
self.de.current_kv = Some((k, v));

return seed.deserialize(&mut *self.de).map(Some);
} else {
return Ok(None);
}
}
}

Expand Down Expand Up @@ -238,6 +248,13 @@ impl<'a, 'de> de::SeqAccess<'de> for SeqAccess<'a, 'de> {
self.de.value = self
.seq
.get(self.index)
.map(|v| {
if is_jsonable(&v) {
v
} else {
Null.into_value(self.seq.ctx().clone())
}
})
.map_err(|e| from_js_error(self.seq.ctx().clone(), e))?;
self.index += 1;
seed.deserialize(&mut *self.de).map(Some)
Expand All @@ -247,6 +264,17 @@ impl<'a, 'de> de::SeqAccess<'de> for SeqAccess<'a, 'de> {
}
}

fn is_jsonable(value: &Value<'_>) -> bool {
!matches!(
value.type_of(),
rquickjs::Type::Undefined
| rquickjs::Type::Symbol
| rquickjs::Type::Function
| rquickjs::Type::Uninitialized
| rquickjs::Type::Constructor
)
}

#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
Expand Down Expand Up @@ -425,4 +453,50 @@ mod tests {
assert_eq!(vec![1, 2, 3], val);
});
}

#[test]
fn test_non_json_object_values_are_dropped() {
let rt = Runtime::default();
rt.context().with(|cx| {
cx.eval::<Value<'_>, _>(
r#"
var unitialized;
var a = {
a: undefined,
b: function() {},
c: Symbol(),
d: () => {},
e: unitialized,
};"#,
)
.unwrap();
let v = cx.globals().get("a").unwrap();

let val = deserialize_value::<BTreeMap<String, ()>>(v);
assert_eq!(BTreeMap::new(), val);
});
}

#[test]
fn test_non_json_array_values_are_null() {
let rt = Runtime::default();
rt.context().with(|cx| {
cx.eval::<Value<'_>, _>(
r#"
var unitialized;
var a = [
undefined,
function() {},
Symbol(),
() => {},
unitialized,
];"#,
)
.unwrap();
let v = cx.globals().get("a").unwrap();

let val = deserialize_value::<Vec<Option<()>>>(v);
assert_eq!(vec![None; 5], val);
});
}
}

0 comments on commit 162b3da

Please sign in to comment.