identifiers: Don't require room IDs to contain a server name

Room IDs being splittable into localpart and servername does not have
much inherent value and there are proposals like MSC4051¹ that propose
changing the format. Relaxing the rules makes Ruma forwards-compatible
with those proposals. The server_name accessor is kept because it is
used by at least one downstream, but is updated to return an `Option`.

¹ https://github.com/matrix-org/matrix-spec-proposals/pull/4051
This commit is contained in:
Jonas Platte 2023-09-28 11:38:38 +02:00
parent 00ee6030c0
commit 984cbda962
No known key found for this signature in database
GPG Key ID: AAA7A61F696C3E0C
10 changed files with 60 additions and 50 deletions

View File

@ -14,6 +14,9 @@ Breaking changes:
- Removed the `events` module, it is once again its own crate (`ruma-events`)
- Removed `From` and `TryFrom` implementations for `RedactedBecause` in favor of named constructors
(`from_json` and `from_raw_event`)
- Updated room IDs to not require a servername
- Removed `localpart` method from `RoomId` and `RoomOrAliasId`
- Changed `server_name` method on `RoomId` and `RoomOrAliasId` to return `Option<&str>`
Improvements:

View File

@ -5,6 +5,7 @@ use ruma_macros::IdZst;
use super::{
matrix_uri::UriAction, MatrixToUri, MatrixUri, OwnedEventId, OwnedServerName, ServerName,
};
use crate::RoomOrAliasId;
/// A Matrix [room ID].
///
@ -33,14 +34,9 @@ impl RoomId {
Self::from_borrowed(&format!("!{}:{server_name}", super::generate_localpart(18))).to_owned()
}
/// Returns the rooms's unique ID.
pub fn localpart(&self) -> &str {
&self.as_str()[1..self.colon_idx()]
}
/// Returns the server name of the room ID.
pub fn server_name(&self) -> &ServerName {
ServerName::from_borrowed(&self.as_str()[self.colon_idx() + 1..])
pub fn server_name(&self) -> Option<&ServerName> {
<&RoomOrAliasId>::from(self).server_name()
}
/// Create a `matrix.to` URI for this room ID.
@ -202,10 +198,6 @@ impl RoomId {
None,
)
}
fn colon_idx(&self) -> usize {
self.as_str().find(':').unwrap()
}
}
#[cfg(test)]
@ -287,23 +279,25 @@ mod tests {
}
#[test]
fn missing_room_id_delimiter() {
assert_eq!(<&RoomId>::try_from("!29fhd83h92h0").unwrap_err(), IdParseError::MissingColon);
fn missing_server_name() {
assert_eq!(
<&RoomId>::try_from("!29fhd83h92h0").expect("Failed to create RoomId."),
"!29fhd83h92h0"
);
}
#[test]
fn invalid_room_id_host() {
assert_eq!(
<&RoomId>::try_from("!29fhd83h92h0:/").unwrap_err(),
IdParseError::InvalidServerName
);
let room_id = <&RoomId>::try_from("!29fhd83h92h0:/").expect("Failed to create RoomId.");
assert_eq!(room_id, "!29fhd83h92h0:/");
assert_eq!(room_id.server_name(), None);
}
#[test]
fn invalid_room_id_port() {
assert_eq!(
<&RoomId>::try_from("!29fhd83h92h0:example.com:notaport").unwrap_err(),
IdParseError::InvalidServerName
);
let room_id = <&RoomId>::try_from("!29fhd83h92h0:example.com:notaport")
.expect("Failed to create RoomId.");
assert_eq!(room_id, "!29fhd83h92h0:example.com:notaport");
assert_eq!(room_id.server_name(), None);
}
}

View File

