Fix tests, some refactoring

CHANGELOG.md

@@ -15,7 +15,8 @@ Breaking changes:
     representation
     * Note that hashes are generally only guaranteed consistent in the lifetime of the program
       though, so do not persist them!
-  * The `hostname` methods have been updated to return string slices instead of `&url::Host`
+  * The `hostname` methods have been rename to `server_name` and updated to return string slices
+    instead of `&url::Host`
 * `Error::InvalidHost` has been renamed to `Error::InvalidServerName`, because it also covers errors
   in the port, not just the host part section of the server name
 

src/event_id.rs

@@ -5,7 +5,7 @@ use std::{borrow::Cow, convert::TryFrom, num::NonZeroU8};
 #[cfg(feature = "diesel")]
 use diesel::sql_types::Text;
 
-use crate::{error::Error, generate_localpart, parse_id, validate_id};
+use crate::{error::Error, generate_localpart, is_valid_server_name, parse_id, validate_id};
 
 /// A Matrix event ID.
 ///
@@ -55,8 +55,11 @@ impl EventId {
     ///
     /// Does not currently ever fail, but may fail in the future if the homeserver cannot be parsed
     /// parsed as a valid host.
-    pub fn new(homeserver_host: &str) -> Result<Self, Error> {
+    pub fn new(server_name: &str) -> Result<Self, Error> {
-        let full_id = format!("${}:{}", generate_localpart(18), homeserver_host);
+        if !is_valid_server_name(server_name) {
+            return Err(Error::InvalidServerName);
+        }
+        let full_id = format!("${}:{}", generate_localpart(18), server_name);
 
         Ok(Self {
             full_id,
@@ -64,14 +67,6 @@ impl EventId {
         })
     }
 
-    /// Returns the host of the event ID, containing the server name (including the port) of the
-    /// originating homeserver. Only applicable to events in the original format as used by Matrix
-    /// room versions 1 and 2.
-    pub fn hostname(&self) -> Option<&str> {
-        self.colon_idx
-            .map(|idx| &self.full_id[idx.get() as usize + 1..])
-    }
-
     /// Returns the event's unique ID. For the original event format as used by Matrix room
     /// versions 1 and 2, this is the "localpart" that precedes the homeserver. For later formats,
     /// this is the entire ID without the leading $ sigil.
@@ -83,6 +78,14 @@ impl EventId {
 
         &self.full_id[1..idx]
     }
+
+    /// Returns the server name of the event ID.
+    ///
+    /// Only applicable to events in the original format as used by Matrix room versions 1 and 2.
+    pub fn server_name(&self) -> Option<&str> {
+        self.colon_idx
+            .map(|idx| &self.full_id[idx.get() as usize + 1..])
+    }
 }
 
 impl TryFrom<Cow<'_, str>> for EventId {

src/room_alias_id.rs

@@ -29,16 +29,15 @@ pub struct RoomAliasId {
 }
 
 impl RoomAliasId {
-    /// Returns the host of the room alias ID, containing the server name (including the port) of
-    /// the originating homeserver.
-    pub fn hostname(&self) -> &str {
-        &self.full_id[self.colon_idx.get() as usize + 1..]
-    }
-
     /// Returns the room's alias.
     pub fn alias(&self) -> &str {
         &self.full_id[1..self.colon_idx.get() as usize]
     }
+
+    /// Returns the server name of the room alias ID.
+    pub fn server_name(&self) -> &str {
+        &self.full_id[self.colon_idx.get() as usize + 1..]
+    }
 }
 
 impl TryFrom<Cow<'_, str>> for RoomAliasId {

src/room_id.rs

@@ -5,7 +5,7 @@ use std::{borrow::Cow, convert::TryFrom, num::NonZeroU8};
 #[cfg(feature = "diesel")]
 use diesel::sql_types::Text;
 
-use crate::{error::Error, generate_localpart, parse_id};
+use crate::{error::Error, generate_localpart, is_valid_server_name, parse_id};
 
 /// A Matrix room ID.
 ///
@@ -33,8 +33,11 @@ impl RoomId {
     /// 18 random ASCII characters.
     ///
     /// Fails if the given homeserver cannot be parsed as a valid host.
-    pub fn new(homeserver_host: &str) -> Result<Self, Error> {
+    pub fn new(server_name: &str) -> Result<Self, Error> {
-        let full_id = format!("!{}:{}", generate_localpart(18), homeserver_host);
+        if !is_valid_server_name(server_name) {
+            return Err(Error::InvalidServerName);
+        }
+        let full_id = format!("!{}:{}", generate_localpart(18), server_name);
 
         Ok(Self {
             full_id,
@@ -42,16 +45,15 @@ impl RoomId {
         })
     }
 
-    /// Returns the host of the room ID, containing the server name (including the port) of the
-    /// originating homeserver.
-    pub fn hostname(&self) -> &str {
-        &self.full_id[self.colon_idx.get() as usize + 1..]
-    }
-
     /// Returns the rooms's unique ID.
     pub fn localpart(&self) -> &str {
         &self.full_id[1..self.colon_idx.get() as usize]
     }
+
+    /// Returns the server name of the room ID.
+    pub fn server_name(&self) -> &str {
+        &self.full_id[self.colon_idx.get() as usize + 1..]
+    }
 }
 
 impl TryFrom<Cow<'_, str>> for RoomId {

src/room_id_or_room_alias_id.rs

@@ -35,17 +35,16 @@ pub struct RoomIdOrAliasId {
 }
 
 impl RoomIdOrAliasId {
-    /// Returns the host of the room (alias) ID, containing the server name (including the port) of
-    /// the originating homeserver.
-    pub fn hostname(&self) -> &str {
-        &self.full_id[self.colon_idx.get() as usize + 1..]
-    }
-
     /// Returns the local part (everything after the `!` or `#` and before the first colon).
     pub fn localpart(&self) -> &str {
         &self.full_id[1..self.colon_idx.get() as usize]
     }
 
+    /// Returns the server name of the room (alias) ID.
+    pub fn server_name(&self) -> &str {
+        &self.full_id[self.colon_idx.get() as usize + 1..]
+    }
+
     /// Whether this is a room id (starts with `'!'`)
     pub fn is_room_id(&self) -> bool {
         self.variant() == Variant::RoomId

src/server_name.rs

@@ -4,6 +4,10 @@
 pub fn is_valid_server_name(name: &str) -> bool {
     use std::net::Ipv6Addr;
 
+    if name.is_empty() {
+        return false;
+    }
+
     let end_of_host = if name.starts_with('[') {
         let end_of_ipv6 = match name.find(']') {
             Some(idx) => idx,
@@ -73,6 +77,11 @@ mod tests {
         assert!(is_valid_server_name("ruma.io:8080"));
     }
 
+    #[test]
+    fn empty_string() {
+        assert!(!is_valid_server_name(""));
+    }
+
     #[test]
     fn invalid_ipv6() {
         assert!(!is_valid_server_name("[test::1]"));

src/user_id.rs

@@ -5,7 +5,7 @@ use std::{borrow::Cow, convert::TryFrom, num::NonZeroU8};
 #[cfg(feature = "diesel")]
 use diesel::sql_types::Text;
 
-use crate::{error::Error, generate_localpart, parse_id};
+use crate::{error::Error, generate_localpart, is_valid_server_name, parse_id};
 
 /// A Matrix user ID.
 ///
@@ -39,32 +39,29 @@ impl UserId {
     /// 12 random ASCII characters.
     ///
     /// Fails if the given homeserver cannot be parsed as a valid host.
-    pub fn new(homeserver_host: &str) -> Result<Self, Error> {
+    pub fn new(server_name: &str) -> Result<Self, Error> {
-        let full_id = format!(
+        if !is_valid_server_name(server_name) {
-            "@{}:{}",
+            return Err(Error::InvalidServerName);
-            generate_localpart(12).to_lowercase(),
+        }
-            homeserver_host
+        let full_id = format!("@{}:{}", generate_localpart(12).to_lowercase(), server_name);
-        );
-        let colon_idx = parse_id(&full_id, &['@'])?;
 
         Ok(Self {
             full_id,
-            colon_idx,
+            colon_idx: NonZeroU8::new(13).unwrap(),
             is_historical: false,
         })
     }
 
-    /// Returns the host of the user ID, containing the server name (including the port) of the
-    /// originating homeserver.
-    pub fn hostname(&self) -> &str {
-        &self.full_id[self.colon_idx.get() as usize + 1..]
-    }
-
     /// Returns the user's localpart.
     pub fn localpart(&self) -> &str {
         &self.full_id[1..self.colon_idx.get() as usize]
     }
 
+    /// Returns the server name of the user ID.
+    pub fn server_name(&self) -> &str {
+        &self.full_id[self.colon_idx.get() as usize + 1..]
+    }
+
     /// Whether this user ID is a historical one, i.e. one that doesn't conform to the latest
     /// specification of the user ID grammar but is still accepted because it was previously
     /// allowed.
@@ -140,6 +137,9 @@ mod tests {
     #[test]
     fn generate_random_valid_user_id() {
         let user_id = UserId::new("example.com").expect("Failed to generate UserId.");
+        assert_eq!(user_id.localpart().len(), 12);
+        assert_eq!(user_id.server_name(), "example.com");
+
         let id_str: &str = user_id.as_ref();
 
         assert!(id_str.starts_with('@'));