Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disallow deserialization of struct and struct variant with named fields from sequence #2640

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Disallow deserialization of struct and struct variant with named fields from sequence #2640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 0 additions & 43 deletions serde_derive/src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,33 +960,6 @@ fn deserialize_struct(
.collect();
let field_visitor = deserialize_field_identifier(&field_names_idents, cattrs);

// untagged struct variants do not get a visit_seq method. The same applies to
// structs that only have a map representation.
let visit_seq = match form {
StructForm::Untagged(..) => None,
_ if cattrs.has_flatten() => None,
_ => {
let mut_seq = if field_names_idents.is_empty() {
quote!(_)
} else {
quote!(mut __seq)
};

let visit_seq = Stmts(deserialize_seq(
&type_path, params, fields, true, cattrs, expecting,
));

Some(quote! {
#[inline]
fn visit_seq<__A>(self, #mut_seq: __A) -> _serde::__private::Result<Self::Value, __A::Error>
where
__A: _serde::de::SeqAccess<#delife>,
{
#visit_seq
}
})
}
};
let visit_map = Stmts(deserialize_map(&type_path, params, fields, cattrs));

let visitor_seed = match form {
Expand Down Expand Up @@ -1064,8 +1037,6 @@ fn deserialize_struct(
_serde::__private::Formatter::write_str(__formatter, #expecting)
}

#visit_seq

#[inline]
fn visit_map<__A>(self, mut __map: __A) -> _serde::__private::Result<Self::Value, __A::Error>
where
Expand Down Expand Up @@ -1118,12 +1089,6 @@ fn deserialize_struct_in_place(

let field_visitor = deserialize_field_identifier(&field_names_idents, cattrs);

let mut_seq = if field_names_idents.is_empty() {
quote!(_)
} else {
quote!(mut __seq)
};
let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, expecting));
let visit_map = Stmts(deserialize_map_in_place(params, fields, cattrs));
let field_names = field_names_idents
.iter()
Expand All @@ -1150,14 +1115,6 @@ fn deserialize_struct_in_place(
_serde::__private::Formatter::write_str(__formatter, #expecting)
}

#[inline]
fn visit_seq<__A>(self, #mut_seq: __A) -> _serde::__private::Result<Self::Value, __A::Error>
where
__A: _serde::de::SeqAccess<#delife>,
{
#visit_seq
}

#[inline]
fn visit_map<__A>(self, mut __map: __A) -> _serde::__private::Result<Self::Value, __A::Error>
where
Expand Down
27 changes: 0 additions & 27 deletions test_suite/tests/test_annotations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2962,33 +2962,6 @@ mod flatten {
);
}

// Reaches crate::private::de::content::VariantDeserializer::struct_variant
// Content::Seq case
// via FlatMapDeserializer::deserialize_enum
#[test]
fn struct_from_seq() {
assert_de_tokens(
&Flatten {
data: Enum::Struct {
index: 0,
value: 42,
},
extra: HashMap::from_iter([("extra_key".into(), "extra value".into())]),
},
&[
Token::Map { len: None },
Token::Str("Struct"), // variant
Token::Seq { len: Some(2) },
Token::U32(0), // index
Token::U32(42), // value
Token::SeqEnd,
Token::Str("extra_key"),
Token::Str("extra value"),
Token::MapEnd,
],
);
}

// Reaches crate::private::de::content::VariantDeserializer::struct_variant
// Content::Map case
// via FlatMapDeserializer::deserialize_enum
Expand Down
9 changes: 0 additions & 9 deletions test_suite/tests/test_de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1381,15 +1381,6 @@ fn test_struct() {
Token::StructEnd,
],
);
test(
Struct { a: 1, b: 2, c: 0 },
&[
Token::Seq { len: Some(3) },
Token::I32(1),
Token::I32(2),
Token::SeqEnd,
],
);
}

#[test]
Expand Down
93 changes: 93 additions & 0 deletions test_suite/tests/test_de_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,20 @@ struct StructSkipAllDenyUnknown {
a: i32,
}

#[derive(PartialEq, Debug, Deserialize)]
struct StructNotFromSeq {
a: i32,
}

#[derive(Default, PartialEq, Debug)]
struct NotDeserializable;

#[derive(Debug, PartialEq, Deserialize)]
struct Flatten {
#[serde(flatten)]
data: Enum,
}

#[derive(PartialEq, Debug, Deserialize)]
enum Enum {
#[allow(dead_code)]
Expand All @@ -68,6 +79,24 @@ enum EnumSkipAll {
Skipped,
}

#[derive(Debug, PartialEq, Deserialize)]
#[serde(tag = "type")]
enum InternallyTagged {
A { a: u8 },
B(StructNotFromSeq),
}

#[derive(Debug, PartialEq, Deserialize)]
#[serde(tag = "type")]
enum Outer {
Inner(Inner),
}

#[derive(Debug, PartialEq, Deserialize)]
enum Inner {
Struct { f: u8 },
}

#[test]
fn test_i8() {
let test = assert_de_tokens_error::<i8>;
Expand Down Expand Up @@ -1179,6 +1208,14 @@ fn test_skip_all_deny_unknown() {
);
}

#[test]
fn test_struct_not_from_seq() {
assert_de_tokens_error::<StructNotFromSeq>(
&[Token::Seq { len: Some(1) }],
"invalid type: sequence, expected struct StructNotFromSeq",
);
}

#[test]
fn test_unknown_variant() {
assert_de_tokens_error::<Enum>(
Expand Down Expand Up @@ -1248,6 +1285,62 @@ fn test_enum_out_of_range() {
);
}

#[test]
fn test_struct_variant_from_seq_flatten() {
assert_de_tokens_error::<Flatten>(
&[
Token::Map { len: None },
Token::Str("Map"), // variant
Token::Seq { len: Some(3) },
Token::U32(0), // a
Token::U32(42), // b
Token::U32(69), // c
Token::SeqEnd,
Token::MapEnd,
],
"invalid type: sequence, expected struct variant Enum::Map",
);
}

#[test]
fn test_struct_variant_from_seq_internally_tagged() {
assert_de_tokens_error::<InternallyTagged>(
&[
Token::Seq { len: Some(2) },
Token::Str("A"),
Token::U8(1),
Token::SeqEnd,
],
"invalid type: sequence, expected struct variant InternallyTagged::A",
);
assert_de_tokens_error::<InternallyTagged>(
&[
Token::Seq { len: Some(2) },
Token::Str("B"),
Token::I32(42),
Token::SeqEnd,
],
"invalid type: sequence, expected struct StructNotFromSeq",
);
}

#[test]
fn test_struct_variant_from_seq_enum_in_internally_tagged_enum() {
assert_de_tokens_error::<Outer>(
&[
Token::Map { len: Some(2) },
Token::Str("type"),
Token::Str("Inner"),
Token::Str("Struct"),
Token::Seq { len: Some(1) },
Token::U8(69),
Token::SeqEnd,
Token::MapEnd,
],
"invalid type: sequence, expected struct variant Inner::Struct",
);
}

#[test]
fn test_short_tuple() {
assert_de_tokens_error::<(u8, u8, u8)>(
Expand Down
47 changes: 0 additions & 47 deletions test_suite/tests/test_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,16 +714,6 @@ fn test_internally_tagged_enum() {
],
);

assert_de_tokens(
&InternallyTagged::A { a: 1 },
&[
Token::Seq { len: Some(2) },
Token::Str("A"),
Token::U8(1),
Token::SeqEnd,
],
);

assert_tokens(
&InternallyTagged::B,
&[
Expand Down Expand Up @@ -788,16 +778,6 @@ fn test_internally_tagged_enum() {
],
);

assert_de_tokens(
&InternallyTagged::E(Struct { f: 6 }),
&[
Token::Seq { len: Some(2) },
Token::Str("E"),
Token::U8(6),
Token::SeqEnd,
],
);

assert_de_tokens_error::<InternallyTagged>(
&[Token::Map { len: Some(0) }, Token::MapEnd],
"missing field `type`",
Expand Down Expand Up @@ -1013,16 +993,6 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() {
Token::MapEnd,
],
);

assert_de_tokens(
&Message::Log { level: Level::Info },
&[
Token::Seq { len: Some(2) },
Token::Str("Log"),
Token::BorrowedStr("Info"),
Token::SeqEnd,
],
);
}

#[test]
Expand Down Expand Up @@ -1524,23 +1494,6 @@ fn test_enum_in_internally_tagged_enum() {
Token::MapEnd,
],
);

// Reaches crate::private::de::content::VariantDeserializer::struct_variant
// Content::Seq case
// via ContentDeserializer::deserialize_enum
assert_de_tokens(
&Outer::Inner(Inner::Struct { f: 1 }),
&[
Token::Map { len: Some(2) },
Token::Str("type"),
Token::Str("Inner"),
Token::Str("Struct"),
Token::Seq { len: Some(1) },
Token::U8(1), // f
Token::SeqEnd,
Token::MapEnd,
],
);
}

#[test]
Expand Down