bevy_reflect: Fix binary deserialization not working for unit structs (#6722)

# Objective 

Fixes #6713

Binary deserialization is failing for unit structs as well as structs with all ignored/skipped fields.

## Solution

Add a check for the number of possible fields in a struct before deserializing. If empty, don't attempt to deserialize any fields (as there will be none).

Note: ~~This does not apply to enums as they do not properly handle skipped fields (see #6721).~~ Enums still do not properly handle skipped fields, but I decided to include the logic for it anyways to account for `#[reflect(ignore)]`'d fields in the meantime.

---

## Changelog

- Fix bug where deserializing unit structs would fail for non-self-describing formats
This commit is contained in:
Gino Valente 2022-11-23 00:01:36 +00:00
parent a1607b8065
commit 4e2374334f
2 changed files with 193 additions and 18 deletions

View file

@ -341,6 +341,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
struct_info.field_names(),
StructVisitor {
struct_info,
registration: self.registration,
registry: self.registry,
},
)?;
@ -411,6 +412,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
enum_info.variant_names(),
EnumVisitor {
enum_info,
registration: self.registration,
registry: self.registry,
},
)?
@ -439,6 +441,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
struct StructVisitor<'a> {
struct_info: &'static StructInfo,
registration: &'a TypeRegistration,
registry: &'a TypeRegistry,
}
@ -463,6 +466,18 @@ impl<'a, 'de> Visitor<'de> for StructVisitor<'a> {
let mut index = 0usize;
let mut output = DynamicStruct::default();
let ignored_len = self
.registration
.data::<SerializationData>()
.map(|data| data.len())
.unwrap_or(0);
let field_len = self.struct_info.field_len().saturating_sub(ignored_len);
if field_len == 0 {
// Handle unit structs and ignored fields
return Ok(output);
}
while let Some(value) = seq.next_element_seed(TypedReflectDeserializer {
registration: self
.struct_info
@ -501,6 +516,21 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> {
let mut index = 0usize;
let mut tuple_struct = DynamicTupleStruct::default();
let ignored_len = self
.registration
.data::<SerializationData>()
.map(|data| data.len())
.unwrap_or(0);
let field_len = self
.tuple_struct_info
.field_len()
.saturating_sub(ignored_len);
if field_len == 0 {
// Handle unit structs and ignored fields
return Ok(tuple_struct);
}
let get_field_registration = |index: usize| -> Result<&'a TypeRegistration, V::Error> {
let field = self.tuple_struct_info.field_at(index).ok_or_else(|| {
de::Error::custom(format_args!(
@ -675,6 +705,7 @@ impl<'a, 'de> Visitor<'de> for MapVisitor<'a> {
struct EnumVisitor<'a> {
enum_info: &'static EnumInfo,
registration: &'a TypeRegistration,
registry: &'a TypeRegistry,
}
@ -701,6 +732,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> {
struct_info.field_names(),
StructVariantVisitor {
struct_info,
registration: self.registration,
registry: self.registry,
},
)?
@ -722,6 +754,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> {
tuple_info.field_len(),
TupleVariantVisitor {
tuple_info,
registration: self.registration,
registry: self.registry,
},
)?
@ -787,6 +820,7 @@ impl<'de> DeserializeSeed<'de> for VariantDeserializer {
struct StructVariantVisitor<'a> {
struct_info: &'static StructVariantInfo,
registration: &'a TypeRegistration,
registry: &'a TypeRegistry,
}
@ -811,6 +845,18 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> {
let mut index = 0usize;
let mut output = DynamicStruct::default();
let ignored_len = self
.registration
.data::<SerializationData>()
.map(|data| data.len())
.unwrap_or(0);
let field_len = self.struct_info.field_len().saturating_sub(ignored_len);
if field_len == 0 {
// Handle all fields being ignored
return Ok(output);
}
while let Some(value) = seq.next_element_seed(TypedReflectDeserializer {
registration: self
.struct_info
@ -831,6 +877,7 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> {
struct TupleVariantVisitor<'a> {
tuple_info: &'static TupleVariantInfo,
registration: &'a TypeRegistration,
registry: &'a TypeRegistry,
}
@ -845,6 +892,18 @@ impl<'a, 'de> Visitor<'de> for TupleVariantVisitor<'a> {
where
V: SeqAccess<'de>,
{
let ignored_len = self
.registration
.data::<SerializationData>()
.map(|data| data.len())
.unwrap_or(0);
let field_len = self.tuple_info.field_len().saturating_sub(ignored_len);
if field_len == 0 {
// Handle all fields being ignored
return Ok(DynamicTuple::default());
}
visit_tuple(&mut seq, self.tuple_info, self.registry)
}
}
@ -1011,10 +1070,15 @@ mod tests {
map_value: HashMap<u8, usize>,
struct_value: SomeStruct,
tuple_struct_value: SomeTupleStruct,
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum,
newtype_enum: SomeEnum,
tuple_enum: SomeEnum,
struct_enum: SomeEnum,
ignored_struct: SomeIgnoredStruct,
ignored_tuple_struct: SomeIgnoredTupleStruct,
ignored_struct_variant: SomeIgnoredEnum,
ignored_tuple_variant: SomeIgnoredEnum,
custom_deserialize: CustomDeserialize,
}
@ -1026,6 +1090,18 @@ mod tests {
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeTupleStruct(String);
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeUnitStruct;
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeIgnoredStruct {
#[reflect(ignore)]
ignored: i32,
}
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeIgnoredTupleStruct(#[reflect(ignore)] i32);
#[derive(Reflect, FromReflect, Debug, PartialEq, Deserialize)]
struct SomeDeserializableStruct {
foo: i64,
@ -1050,14 +1126,27 @@ mod tests {
Struct { foo: String },
}
#[derive(Reflect, FromReflect, Debug, PartialEq)]
enum SomeIgnoredEnum {
Tuple(#[reflect(ignore)] f32, #[reflect(ignore)] f32),
Struct {
#[reflect(ignore)]
foo: String,
},
}
fn get_registry() -> TypeRegistry {
let mut registry = TypeRegistry::default();
registry.register::<MyStruct>();
registry.register::<SomeStruct>();
registry.register::<SomeTupleStruct>();
registry.register::<SomeUnitStruct>();
registry.register::<SomeIgnoredStruct>();
registry.register::<SomeIgnoredTupleStruct>();
registry.register::<CustomDeserialize>();
registry.register::<SomeDeserializableStruct>();
registry.register::<SomeEnum>();
registry.register::<SomeIgnoredEnum>();
registry.register::<i8>();
registry.register::<String>();
registry.register::<i64>();
@ -1090,12 +1179,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 0 },
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::default(),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
custom_deserialize: CustomDeserialize {
value: 100,
inner_struct: SomeDeserializableStruct { foo: 101 },
@ -1125,12 +1221,17 @@ mod tests {
foo: 999999999,
),
tuple_struct_value: ("Tuple Struct"),
unit_struct: (),
unit_enum: Unit,
newtype_enum: NewType(123),
tuple_enum: Tuple(1.23, 3.21),
struct_enum: Struct(
foo: "Struct variant value",
),
ignored_struct: (),
ignored_tuple_struct: (),
ignored_struct_variant: Struct(),
ignored_tuple_variant: Tuple(),
custom_deserialize: (
value: 100,
renamed: (
@ -1337,12 +1438,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 0 },
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::default(),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
custom_deserialize: CustomDeserialize {
value: 100,
inner_struct: SomeDeserializableStruct { foo: 101 },
@ -1363,7 +1471,8 @@ mod tests {
108, 101, 32, 83, 116, 114, 117, 99, 116, 0, 0, 0, 0, 1, 0, 0, 0, 123, 0, 0, 0, 0, 0,
0, 0, 2, 0, 0, 0, 164, 112, 157, 63, 164, 112, 77, 64, 3, 0, 0, 0, 20, 0, 0, 0, 0, 0,
0, 0, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97,
108, 117, 101, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0, 0,
108, 117, 101, 1, 0, 0, 0, 0, 0, 0, 0, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0,
0,
];
let deserializer = UntypedReflectDeserializer::new(&registry);
@ -1392,12 +1501,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 0 },
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::default(),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
custom_deserialize: CustomDeserialize {
value: 100,
inner_struct: SomeDeserializableStruct { foo: 101 },
@ -1409,14 +1525,15 @@ mod tests {
let input = vec![
129, 217, 40, 98, 101, 118, 121, 95, 114, 101, 102, 108, 101, 99, 116, 58, 58, 115,
101, 114, 100, 101, 58, 58, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121,
83, 116, 114, 117, 99, 116, 158, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111, 114,
108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0, 1, 2,
149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172, 84, 117,
112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 164, 85, 110, 105, 116, 129, 167, 78,
101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146, 202, 63, 157,
112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116, 145, 180, 83,
116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97, 108, 117,
101, 146, 100, 145, 101,
83, 116, 114, 117, 99, 116, 220, 0, 19, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111,
114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0,
1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172, 84,
117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105, 116, 129,
167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146, 202,
63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116, 145,
180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97, 108,
117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165, 84, 117, 112,
108, 101, 144, 146, 100, 145, 101,
];
let deserializer = UntypedReflectDeserializer::new(&registry);

View file

@ -488,10 +488,15 @@ mod tests {
map_value: HashMap<u8, usize>,
struct_value: SomeStruct,
tuple_struct_value: SomeTupleStruct,
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum,
newtype_enum: SomeEnum,
tuple_enum: SomeEnum,
struct_enum: SomeEnum,
ignored_struct: SomeIgnoredStruct,
ignored_tuple_struct: SomeIgnoredTupleStruct,
ignored_struct_variant: SomeIgnoredEnum,
ignored_tuple_variant: SomeIgnoredEnum,
custom_serialize: CustomSerialize,
}
@ -503,6 +508,18 @@ mod tests {
#[derive(Reflect, Debug, PartialEq)]
struct SomeTupleStruct(String);
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeUnitStruct;
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeIgnoredStruct {
#[reflect(ignore)]
ignored: i32,
}
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct SomeIgnoredTupleStruct(#[reflect(ignore)] i32);
#[derive(Reflect, Debug, PartialEq)]
enum SomeEnum {
Unit,
@ -511,6 +528,15 @@ mod tests {
Struct { foo: String },
}
#[derive(Reflect, FromReflect, Debug, PartialEq)]
enum SomeIgnoredEnum {
Tuple(#[reflect(ignore)] f32, #[reflect(ignore)] f32),
Struct {
#[reflect(ignore)]
foo: String,
},
}
#[derive(Reflect, Debug, PartialEq, Serialize)]
struct SomeSerializableStruct {
foo: i64,
@ -532,6 +558,10 @@ mod tests {
registry.register::<MyStruct>();
registry.register::<SomeStruct>();
registry.register::<SomeTupleStruct>();
registry.register::<SomeUnitStruct>();
registry.register::<SomeIgnoredStruct>();
registry.register::<SomeIgnoredTupleStruct>();
registry.register::<SomeIgnoredEnum>();
registry.register::<CustomSerialize>();
registry.register::<SomeEnum>();
registry.register::<SomeSerializableStruct>();
@ -557,12 +587,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 123 },
ignored_tuple_struct: SomeIgnoredTupleStruct(123),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::from("Struct Variant"),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(1.23, 3.45),
custom_serialize: CustomSerialize {
value: 100,
inner_struct: SomeSerializableStruct { foo: 101 },
@ -600,12 +637,17 @@ mod tests {
foo: 999999999,
),
tuple_struct_value: ("Tuple Struct"),
unit_struct: (),
unit_enum: Unit,
newtype_enum: NewType(123),
tuple_enum: Tuple(1.23, 3.21),
struct_enum: Struct(
foo: "Struct variant value",
),
ignored_struct: (),
ignored_tuple_struct: (),
ignored_struct_variant: Struct(),
ignored_tuple_variant: Tuple(),
custom_serialize: (
value: 100,
renamed: (
@ -745,12 +787,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 123 },
ignored_tuple_struct: SomeIgnoredTupleStruct(123),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::from("Struct Variant"),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(1.23, 3.45),
custom_serialize: CustomSerialize {
value: 100,
inner_struct: SomeSerializableStruct { foo: 101 },
@ -774,7 +823,8 @@ mod tests {
112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 0, 0, 0, 0, 1, 0, 0, 0, 123, 0, 0, 0, 0,
0, 0, 0, 2, 0, 0, 0, 164, 112, 157, 63, 164, 112, 77, 64, 3, 0, 0, 0, 20, 0, 0, 0, 0,
0, 0, 0, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97,
108, 117, 101, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0, 0,
108, 117, 101, 1, 0, 0, 0, 0, 0, 0, 0, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0,
0,
];
assert_eq!(expected, bytes);
@ -795,12 +845,19 @@ mod tests {
map_value: map,
struct_value: SomeStruct { foo: 999999999 },
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
unit_struct: SomeUnitStruct,
unit_enum: SomeEnum::Unit,
newtype_enum: SomeEnum::NewType(123),
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
struct_enum: SomeEnum::Struct {
foo: String::from("Struct variant value"),
},
ignored_struct: SomeIgnoredStruct { ignored: 123 },
ignored_tuple_struct: SomeIgnoredTupleStruct(123),
ignored_struct_variant: SomeIgnoredEnum::Struct {
foo: String::from("Struct Variant"),
},
ignored_tuple_variant: SomeIgnoredEnum::Tuple(1.23, 3.45),
custom_serialize: CustomSerialize {
value: 100,
inner_struct: SomeSerializableStruct { foo: 101 },
@ -815,14 +872,15 @@ mod tests {
let expected: Vec<u8> = vec![
129, 217, 41, 98, 101, 118, 121, 95, 114, 101, 102, 108, 101, 99, 116, 58, 58, 115,
101, 114, 100, 101, 58, 58, 115, 101, 114, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77,
121, 83, 116, 114, 117, 99, 116, 158, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111,
114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0,
1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172, 84,
117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 164, 85, 110, 105, 116, 129, 167,
78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146, 202, 63,
157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116, 145, 180,
83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97, 108, 117,
101, 146, 100, 145, 101,
121, 83, 116, 114, 117, 99, 116, 220, 0, 19, 123, 172, 72, 101, 108, 108, 111, 32, 119,
111, 114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255,
0, 1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172,
84, 117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105, 116,
129, 167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146,
202, 63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116,
145, 180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97,
108, 117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165, 84, 117,
112, 108, 101, 144, 146, 100, 145, 101,
];
assert_eq!(expected, bytes);