From 60bf821c630f1512607c49dcd6af45d0e2945a49 Mon Sep 17 00:00:00 2001 From: Link Mauve Date: Thu, 25 Dec 2025 02:42:00 +0100 Subject: [PATCH] jid: Fix JID parts serde deserialization serde was bypassing the validation these types rely on, making them completely unsound in an XMPP context. Fixes #165. --- jid/CHANGELOG.md | 3 ++ jid/src/parts.rs | 132 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/jid/CHANGELOG.md b/jid/CHANGELOG.md index 07028f8e0b964022b30e05b4acfa94a5c55a18ce..f07f75683321e60d95b2d6aca1e63dc4ba27a26c 100644 --- a/jid/CHANGELOG.md +++ b/jid/CHANGELOG.md @@ -2,6 +2,9 @@ Version NEXT: Version 0.12.1, release 2025-11-02: * Changes: + - Make serde deserialization actually validate the `NodePart`, `DomainPart` + and `ResourcePart`, it was previously completely unsound, creating these + types without validating their invariants. - The 'quote' feature now uses `jid::Jid` instead of `::jid::Jid` to stop requiring importing the module as a dependency of the project. The `jid` module just needs to be made available, for example: `use diff --git a/jid/src/parts.rs b/jid/src/parts.rs index 4ae513e2ebf9aa3d5952d5102f655255abc82f59..7fb85c465995998da69ee9a60a6f8522499ae3ca 100644 --- a/jid/src/parts.rs +++ b/jid/src/parts.rs @@ -44,6 +44,39 @@ macro_rules! def_part_into_inner_doc { }; } +#[derive(Deserialize)] +struct NodeDeserializer<'a>(&'a str); + +impl TryFrom> for NodePart { + type Error = Error; + + fn try_from(deserializer: NodeDeserializer) -> Result { + Ok(NodePart::new(deserializer.0)?.into_owned()) + } +} + +#[derive(Deserialize)] +struct DomainDeserializer<'a>(&'a str); + +impl TryFrom> for DomainPart { + type Error = Error; + + fn try_from(deserializer: DomainDeserializer) -> Result { + Ok(DomainPart::new(deserializer.0)?.into_owned()) + } +} + +#[derive(Deserialize)] +struct ResourceDeserializer<'a>(&'a str); + +impl TryFrom> for ResourcePart { + type Error = Error; + + fn try_from(deserializer: ResourceDeserializer) -> Result { + Ok(ResourcePart::new(deserializer.0)?.into_owned()) + } +} + macro_rules! def_part_types { ( $(#[$mainmeta:meta])* @@ -203,6 +236,7 @@ def_part_types! { /// /// The corresponding slice type is [`NodeRef`]. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] + #[cfg_attr(feature = "serde", serde(try_from = "NodeDeserializer"))] pub struct NodePart(String) use node_check(); /// `str`-like type which conforms to the requirements of [`NodePart`]. @@ -217,6 +251,7 @@ def_part_types! { /// (optional) `/` in any [`Jid`][crate::Jid], whether /// [`BareJid`][crate::BareJid] or [`FullJid`][crate::FullJid]. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] + #[cfg_attr(feature = "serde", serde(try_from = "DomainDeserializer"))] pub struct DomainPart(String) use domain_check(); /// `str`-like type which conforms to the requirements of [`DomainPart`]. @@ -230,6 +265,7 @@ def_part_types! { /// The [`ResourcePart`] is the optional part after the `/` in a /// [`Jid`][crate::Jid]. It is mandatory in [`FullJid`][crate::FullJid]. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] + #[cfg_attr(feature = "serde", serde(try_from = "ResourceDeserializer"))] pub struct ResourcePart(String) use resource_check(); /// `str`-like type which conforms to the requirements of @@ -292,4 +328,100 @@ mod tests { assert_eq!(n1, n3); assert_ne!(n1, n2); } + + #[cfg(feature = "serde")] + #[test] + fn nodepart_serde() { + serde_test::assert_de_tokens( + &NodePart(String::from("test")), + &[ + serde_test::Token::TupleStruct { + name: "NodePart", + len: 1, + }, + serde_test::Token::BorrowedStr("test"), + serde_test::Token::TupleStructEnd, + ], + ); + + serde_test::assert_de_tokens_error::( + &[ + serde_test::Token::TupleStruct { + name: "NodePart", + len: 1, + }, + serde_test::Token::BorrowedStr("invalid@domain"), + serde_test::Token::TupleStructEnd, + ], + "localpart doesn’t pass nodeprep validation", + ); + } + + #[cfg(feature = "serde")] + #[test] + fn domainpart_serde() { + serde_test::assert_de_tokens( + &DomainPart(String::from("[::1]")), + &[ + serde_test::Token::TupleStruct { + name: "DomainPart", + len: 1, + }, + serde_test::Token::BorrowedStr("[::1]"), + serde_test::Token::TupleStructEnd, + ], + ); + + serde_test::assert_de_tokens( + &DomainPart(String::from("domain.example")), + &[ + serde_test::Token::TupleStruct { + name: "DomainPart", + len: 1, + }, + serde_test::Token::BorrowedStr("domain.example"), + serde_test::Token::TupleStructEnd, + ], + ); + + serde_test::assert_de_tokens_error::( + &[ + serde_test::Token::TupleStruct { + name: "DomainPart", + len: 1, + }, + serde_test::Token::BorrowedStr("invalid@domain"), + serde_test::Token::TupleStructEnd, + ], + "domain doesn’t pass idna validation", + ); + } + + #[cfg(feature = "serde")] + #[test] + fn resourcepart_serde() { + serde_test::assert_de_tokens( + &ResourcePart(String::from("test")), + &[ + serde_test::Token::TupleStruct { + name: "ResourcePart", + len: 1, + }, + serde_test::Token::BorrowedStr("test"), + serde_test::Token::TupleStructEnd, + ], + ); + + serde_test::assert_de_tokens_error::( + &[ + serde_test::Token::TupleStruct { + name: "ResourcePart", + len: 1, + }, + serde_test::Token::BorrowedStr("🤖"), + serde_test::Token::TupleStructEnd, + ], + "resource doesn’t pass resourceprep validation", + ); + } }