From df6f35fe8373a41290338d56c0726220772c53bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gaspard?= Date: Tue, 20 Feb 2024 06:22:07 +0100 Subject: [PATCH 1/3] fix: round-trip properly serde_json::Value --- src/de.rs | 16 ++-------------- tests/common/mod.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/de.rs b/src/de.rs index efae1d2..c7bbcd6 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1,4 +1,4 @@ -use js_sys::{Array, ArrayBuffer, JsString, Number, Object, Symbol, Uint8Array}; +use js_sys::{Array, ArrayBuffer, JsString, Number, Object, Uint8Array}; use serde::de::value::{MapDeserializer, SeqDeserializer}; use serde::de::{self, IntoDeserializer}; use std::convert::TryFrom; @@ -314,19 +314,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { // We need to handle this here because serde uses `deserialize_any` // for internally tagged enums visitor.visit_byte_buf(bytes) - } else if self.value.is_object() && - // The only reason we want to support objects here is because serde uses - // `deserialize_any` for internally tagged enums - // (see https://github.com/RReverser/serde-wasm-bindgen/pull/4#discussion_r352245020). - // - // We expect such enums to be represented via plain JS objects, so let's explicitly - // exclude Sets, Maps and any other iterables. These should be deserialized via concrete - // `deserialize_*` methods instead of us trying to guess the right target type. - // - // Hopefully we can rid of these hacks altogether once - // https://github.com/serde-rs/serde/issues/1183 is implemented / fixed on serde side. - !Symbol::iterator().js_in(&self.value) - { + } else if self.value.is_object() { self.deserialize_map(visitor) } else { self.invalid_type(visitor) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4004716..35f2163 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -664,6 +664,18 @@ fn enums() { } } +#[wasm_bindgen_test] +fn serde_json_value() { + test_via_round_trip_with_config( + serde_json::from_str::("[0, \"foo\"]").unwrap(), + &SERIALIZER, + ); + test_via_round_trip_with_config( + serde_json::from_str::(r#"{"foo": "bar"}"#).unwrap(), + &SERIALIZER, + ); +} + #[wasm_bindgen_test] fn preserved_value() { #[derive(serde::Deserialize, serde::Serialize, PartialEq, Clone, Debug)] From b6caa4dfea26c94524da43ab6d8aa1590507647d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gaspard?= Date: Sun, 25 Feb 2024 06:53:56 +0100 Subject: [PATCH 2/3] also test with Serializer::json_compatible --- tests/common/mod.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 35f2163..bb20957 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -13,6 +13,8 @@ use wasm_bindgen_test::wasm_bindgen_test; const SERIALIZER: Serializer = Serializer::new(); +const JSON_SERIALIZER: Serializer = Serializer::json_compatible(); + const BIGINT_SERIALIZER: Serializer = Serializer::new().serialize_large_number_types_as_bigints(true); @@ -665,7 +667,19 @@ fn enums() { } #[wasm_bindgen_test] -fn serde_json_value() { +fn serde_json_value_with_json() { + test_via_round_trip_with_config( + serde_json::from_str::("[0, \"foo\"]").unwrap(), + &JSON_SERIALIZER, + ); + test_via_round_trip_with_config( + serde_json::from_str::(r#"{"foo": "bar"}"#).unwrap(), + &JSON_SERIALIZER, + ); +} + +#[wasm_bindgen_test] +fn serde_json_value_with_default() { test_via_round_trip_with_config( serde_json::from_str::("[0, \"foo\"]").unwrap(), &SERIALIZER, From aeb2188b3b09146d808fa12fc665287f63b52177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gaspard?= Date: Tue, 27 Feb 2024 22:21:41 +0100 Subject: [PATCH 3/3] reintroduce type check to avoid deserializing too arbitrary stuff as the wrong type --- src/de.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/de.rs b/src/de.rs index c7bbcd6..e52cb67 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1,4 +1,4 @@ -use js_sys::{Array, ArrayBuffer, JsString, Number, Object, Uint8Array}; +use js_sys::{Array, ArrayBuffer, JsString, Map, Number, Object, Symbol, Uint8Array}; use serde::de::value::{MapDeserializer, SeqDeserializer}; use serde::de::{self, IntoDeserializer}; use std::convert::TryFrom; @@ -314,7 +314,22 @@ impl<'de> de::Deserializer<'de> for Deserializer { // We need to handle this here because serde uses `deserialize_any` // for internally tagged enums visitor.visit_byte_buf(bytes) - } else if self.value.is_object() { + } else if self.value.is_object() && + // The only reason we want to support objects here is because serde uses + // `deserialize_any` for internally tagged enums + // (see https://github.com/RReverser/serde-wasm-bindgen/pull/4#discussion_r352245020). + // + // We expect such enums to be represented via plain JS objects, so let's explicitly + // exclude Sets and other iterables. These should be deserialized via concrete + // `deserialize_*` methods instead of us trying to guess the right target type. + // + // We still do support Map, so that the format described here stays a self-describing + // format: we happen to serialize to Map, and it is not ambiguous. + // + // Hopefully we can rid of these hacks altogether once + // https://github.com/serde-rs/serde/issues/1183 is implemented / fixed on serde side. + (!Symbol::iterator().js_in(&self.value) || self.value.has_type::()) + { self.deserialize_map(visitor) } else { self.invalid_type(visitor)