bevy_reflect: Fix deserialization with readers (bevyengine#6894)

# Objective

Fixes bevyengine#6891

## Solution

Replaces deserializing map keys as `&str` with deserializing them as `String`.

This bug seems to occur when using something like `File` or `BufReader` rather than bytes or a string directly (I only tested `File` and `BufReader` for `rmp-serde` and `serde_json`). This might be an issue with other `Read` impls as well (except `&[u8]` it seems).

We already had passing tests for Message Pack but none that use a `File` or `BufReader`. This PR also adds or modifies tests to check for this in the future.

This change was also based on [feedback](bevyengine#4561 (comment)) I received in a previous PR.

---

## Changelog

- Fix bug where scene deserialization using certain readers could fail (e.g. `BufReader`, `File`, etc.)

crates/bevy_reflect/Cargo.toml

@@ -29,7 +29,7 @@ downcast-rs = "1.2"
 parking_lot = "0.12.1"
 thiserror = "1.0"
 once_cell = "1.11"
-serde = "1"
+serde = { version = "1", features = ["derive"] }
 smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true }
 glam = { version = "0.22", features = ["serde"], optional = true }
 

crates/bevy_reflect/src/serde/de.rs

@@ -12,6 +12,7 @@ use serde::de::{
 };
 use serde::Deserialize;
 use std::any::TypeId;
+use std::borrow::Cow;
 use std::fmt;
 use std::fmt::{Debug, Display, Formatter};
 use std::slice::Iter;
@@ -210,6 +211,13 @@ impl<'de> Visitor<'de> for U32Visitor {
  }
 }
 
+/// Helper struct for deserializing strings without allocating (when possible).
+///
+/// Based on [this comment](https:/bevyengine/bevy/pull/6894#discussion_r1045069010).
+#[derive(Deserialize)]
+#[serde(transparent)]
+struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>);
+
 /// A general purpose deserializer for reflected types.
 ///
 /// This will return a [`Box<dyn Reflect>`] containing the deserialized data.
@@ -273,8 +281,9 @@ impl<'a, 'de> Visitor<'de> for UntypedReflectDeserializerVisitor<'a> {
  A: MapAccess<'de>,
  {
  let type_name = map
- .next_key::<String>()?
- .ok_or_else(|| Error::invalid_length(0, &"at least one entry"))?;
+ .next_key::<BorrowableCowStr>()?
+ .ok_or_else(|| Error::invalid_length(0, &"at least one entry"))?
+ .0;
 
  let registration = self.registry.get_with_name(&type_name).ok_or_else(|| {
  Error::custom(format_args!("No registration found for `{type_name}`"))
@@ -1536,10 +1545,11 @@ mod tests {
  108, 101, 144, 146, 100, 145, 101,
  ];
 
- let deserializer = UntypedReflectDeserializer::new(&registry);
+ let mut reader = std::io::BufReader::new(input.as_slice());
 
+ let deserializer = UntypedReflectDeserializer::new(&registry);
  let dynamic_output = deserializer
- .deserialize(&mut rmp_serde::Deserializer::new(input.as_slice()))
+ .deserialize(&mut rmp_serde::Deserializer::new(&mut reader))
  .unwrap();
 
  let output = <MyStruct as FromReflect>::from_reflect(dynamic_output.as_ref()).unwrap();

crates/bevy_scene/Cargo.toml

@@ -34,3 +34,4 @@ thiserror = "1.0"
 [dev-dependencies]
 postcard = { version = "1.0", features = ["alloc"] }
 bincode = "1.3"
+rmp-serde = "1.1"

crates/bevy_scene/src/serde.rs

@@ -9,6 +9,7 @@ use serde::{
  ser::SerializeStruct,
  Deserialize, Deserializer, Serialize, Serializer,
 };
+use std::borrow::Cow;
 use std::fmt::Formatter;
 
 pub const SCENE_STRUCT: &str = "Scene";
@@ -352,14 +353,14 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> {
  {
  let mut added = HashSet::new();
  let mut components = Vec::new();
- while let Some(key) = map.next_key::<&str>()? {
- if !added.insert(key) {
+ while let Some(BorrowableCowStr(key)) = map.next_key()? {
+ if !added.insert(key.clone()) {
  return Err(Error::custom(format!("duplicate component: `{key}`")));
  }
 
  let registration = self
  .registry
- .get_with_name(key)
+ .get_with_name(&key)
  .ok_or_else(|| Error::custom(format!("no registration found for `{key}`")))?;
  components.push(
  map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?,
@@ -384,6 +385,13 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> {
  }
 }
 
+/// Helper struct for deserializing strings without allocating (when possible).
+///
+/// Based on [this comment](https:/bevyengine/bevy/pull/6894#discussion_r1045069010).
+#[derive(Deserialize)]
+#[serde(transparent)]
+struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>);
+
 #[cfg(test)]
 mod tests {
  use crate::serde::{SceneDeserializer, SceneSerializer};
@@ -394,6 +402,8 @@ mod tests {
  use bevy_reflect::{FromReflect, Reflect, ReflectSerialize};
  use bincode::Options;
  use serde::de::DeserializeSeed;
+ use serde::Serialize;
+ use std::io::BufReader;
 
  #[derive(Component, Reflect, Default)]
  #[reflect(Component)]
@@ -567,6 +577,49 @@ mod tests {
  assert_scene_eq(&scene, &deserialized_scene);
  }
 
+ #[test]
+ fn should_roundtrip_messagepack() {
+ let mut world = create_world();
+
+ world.spawn(MyComponent {
+ foo: [1, 2, 3],
+ bar: (1.3, 3.7),
+ baz: MyEnum::Tuple("Hello World!".to_string()),
+ });
+
+ let registry = world.resource::<AppTypeRegistry>();
+
+ let scene = DynamicScene::from_world(&world, registry);
+
+ let scene_serializer = SceneSerializer::new(&scene, &registry.0);
+ let mut buf = Vec::new();
+ let mut ser = rmp_serde::Serializer::new(&mut buf);
+ scene_serializer.serialize(&mut ser).unwrap();
+
+ assert_eq!(
+ vec![
+ 145, 129, 0, 145, 129, 217, 37, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, 58,
+ 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67,
+ 111, 109, 112, 111, 110, 101, 110, 116, 147, 147, 1, 2, 3, 146, 202, 63, 166, 102,
+ 102, 202, 64, 108, 204, 205, 129, 165, 84, 117, 112, 108, 101, 172, 72, 101, 108,
+ 108, 111, 32, 87, 111, 114, 108, 100, 33
+ ],
+ buf
+ );
+
+ let scene_deserializer = SceneDeserializer {
+ type_registry: &registry.0.read(),
+ };
+ let mut reader = BufReader::new(buf.as_slice());
+
+ let deserialized_scene = scene_deserializer
+ .deserialize(&mut rmp_serde::Deserializer::new(&mut reader))
+ .unwrap();
+
+ assert_eq!(1, deserialized_scene.entities.len());
+ assert_scene_eq(&scene, &deserialized_scene);
+ }
+
  #[test]
  fn should_roundtrip_bincode() {
  let mut world = create_world();