From fa3f8e5ce231a5844e59f0d5e1081b5c6f8274c8 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 1 Oct 2020 16:27:48 +0200 Subject: manually implement Deserialize for Agent The rationale is included in the comment below. The gist is that we want to avoid deserializing Agent (and others) directly, as that would circumvent the checks. As a small bonus, we now skip the phantom_data field in serialization, as that's just an implementation detail that other consumers shouldn't worry about. --- src/lib.rs | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 156 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/lib.rs b/src/lib.rs index 2dff6a4..5ba6340 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -444,7 +444,7 @@ impl TryFrom<&raw::Agent> for AgentKind { /// `Kind`. An escape hatch is the method [`.erase()`][Agent::erase], which erases the kind /// information and produces the default `Agent<()>`. Functions/methods that only take `Agent<()>` /// can therefore be used by any other agent as well. -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] #[derive(Debug, Clone, Hash, PartialEq, Eq, Getters, CopyGetters)] // For the reasoning of #[repr(C)] see Agent::transmute. #[repr(C)] @@ -513,9 +513,164 @@ pub struct Agent { #[get_copy = "pub"] master_agent: Option, + #[cfg_attr(feature = "serde", serde(skip_serializing))] phantom_data: PhantomData, } +// We could derive this, however that would derive Deserialize generically for Agent, where T is +// deserializable. In particular, this would mean that you could deserialize Agent, +// Agent and Agent directly, which would not be too bad - the problem is that serde +// has no way of knowing if the serialized agent actually is a character/player/gadget agent, as +// that information only exists on the type level. This meant that you could "coerce" agents into +// the wrong type, safely, using a serialization followed by a deserialization. +// Now this doesn't actually lead to memory unsafety or other bad behaviour, but it could mean that +// the program would panic if you tried to access a non-existing field, e.g. by calling +// Agent::id() on a non-character agent. +// In order to prevent this, we manually implement Deserialize only for Agent<()>, so that the +// usual conversion functions with the proper checks have to be used. +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for Agent { + fn deserialize>(deserializer: D) -> Result { + use serde::de::{self, Deserialize, Deserializer, MapAccess, SeqAccess, Visitor}; + use std::fmt; + + #[derive(serde::Deserialize)] + #[serde(field_identifier, rename_all = "snake_case")] + enum Field { + Addr, + Kind, + Toughness, + Concentration, + Healing, + Condition, + InstanceId, + FirstAware, + LastAware, + MasterAgent, + }; + + struct AgentVisitor; + + impl<'de> Visitor<'de> for AgentVisitor { + type Value = Agent; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("struct Agent") + } + + fn visit_map>(self, mut map: V) -> Result { + let mut addr = None; + let mut kind = None; + let mut toughness = None; + let mut concentration = None; + let mut healing = None; + let mut condition = None; + let mut instance_id = None; + let mut first_aware = None; + let mut last_aware = None; + let mut master_agent = None; + + while let Some(key) = map.next_key()? { + match key { + Field::Addr => { + if addr.is_some() { + return Err(de::Error::duplicate_field("addr")); + } + addr = Some(map.next_value()?); + } + Field::Kind => { + if kind.is_some() { + return Err(de::Error::duplicate_field("kind")); + } + kind = Some(map.next_value()?); + } + Field::Toughness => { + if toughness.is_some() { + return Err(de::Error::duplicate_field("toughness")); + } + toughness = Some(map.next_value()?); + } + Field::Concentration => { + if concentration.is_some() { + return Err(de::Error::duplicate_field("concentration")); + } + concentration = Some(map.next_value()?); + } + Field::Healing => { + if healing.is_some() { + return Err(de::Error::duplicate_field("healing")); + } + healing = Some(map.next_value()?); + } + Field::Condition => { + if condition.is_some() { + return Err(de::Error::duplicate_field("condition")); + } + condition = Some(map.next_value()?); + } + Field::InstanceId=> { + if instance_id.is_some() { + return Err(de::Error::duplicate_field("instance_id")); + } + instance_id = Some(map.next_value()?); + } + Field::FirstAware=> { + if first_aware.is_some() { + return Err(de::Error::duplicate_field("first_aware")); + } + first_aware = Some(map.next_value()?); + } + Field::LastAware => { + if last_aware.is_some() { + return Err(de::Error::duplicate_field("last_aware")); + } + last_aware = Some(map.next_value()?); + } + Field::MasterAgent => { + if master_agent.is_some() { + return Err(de::Error::duplicate_field("master_agent")); + } + master_agent = Some(map.next_value()?); + } + } + } + + Ok(Agent { + addr: addr.ok_or_else(|| de::Error::missing_field("addr"))?, + kind: kind.ok_or_else(|| de::Error::missing_field("kind"))?, + toughness: toughness.ok_or_else(|| de::Error::missing_field("toughness"))?, + concentration: concentration + .ok_or_else(|| de::Error::missing_field("concentration"))?, + healing: healing.ok_or_else(|| de::Error::missing_field("healing"))?, + condition: condition.ok_or_else(|| de::Error::missing_field("condition"))?, + instance_id: instance_id + .ok_or_else(|| de::Error::missing_field("instance_id"))?, + first_aware: first_aware + .ok_or_else(|| de::Error::missing_field("first_aware"))?, + last_aware: last_aware.ok_or_else(|| de::Error::missing_field("last_aware"))?, + master_agent: master_agent + .ok_or_else(|| de::Error::missing_field("master_agent"))?, + phantom_data: PhantomData, + }) + } + }; + + const FIELDS: &'static [&'static str] = &[ + "addr", + "kind", + "toughness", + "concentration", + "healing", + "condition", + "instance_id", + "first_aware", + "last_aware", + "master_agent", + ]; + deserializer.deserialize_struct("Agent", FIELDS, AgentVisitor) + } +} + impl TryFrom<&raw::Agent> for Agent { type Error = EvtcError; -- cgit v1.2.3