From 01156ad661404cb222c355b516e740bc2db7c9b8 Mon Sep 17 00:00:00 2001 From: Florian Jacob Date: Tue, 5 Jun 2018 00:43:53 +0200 Subject: [PATCH] Properly handle CanonicalAliasEvent and NameEvent content being absent, null or empty, which is allowed by the spec to show those events were deleted: https://matrix.org/docs/spec/client_server/r0.4.0.html#m-room-canonical-alias https://matrix.org/docs/spec/client_server/r0.4.0.html#m-room-canonical-alias --- src/lib.rs | 27 +++++++++++++++- src/room/canonical_alias.rs | 61 ++++++++++++++++++++++++++++++++++++- src/room/name.rs | 51 ++++++++++++++++++++++++++++++- src/stripped.rs | 2 +- 4 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 07f832d1..d1d53936 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,7 +104,7 @@ use std::fmt::{Debug, Display, Error as FmtError, Formatter, Result as FmtResult use ruma_identifiers::{EventId, RoomId, UserId}; use serde::{ - de::{Error as SerdeError, Visitor}, + de::{Error as SerdeError, IntoDeserializer, Visitor}, Deserialize, Deserializer, Serialize, Serializer, }; use serde_json::Value; @@ -346,6 +346,31 @@ impl<'de> Deserialize<'de> for EventType { } } +/// Serde deserialization decorator to map empty Strings to None, +/// and forward non-empty Strings to the Deserialize implementation for T. +/// Useful for the typical +/// "A room with an X event with an absent, null, or empty Y field +/// should be treated the same as a room with no such event." +/// formulation in the spec. +/// +/// To be used like this: +/// `#[serde(deserialize_with = "empty_string_as_none"]` +/// Relevant serde issue: https://github.com/serde-rs/serde/issues/1425 +fn empty_string_as_none<'de, D, T>(de: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, + T: serde::Deserialize<'de>, +{ + let opt = Option::::deserialize(de)?; + let opt = opt.as_ref().map(String::as_str); + match opt { + None | Some("") => Ok(None), + // If T = String, like in m.room.name, the second deserialize is actually superfluous. + // TODO: optimize that somehow? + Some(s) => T::deserialize(s.into_deserializer()).map(Some), + } +} + #[cfg(test)] mod tests { use serde_json::{from_str, to_string}; diff --git a/src/room/canonical_alias.rs b/src/room/canonical_alias.rs index bbd6d689..c8b4c6bc 100644 --- a/src/room/canonical_alias.rs +++ b/src/room/canonical_alias.rs @@ -1,5 +1,6 @@ //! Types for the *m.room.canonical_alias* event. +use crate::empty_string_as_none; use ruma_identifiers::RoomAliasId; use serde::{Deserialize, Serialize}; @@ -12,5 +13,63 @@ state_event! { #[derive(Clone, Debug, Deserialize, Serialize)] pub struct CanonicalAliasEventContent { /// The canonical alias. - pub alias: RoomAliasId, + /// Rooms with `alias: None` should be treated the same as a room with no canonical alias. + // The spec says “A room with an m.room.canonical_alias event with an absent, null, or empty alias field + // should be treated the same as a room with no m.room.canonical_alias event.”. + // Serde maps null fields to None by default, serde(default) maps an absent field to None, + // and empty_string_as_none does exactly that, preventing empty strings getting parsed as RoomAliasId. + #[serde(default)] + #[serde(deserialize_with = "empty_string_as_none")] + pub alias: Option, +} + +#[cfg(test)] +mod tests { + use serde_json::from_str; + + use super::CanonicalAliasEventContent; + use ruma_identifiers::RoomAliasId; + use std::convert::TryFrom; + + #[test] + fn absent_field_as_none() { + assert_eq!( + from_str::(r#"{}"#) + .unwrap() + .alias, + None + ); + } + + #[test] + fn null_field_as_none() { + assert_eq!( + from_str::(r#"{"alias":null}"#) + .unwrap() + .alias, + None + ); + } + + #[test] + fn empty_field_as_none() { + assert_eq!( + from_str::(r#"{"alias":""}"#) + .unwrap() + .alias, + None + ); + } + + #[test] + fn nonempty_field_as_some() { + let alias = Some(RoomAliasId::try_from("#somewhere:localhost").unwrap()); + + assert_eq!( + from_str::(r##"{"alias":"#somewhere:localhost"}"##) + .unwrap() + .alias, + alias + ); + } } diff --git a/src/room/name.rs b/src/room/name.rs index 629a89aa..ce34b734 100644 --- a/src/room/name.rs +++ b/src/room/name.rs @@ -1,5 +1,6 @@ //! Types for the *m.room.name* event. +use crate::empty_string_as_none; use serde::{Deserialize, Serialize}; state_event! { @@ -11,5 +12,53 @@ state_event! { #[derive(Clone, Debug, Deserialize, Serialize)] pub struct NameEventContent { /// The name of the room. This MUST NOT exceed 255 bytes. - pub name: String, + /// Rooms with `name: None` should be treated the same as a room with no name. + // The spec says “A room with an m.room.name event with an absent, null, or empty name field + // should be treated the same as a room with no m.room.name event.”. + // Serde maps null fields to None by default, serde(default) maps an absent field to None, + // and empty_string_as_none completes the handling. + #[serde(default)] + #[serde(deserialize_with = "empty_string_as_none")] + pub name: Option, +} + +#[cfg(test)] +mod tests { + use super::NameEventContent; + use serde_json::from_str; + + #[test] + fn absent_field_as_none() { + assert_eq!(from_str::(r#"{}"#).unwrap().name, None); + } + + #[test] + fn null_field_as_none() { + assert_eq!( + from_str::(r#"{"name":null}"#) + .unwrap() + .name, + None + ); + } + + #[test] + fn empty_field_as_none() { + assert_eq!( + from_str::(r#"{"name":""}"#).unwrap().name, + None + ); + } + + #[test] + fn nonempty_field_as_some() { + let name = Some("The room name".to_string()); + + assert_eq!( + from_str::(r##"{"name":"The room name"}"##) + .unwrap() + .name, + name + ); + } } diff --git a/src/stripped.rs b/src/stripped.rs index c8b8a1f6..f640a01e 100644 --- a/src/stripped.rs +++ b/src/stripped.rs @@ -333,7 +333,7 @@ mod tests { match from_str::(name_event).unwrap() { StrippedState::RoomName(event) => { - assert_eq!(event.content.name, "Ruma"); + assert_eq!(event.content.name, Some("Ruma".to_string())); assert_eq!(event.event_type, EventType::RoomName); assert_eq!(event.state_key, ""); assert_eq!(event.sender.to_string(), "@example:localhost");