From cb857db71e43401103b53f96e81dd338ea2b997c Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Fri, 17 Apr 2020 12:28:58 +0200 Subject: [PATCH] Fix tests, some refactoring --- CHANGELOG.md | 3 ++- src/event_id.rs | 25 ++++++++++++++----------- src/room_alias_id.rs | 11 +++++------ src/room_id.rs | 20 +++++++++++--------- src/room_id_or_room_alias_id.rs | 11 +++++------ src/server_name.rs | 9 +++++++++ src/user_id.rs | 30 +++++++++++++++--------------- 7 files changed, 61 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6320241b..e9e40bc5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/event_id.rs b/src/event_id.rs index cd536fd1..40b1ef37 100644 --- a/src/event_id.rs +++ b/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 { - let full_id = format!("${}:{}", generate_localpart(18), homeserver_host); + pub fn new(server_name: &str) -> Result { + 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> for EventId { diff --git a/src/room_alias_id.rs b/src/room_alias_id.rs index 8b3445b3..6bf23924 100644 --- a/src/room_alias_id.rs +++ b/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> for RoomAliasId { diff --git a/src/room_id.rs b/src/room_id.rs index 75aa62c9..9505ac3e 100644 --- a/src/room_id.rs +++ b/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 { - let full_id = format!("!{}:{}", generate_localpart(18), homeserver_host); + pub fn new(server_name: &str) -> Result { + 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> for RoomId { diff --git a/src/room_id_or_room_alias_id.rs b/src/room_id_or_room_alias_id.rs index a83b019e..9023824c 100644 --- a/src/room_id_or_room_alias_id.rs +++ b/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 diff --git a/src/server_name.rs b/src/server_name.rs index b0b2d15e..65a75596 100644 --- a/src/server_name.rs +++ b/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]")); diff --git a/src/user_id.rs b/src/user_id.rs index 5b3fd7d2..63532ccb 100644 --- a/src/user_id.rs +++ b/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 { - let full_id = format!( - "@{}:{}", - generate_localpart(12).to_lowercase(), - homeserver_host - ); - let colon_idx = parse_id(&full_id, &['@'])?; + pub fn new(server_name: &str) -> Result { + if !is_valid_server_name(server_name) { + return Err(Error::InvalidServerName); + } + let full_id = format!("@{}:{}", generate_localpart(12).to_lowercase(), server_name); 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('@'));