From 824cefb3e903620afe649af1b000f0d96066b26d Mon Sep 17 00:00:00 2001
From: gnieto <gnieto.talo@gmail.com>
Date: Mon, 3 Jan 2022 15:58:38 +0100
Subject: [PATCH] Properly validate localpart when parsing UserId

When a `UserId` was created via `parse_with_server_name` (and its
variants), the localpart was not validated. This lead to invalid user
ids being constructed and lead to a crash when `is_historical` was
called.

This changes will prevent that this invalid user is can be constructed.
---
 crates/ruma-identifiers/CHANGELOG.md   |  7 +++++++
 crates/ruma-identifiers/src/user_id.rs | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/crates/ruma-identifiers/CHANGELOG.md b/crates/ruma-identifiers/CHANGELOG.md
index 0168c01b..6fced00e 100644
--- a/crates/ruma-identifiers/CHANGELOG.md
+++ b/crates/ruma-identifiers/CHANGELOG.md
@@ -12,6 +12,13 @@ Breaking changes:
 * Rename `RoomVersionId::Version{X}` variants to `RoomVersionId::V{X}`
 * Rename `RoomIdOrAliasId` to `RoomOrAliasId`
 
+Bug fixes:
+
+* Properly validate localpart when building a `UserId` via `parse_with_server_name` (and its
+  variants). This function was accepting localparts which were violating the
+  `localpart_is_fully_conforming` validation. If an invalid user was built, it would panic when
+  calling `is_historical`.
+
 # 0.20.0
 
 Breaking changes:
diff --git a/crates/ruma-identifiers/src/user_id.rs b/crates/ruma-identifiers/src/user_id.rs
index d6dc7974..5c422e73 100644
--- a/crates/ruma-identifiers/src/user_id.rs
+++ b/crates/ruma-identifiers/src/user_id.rs
@@ -49,6 +49,7 @@ impl UserId {
         if id_str.starts_with('@') {
             Self::parse(id)
         } else {
+            let _ = localpart_is_fully_conforming(id_str)?;
             Ok(Self::from_owned(format!("@{}:{}", id_str, server_name).into()))
         }
     }
@@ -65,6 +66,7 @@ impl UserId {
         if id_str.starts_with('@') {
             Self::parse_rc(id)
         } else {
+            let _ = localpart_is_fully_conforming(id_str)?;
             Ok(Self::from_rc(format!("@{}:{}", id_str, server_name).into()))
         }
     }
@@ -81,6 +83,7 @@ impl UserId {
         if id_str.starts_with('@') {
             Self::parse_arc(id)
         } else {
+            let _ = localpart_is_fully_conforming(id_str)?;
             Ok(Self::from_arc(format!("@{}:{}", id_str, server_name).into()))
         }
     }
@@ -164,6 +167,23 @@ mod tests {
         assert!(!user_id.is_historical());
     }
 
+    #[test]
+    fn invalid_user_id() {
+        let localpart = "τ";
+        let user_id = "@τ:example.com";
+        let server_name = server_name!("example.com");
+
+        assert!(<&UserId>::try_from(user_id).is_err());
+        assert!(UserId::parse_with_server_name(user_id, server_name).is_err());
+        assert!(UserId::parse_with_server_name(localpart, server_name).is_err());
+        assert!(UserId::parse_with_server_name_rc(user_id, server_name).is_err());
+        assert!(UserId::parse_with_server_name_rc(localpart, server_name).is_err());
+        assert!(UserId::parse_with_server_name_arc(user_id, server_name).is_err());
+        assert!(UserId::parse_with_server_name_arc(localpart, server_name).is_err());
+        assert!(UserId::parse_rc(user_id).is_err());
+        assert!(UserId::parse_arc(user_id).is_err());
+    }
+
     #[test]
     fn valid_historical_user_id() {
         let user_id =