From 8b6d2ee26000717c399bfe42829876166abcd837 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= <jorgem@element.io>
Date: Fri, 8 Nov 2024 09:16:32 +0100
Subject: [PATCH] refactor(ffi): Improve `is_room_alias_format_valid` so it's
 more strict.

Previously this only used the Ruma checks, which only handled the initial `#` char and the domain part. With these changes, the name part is also validated, checking it's lowercase, with no whitespaces and containing only allowed chars, similar to what `DisplayName::to_room_alias_name` does.
---
 bindings/matrix-sdk-ffi/src/room_alias.rs | 81 ++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/bindings/matrix-sdk-ffi/src/room_alias.rs b/bindings/matrix-sdk-ffi/src/room_alias.rs
index 491820c5799..6c001c64747 100644
--- a/bindings/matrix-sdk-ffi/src/room_alias.rs
+++ b/bindings/matrix-sdk-ffi/src/room_alias.rs
@@ -1,10 +1,32 @@
 use matrix_sdk::RoomDisplayName;
 use ruma::RoomAliasId;
 
-/// Verifies the passed `String` matches the expected room alias format.
+const INVALID_ROOM_ALIAS_NAME_CHARS: &str = "#,:";
+
+/// Verifies the passed `String` matches the expected room alias format:
+///
+/// This means it's lowercase, with no whitespace chars, has a single leading
+/// `#` char and a single `:` separator between the name and domain parts, and
+/// the name part only contains characters that can't be percent encoded.
 #[matrix_sdk_ffi_macros::export]
 fn is_room_alias_format_valid(alias: String) -> bool {
-    RoomAliasId::parse(alias).is_ok()
+    let alias_parts: Vec<&str> = alias.split(':').collect();
+    if alias_parts.len() != 2 {
+        return false;
+    }
+
+    let name_part = alias_parts[0];
+    let has_valid_format = name_part.chars().skip(1).all(|c| {
+        c.is_ascii()
+            && !c.is_whitespace()
+            && !c.is_control()
+            && !INVALID_ROOM_ALIAS_NAME_CHARS.contains(c)
+    });
+
+    let is_lowercase = alias.to_lowercase() == alias;
+
+    // Checks both the name part and the server part
+    has_valid_format && is_lowercase && RoomAliasId::parse(alias).is_ok()
 }
 
 /// Transforms a Room's display name into a valid room alias name.
@@ -12,3 +34,58 @@ fn is_room_alias_format_valid(alias: String) -> bool {
 fn room_alias_name_from_room_display_name(room_name: String) -> String {
     RoomDisplayName::Named(room_name).to_room_alias_name()
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::room_alias::is_room_alias_format_valid;
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_it_has_no_leading_hash_char_is_not_valid() {
+        assert!(!is_room_alias_format_valid("alias:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_it_has_several_colon_chars_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias:something:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_it_has_no_colon_chars_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias.domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_server_part_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias:".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_name_part_has_whitespace_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias with whitespace:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_name_part_has_control_char_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias\u{0009}:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_name_part_has_invalid_char_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias,test:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_name_part_is_not_lowercase_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#Alias:domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_server_part_is_not_lowercase_is_not_valid() {
+        assert!(!is_room_alias_format_valid("#alias:Domain.org".to_owned()))
+    }
+
+    #[test]
+    fn test_is_room_alias_format_valid_when_has_valid_format() {
+        assert!(is_room_alias_format_valid("#alias.test:domain.org".to_owned()))
+    }
+}