@ -3,6 +3,7 @@
use std::hint::unreachable_unchecked;
use ruma_macros::IdZst;
use tracing::warn;
use super::{server_name::ServerName, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId};
@ -30,14 +31,22 @@ use super::{server_name::ServerName, OwnedRoomAliasId, OwnedRoomId, RoomAliasId,
pub struct RoomOrAliasId(str);
impl RoomOrAliasId {
/// Returns the local part (everything after the `!` or `#` and before the first colon).
pub fn localpart(&self) -> &str {
&self.as_str()[1..self.colon_idx()]
}
/// Returns the server name of the room (alias) ID.
pub fn server_name(&self) -> &ServerName {
ServerName::from_borrowed(&self.as_str()[self.colon_idx() + 1..])
pub fn server_name(&self) -> Option<&ServerName> {
let colon_idx = self.as_str().find(':')?;
let server_name = &self.as_str()[colon_idx + 1..];
match server_name.try_into() {
Ok(parsed) => Some(parsed),
// Room aliases are verified to contain a server name at parse time
Err(e) => {
warn!(
target: "ruma_common::identifiers::room_id",
server_name,
"Room ID contains colon but no valid server name afterwards: {e}",
);
None
}
}
}
/// Whether this is a room id (starts with `'!'`)
@ -50,10 +59,6 @@ impl RoomOrAliasId {
self.variant() == Variant::RoomAliasId
}
fn colon_idx(&self) -> usize {
self.as_str().find(':').unwrap()
}
fn variant(&self) -> Variant {
match self.as_str().bytes().next() {
Some(b'!') => Variant::RoomId,

View File

@ -2,7 +2,7 @@ use crate::{validate_delimited_id, Error};
pub fn validate(s: &str) -> Result<(), Error> {
if s.contains(':') {
validate_delimited_id(s, &['$'])?;
validate_delimited_id(s, b'$')?;
} else if !s.starts_with('$') {
return Err(Error::MissingLeadingSigil);
}

View File

@ -22,13 +22,13 @@ pub use error::Error;
const MAX_BYTES: usize = 255;
/// Checks if an identifier is valid.
fn validate_id(id: &str, valid_sigils: &[char]) -> Result<(), Error> {
fn validate_id(id: &str, first_byte: u8) -> Result<(), Error> {
#[cfg(not(feature = "compat-arbitrary-length-ids"))]
if id.len() > MAX_BYTES {
return Err(Error::MaximumLengthExceeded);
}
if !id.starts_with(valid_sigils) {
if id.as_bytes().first() != Some(&first_byte) {
return Err(Error::MissingLeadingSigil);
}
@ -36,15 +36,15 @@ fn validate_id(id: &str, valid_sigils: &[char]) -> Result<(), Error> {
}
/// Checks an identifier that contains a localpart and hostname for validity.
fn parse_id(id: &str, valid_sigils: &[char]) -> Result<usize, Error> {
validate_id(id, valid_sigils)?;
fn parse_id(id: &str, first_byte: u8) -> Result<usize, Error> {
validate_id(id, first_byte)?;
let colon_idx = id.find(':').ok_or(Error::MissingColon)?;
server_name::validate(&id[colon_idx + 1..])?;
Ok(colon_idx)
}
/// Checks an identifier that contains a localpart and hostname for validity.
fn validate_delimited_id(id: &str, valid_sigils: &[char]) -> Result<(), Error> {
parse_id(id, valid_sigils)?;
fn validate_delimited_id(id: &str, first_byte: u8) -> Result<(), Error> {
parse_id(id, first_byte)?;
Ok(())
}

View File

@ -1,5 +1,5 @@
use crate::{validate_delimited_id, Error};
pub fn validate(s: &str) -> Result<(), Error> {
validate_delimited_id(s, &['#'])
validate_delimited_id(s, b'#')
}

View File

@ -1,5 +1,5 @@
use crate::{validate_delimited_id, Error};
use crate::{validate_id, Error};
pub fn validate(s: &str) -> Result<(), Error> {
validate_delimited_id(s, &['!'])
validate_id(s, b'!')
}

View File

@ -1,5 +1,9 @@
use crate::{validate_delimited_id, Error};
use crate::Error;
pub fn validate(s: &str) -> Result<(), Error> {
validate_delimited_id(s, &['#', '!'])
match s.as_bytes().first() {
Some(b'#') => crate::room_alias_id::validate(s),
Some(b'!') => crate::room_id::validate(s),
_ => Err(Error::MissingLeadingSigil),
}
}

View File

@ -1,7 +1,7 @@
use crate::{parse_id, Error};
pub fn validate(s: &str) -> Result<(), Error> {
let colon_idx = parse_id(s, &['@'])?;
let colon_idx = parse_id(s, b'@')?;
let localpart = &s[1..colon_idx];
let _ = localpart_is_fully_conforming(localpart)?;

View File

@ -162,14 +162,18 @@ pub fn auth_check<E: Event>(
}
// If the domain of the room_id does not match the domain of the sender, reject
if incoming_event.room_id().server_name() != sender.server_name() {
warn!("creation events server does not match sender");
return Ok(false); // creation events room id does not match senders
let Some(room_id_server_name) = incoming_event.room_id().server_name() else {
warn!("room ID has no servername");
return Ok(false);
};
if room_id_server_name != sender.server_name() {
warn!("servername of room ID does not match servername of sender");
return Ok(false);
}
let content: RoomCreateContentFields = from_json_str(incoming_event.content().get())?;
// If content.room_version is present and is not a recognized version, reject
let content: RoomCreateContentFields = from_json_str(incoming_event.content().get())?;
if content.room_version.map(|v| v.deserialize().is_err()).unwrap_or(false) {
warn!("invalid room version found in m.room.create event");
return Ok(false);