Skip to content

Commit

Permalink
More fixes for flattened enums
Browse files Browse the repository at this point in the history
  • Loading branch information
@juntyr
juntyr committed Apr 11, 2024
1 parent 5a8ab5d commit e063a25
Showing 1 changed file with 49 additions and 17 deletions.
66 changes: 49 additions & 17 deletions fuzz/fuzz_targets/bench/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1149,12 +1149,18 @@ impl<'a, 'de> DeserializeSeed<'de> for BorrowedTypedSerdeData<'a> {
ty: self.ty,
value: expected,
})
.deserialize(deserializer) {
.deserialize(deserializer)
{
Ok(()) => Ok(()),
// Duplicate struct fields only cause issues inside internally (or adjacently)
// tagged or untagged enums (or in flattened fields where we detect them
// before they cause issues), so we allow them in arbitrary otherwise
Err(err) if format!("{err}").starts_with("Unexpected duplicate field named") => Ok(()),
Err(err)
if format!("{err}")
.starts_with("Unexpected duplicate field named") =>
{
Ok(())
}
Err(err) => panic!(
"expected untagged {:?} but failed with {}",
Some(expected),
Expand Down Expand Up @@ -5135,7 +5141,7 @@ impl<'a> SerdeDataType<'a> {
}
let value = SerdeDataValue::Struct { fields: r#struct };
let mut has_flatten_map = false;
let mut has_unknown_key_inside_flatten = false;
let mut has_unknown_key_inside_flatten = tag.is_some();
for (ty, flatten) in fields.1.iter().zip(fields.2.iter()) {
if *flatten && !ty.supported_inside_untagged(pretty, false, false) {
// Flattened fields are deserialised through serde's content type
Expand Down Expand Up @@ -5236,7 +5242,7 @@ impl<'a> SerdeDataType<'a> {
{
return Err(arbitrary::Error::IncorrectFormat);
}
if matches!(representation, SerdeEnumRepresentation::InternallyTagged { tag: _ } if !inner.supported_inside_internally_tagged_newtype(false))
if matches!(representation, SerdeEnumRepresentation::InternallyTagged { tag: _ } if !inner.supported_inside_internally_tagged_newtype(false, false))
{
return Err(arbitrary::Error::IncorrectFormat);
}
Expand Down Expand Up @@ -5534,6 +5540,7 @@ impl<'a> SerdeDataType<'a> {
fn supported_inside_internally_tagged_newtype(
&self,
inside_untagged_newtype_variant: bool,
has_unknown_key: bool,
) -> bool {
// See https://github.com/serde-rs/serde/blob/ddc1ee564b33aa584e5a66817aafb27c3265b212/serde/src/private/ser.rs#L94-L336
match self {
Expand Down Expand Up @@ -5572,9 +5579,11 @@ impl<'a> SerdeDataType<'a> {
// which is only serialised with the tag
!inside_untagged_newtype_variant
}
SerdeDataType::Newtype { name: _, inner: ty } => {
ty.supported_inside_internally_tagged_newtype(inside_untagged_newtype_variant)
}
SerdeDataType::Newtype { name: _, inner: ty } => ty
.supported_inside_internally_tagged_newtype(
inside_untagged_newtype_variant,
has_unknown_key,
),
SerdeDataType::TupleStruct { name: _, fields: _ } => false,
SerdeDataType::Struct {
name: _,
Expand All @@ -5586,6 +5595,15 @@ impl<'a> SerdeDataType<'a> {
variants,
representation,
} => {
if matches!(representation, SerdeEnumRepresentation::ExternallyTagged)
&& has_unknown_key
{
// BUG: an externally tagged enum inside an internally tagged
// enum expects to find a map with a single key, but an
// unknown key will cause a failure
return false;
}

variants.1.iter().all(|ty| match ty {
SerdeDataVariantType::Unit | SerdeDataVariantType::TaggedOther => {
// BUG: an untagged unit variant requires a unit,
Expand All @@ -5595,7 +5613,7 @@ impl<'a> SerdeDataType<'a> {
}
SerdeDataVariantType::Newtype { inner: ty } => {
if matches!(representation, SerdeEnumRepresentation::Untagged) {
ty.supported_inside_internally_tagged_newtype(true)
ty.supported_inside_internally_tagged_newtype(true, has_unknown_key)
} else {
true
}
Expand All @@ -5611,12 +5629,17 @@ impl<'a> SerdeDataType<'a> {
}
}

fn supported_inside_flatten(&self, inside_untagged_newtype_variant: bool) -> bool {
fn supported_inside_flatten(
&self,
inside_untagged_newtype_variant: bool,
inside_flattened_option: bool,
) -> bool {
match self {
SerdeDataType::Unit => {
// BUG: a unit inside an untagged newtype variant expects a unit
// but only the tag is there
!inside_untagged_newtype_variant
// BUG: a unit inside a flattened Some is interpreted as None
!inside_untagged_newtype_variant && !inside_flattened_option
}
SerdeDataType::Bool => false,
SerdeDataType::I8 => false,
Expand All @@ -5637,19 +5660,25 @@ impl<'a> SerdeDataType<'a> {
SerdeDataType::String => false,
SerdeDataType::ByteBuf => false,
SerdeDataType::Option { inner } => {
inner.supported_inside_flatten(inside_untagged_newtype_variant)
inner.supported_inside_flatten(inside_untagged_newtype_variant, true)
}
SerdeDataType::Array { kind: _, len: _ } => false,
SerdeDataType::Tuple { elems: _ } => false,
SerdeDataType::Vec { item: _ } => false,
SerdeDataType::Map { key, value: _ } => key.supported_inside_flatten_key(),
SerdeDataType::Map { key, value } => {
key.supported_inside_flatten_key()
&& value.supported_inside_flatten(inside_untagged_newtype_variant, false)
}
SerdeDataType::UnitStruct { name: _ } => false,
SerdeDataType::Newtype { name, inner } => {
if *name == RAW_VALUE_TOKEN {
return false;
}

inner.supported_inside_flatten(inside_untagged_newtype_variant)
inner.supported_inside_flatten(
inside_untagged_newtype_variant,
inside_flattened_option,
)
}
SerdeDataType::TupleStruct { name: _, fields: _ } => false,
SerdeDataType::Struct {
Expand Down Expand Up @@ -5678,9 +5707,12 @@ impl<'a> SerdeDataType<'a> {
}
SerdeDataVariantType::Newtype { inner } => {
if matches!(representation, SerdeEnumRepresentation::Untagged) {
inner.supported_inside_flatten(true)
inner.supported_inside_flatten(true, inside_flattened_option)
} else {
inner.supported_inside_flatten(inside_untagged_newtype_variant)
inner.supported_inside_flatten(
inside_untagged_newtype_variant,
inside_flattened_option,
)
}
}
SerdeDataVariantType::Tuple { fields: _ } => {
Expand Down Expand Up @@ -5871,7 +5903,7 @@ impl<'a> SerdeDataType<'a> {
if matches!(representation, SerdeEnumRepresentation::InternallyTagged { tag: _ }) {
// BUG: an flattened internally tagged newtype alongside other flattened data
// must not contain a unit, unit struct, or untagged unit variant
if !inner.supported_inside_internally_tagged_newtype(true) {
if !inner.supported_inside_internally_tagged_newtype(true, *has_unknown_key) {
return false;
}

Expand Down Expand Up @@ -6016,7 +6048,7 @@ fn arbitrary_struct_fields_recursion_guard<'a>(
while u.arbitrary()? {
fields.push(<&str>::arbitrary(u)?);
let ty = SerdeDataType::arbitrary(u)?;
flattened.push(u.arbitrary()? && ty.supported_inside_flatten(false));
flattened.push(u.arbitrary()? && ty.supported_inside_flatten(false, false));
types.push(ty);
}

Expand Down

0 comments on commit e063a25

Please sign in to comment.