-
Notifications
You must be signed in to change notification settings - Fork 7
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.
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
serde_test
: Don't forward (almost) everything to deserialize_any
#2
Comments
@dtolnay Is this something you'd be interested in? I can try to work on it if you are. This issue has caused me to miss multiple bugs in my code, and I'm sure I'm not the only one. |
Yeah I am open to this. I would want to better understand what currently passing (reasonable, correct) tests this would break though, but maybe that would be clearer from seeing the implementation of the change. |
I attempted to simply error on the fallback to diff --git a/serde_test/src/de.rs b/serde_test/src/de.rs
index 673a0c0e..dce6206c 100644
--- a/serde_test/src/de.rs
+++ b/serde_test/src/de.rs
@@ -278,7 +278,10 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
{
visitor.visit_enum(DeserializerEnumVisitor { de: self })
}
- _ => self.deserialize_any(visitor),
+ token => Err(de::Error::invalid_type(
+ token.into_unexpected(),
+ &format!("enum {}", name).as_str(),
+ )),
}
}
@@ -291,7 +294,10 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
assert_next_token!(self, Token::UnitStruct { name: name });
visitor.visit_unit()
}
- _ => self.deserialize_any(visitor),
+ token => Err(de::Error::invalid_type(
+ token.into_unexpected(),
+ &format!("unit struct {}", name).as_str(),
+ )),
}
}
@@ -333,7 +339,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
self.next_token();
self.visit_seq(Some(len), Token::TupleStructEnd, visitor)
}
- _ => self.deserialize_any(visitor),
+ token => Err(de::Error::invalid_type(token.into_unexpected(), &"tuple")),
}
}
@@ -367,7 +373,10 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
assert_next_token!(self, Token::TupleStruct { name: name, len: n });
self.visit_seq(Some(len), Token::TupleStructEnd, visitor)
}
- _ => self.deserialize_any(visitor),
+ token => Err(de::Error::invalid_type(
+ token.into_unexpected(),
+ &"tuple struct",
+ )),
}
}
@@ -389,7 +398,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
self.next_token();
self.visit_map(Some(fields.len()), Token::MapEnd, visitor)
}
- _ => self.deserialize_any(visitor),
+ token => Err(de::Error::invalid_type(token.into_unexpected(), &"struct")),
}
}
diff --git a/serde_test/src/token.rs b/serde_test/src/token.rs
index 22513614..fb5bbf0d 100644
--- a/serde_test/src/token.rs
+++ b/serde_test/src/token.rs
@@ -1,5 +1,7 @@
use std::fmt::{self, Debug, Display};
+use serde::de::Unexpected;
+
#[derive(Copy, Clone, PartialEq, Debug)]
pub enum Token {
/// A serialized `bool`.
@@ -517,3 +519,44 @@ impl Display for Token {
Debug::fmt(self, formatter)
}
}
+
+impl Token {
+ pub(crate) fn into_unexpected(self) -> Unexpected<'static> {
+ match self {
+ Token::Bool(val) => Unexpected::Bool(val),
+ Token::I8(val) => Unexpected::Signed(val as i64),
+ Token::I16(val) => Unexpected::Signed(val as i64),
+ Token::I32(val) => Unexpected::Signed(val as i64),
+ Token::I64(val) => Unexpected::Signed(val),
+ Token::U8(val) => Unexpected::Unsigned(val as u64),
+ Token::U16(val) => Unexpected::Unsigned(val as u64),
+ Token::U32(val) => Unexpected::Unsigned(val as u64),
+ Token::U64(val) => Unexpected::Unsigned(val),
+ Token::F32(val) => Unexpected::Float(val as f64),
+ Token::F64(val) => Unexpected::Float(val),
+ Token::Char(val) => Unexpected::Char(val),
+ Token::Str(val) | Token::BorrowedStr(val) | Token::String(val) => Unexpected::Str(val),
+ Token::Bytes(val) | Token::BorrowedBytes(val) | Token::ByteBuf(val) => {
+ Unexpected::Bytes(val)
+ }
+ Token::None | Token::Some => Unexpected::Option,
+ Token::Unit | Token::UnitStruct { .. } => Unexpected::Unit,
+ Token::UnitVariant { .. } => Unexpected::UnitVariant,
+ Token::NewtypeStruct { .. } => Unexpected::NewtypeStruct,
+ Token::NewtypeVariant { .. } => Unexpected::NewtypeVariant,
+ Token::Seq { .. }
+ | Token::Struct { .. }
+ | Token::Tuple { .. }
+ | Token::TupleStruct { .. } => Unexpected::Seq,
+ Token::SeqEnd | Token::TupleEnd | Token::TupleStructEnd => todo!(),
+ Token::TupleVariant { .. } => Unexpected::TupleVariant,
+ Token::TupleVariantEnd => todo!(),
+ Token::Map { .. } => Unexpected::Map,
+ Token::MapEnd => todo!(),
+ Token::StructEnd => todo!(),
+ Token::StructVariant { .. } => Unexpected::StructVariant,
+ Token::StructVariantEnd => todo!(),
+ Token::Enum { .. } => Unexpected::Enum,
+ }
+ }
+} This is obviously not complete given the presence of Even if the |
By forwarding things like
deserialize_bool
todeserialize_any
, thedeserialize_bool
behavior can then deserialize a string, for example. This is unexpected and lets through a ton of stuff that, in my opinion, shouldn't be.serde_test
is for testing, not general purpose use, so it should remain strict in what it accepts.deserialize_bool
should reject everything other thantrue
orfalse
, unless there's some reason I'm missing?The text was updated successfully, but these errors were encountered: