Skip to content

Commit 7c600fd

Browse files
committed
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. Moved the code to the SDK crate so it can be properly tested.
1 parent 965a59d commit 7c600fd

File tree

2 files changed

+86
-3
lines changed

2 files changed

+86
-3
lines changed

bindings/matrix-sdk-ffi/src/room_alias.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
use matrix_sdk::RoomDisplayName;
2-
use ruma::RoomAliasId;
32

4-
/// Verifies the passed `String` matches the expected room alias format.
3+
/// Verifies the passed `String` matches the expected room alias format:
4+
///
5+
/// This means it's lowercase, with no whitespace chars, has a single leading
6+
/// `#` char and a single `:` separator between the local and domain parts, and
7+
/// the local part only contains characters that can't be percent encoded.
58
#[matrix_sdk_ffi_macros::export]
69
fn is_room_alias_format_valid(alias: String) -> bool {
7-
RoomAliasId::parse(alias).is_ok()
10+
matrix_sdk::utils::is_room_alias_format_valid(alias)
811
}
912

1013
/// Transforms a Room's display name into a valid room alias name.

crates/matrix-sdk/src/utils.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use futures_util::StreamExt;
2424
use ruma::{
2525
events::{AnyMessageLikeEventContent, AnyStateEventContent},
2626
serde::Raw,
27+
RoomAliasId,
2728
};
2829
use serde_json::value::{RawValue as RawJsonValue, Value as JsonValue};
2930
#[cfg(feature = "e2e-encryption")]
@@ -190,8 +191,37 @@ impl IntoRawStateEventContent for &Box<RawJsonValue> {
190191
}
191192
}
192193

194+
const INVALID_ROOM_ALIAS_NAME_CHARS: &str = "#,:";
195+
196+
/// Verifies the passed `String` matches the expected room alias format:
197+
///
198+
/// This means it's lowercase, with no whitespace chars, has a single leading
199+
/// `#` char and a single `:` separator between the local and domain parts, and
200+
/// the local part only contains characters that can't be percent encoded.
201+
pub fn is_room_alias_format_valid(alias: String) -> bool {
202+
let alias_parts: Vec<&str> = alias.split(':').collect();
203+
if alias_parts.len() != 2 {
204+
return false;
205+
}
206+
207+
let local_part = alias_parts[0];
208+
let has_valid_format = local_part.chars().skip(1).all(|c| {
209+
c.is_ascii()
210+
&& !c.is_whitespace()
211+
&& !c.is_control()
212+
&& !INVALID_ROOM_ALIAS_NAME_CHARS.contains(c)
213+
});
214+
215+
let is_lowercase = alias.to_lowercase() == alias;
216+
217+
// Checks both the local part and the domain part
218+
has_valid_format && is_lowercase && RoomAliasId::parse(alias).is_ok()
219+
}
220+
193221
#[cfg(test)]
194222
mod test {
223+
use crate::utils::is_room_alias_format_valid;
224+
195225
#[cfg(feature = "e2e-encryption")]
196226
#[test]
197227
fn test_channel_observable_get_set() {
@@ -202,4 +232,54 @@ mod test {
202232
assert_eq!(observable.set(10), 1);
203233
assert_eq!(observable.get(), 10);
204234
}
235+
236+
#[test]
237+
fn test_is_room_alias_format_valid_when_it_has_no_leading_hash_char_is_not_valid() {
238+
assert!(!is_room_alias_format_valid("alias:domain.org".to_owned()))
239+
}
240+
241+
#[test]
242+
fn test_is_room_alias_format_valid_when_it_has_several_colon_chars_is_not_valid() {
243+
assert!(!is_room_alias_format_valid("#alias:something:domain.org".to_owned()))
244+
}
245+
246+
#[test]
247+
fn test_is_room_alias_format_valid_when_it_has_no_colon_chars_is_not_valid() {
248+
assert!(!is_room_alias_format_valid("#alias.domain.org".to_owned()))
249+
}
250+
251+
#[test]
252+
fn test_is_room_alias_format_valid_when_server_part_is_not_valid() {
253+
assert!(!is_room_alias_format_valid("#alias:".to_owned()))
254+
}
255+
256+
#[test]
257+
fn test_is_room_alias_format_valid_when_name_part_has_whitespace_is_not_valid() {
258+
assert!(!is_room_alias_format_valid("#alias with whitespace:domain.org".to_owned()))
259+
}
260+
261+
#[test]
262+
fn test_is_room_alias_format_valid_when_name_part_has_control_char_is_not_valid() {
263+
assert!(!is_room_alias_format_valid("#alias\u{0009}:domain.org".to_owned()))
264+
}
265+
266+
#[test]
267+
fn test_is_room_alias_format_valid_when_name_part_has_invalid_char_is_not_valid() {
268+
assert!(!is_room_alias_format_valid("#alias,test:domain.org".to_owned()))
269+
}
270+
271+
#[test]
272+
fn test_is_room_alias_format_valid_when_name_part_is_not_lowercase_is_not_valid() {
273+
assert!(!is_room_alias_format_valid("#Alias:domain.org".to_owned()))
274+
}
275+
276+
#[test]
277+
fn test_is_room_alias_format_valid_when_server_part_is_not_lowercase_is_not_valid() {
278+
assert!(!is_room_alias_format_valid("#alias:Domain.org".to_owned()))
279+
}
280+
281+
#[test]
282+
fn test_is_room_alias_format_valid_when_has_valid_format() {
283+
assert!(is_room_alias_format_valid("#alias.test:domain.org".to_owned()))
284+
}
205285
}

0 commit comments

Comments
 (0)