fix(appservices): don't panic on empty registration url

perf(appservices): cache regex for namespaces

src/service/admin/mod.rs

@@ -8,6 +8,7 @@ use std::{
 use clap::Parser;
 use regex::Regex;
 use ruma::{
+    api::appservice::Registration,
     events::{
         room::{
             canonical_alias::RoomCanonicalAliasEventContent,
@@ -335,10 +336,9 @@ impl Service {
                 if body.len() > 2 && body[0].trim() == "```" && body.last().unwrap().trim() == "```"
                 {
                     let appservice_config = body[1..body.len() - 1].join("\n");
-                    let parsed_config =
-                        serde_yaml::from_str::<serde_yaml::Value>(&appservice_config);
+                    let parsed_config = serde_yaml::from_str::<Registration>(&appservice_config);
                     match parsed_config {
-                        Ok(yaml) => match services().appservice.register_appservice(yaml) {
+                        Ok(yaml) => match services().appservice.register_appservice(yaml).await {
                             Ok(id) => RoomMessageEventContent::text_plain(format!(
                                 "Appservice registered with ID: {id}."
                             )),
@@ -361,6 +361,7 @@ impl Service {
             } => match services()
                 .appservice
                 .unregister_appservice(&appservice_identifier)
+                .await
             {
                 Ok(()) => RoomMessageEventContent::text_plain("Appservice unregistered."),
                 Err(e) => RoomMessageEventContent::text_plain(format!(

src/service/appservice/data.rs

@@ -1,8 +1,10 @@
+use ruma::api::appservice::Registration;
+
 use crate::Result;
 
 pub trait Data: Send + Sync {
     /// Registers an appservice and returns the ID to the caller
-    fn register_appservice(&self, yaml: serde_yaml::Value) -> Result<String>;
+    fn register_appservice(&self, yaml: Registration) -> Result<String>;
 
     /// Remove an appservice registration
     ///
@@ -11,9 +13,9 @@ pub trait Data: Send + Sync {
     /// * `service_name` - the name you send to register the service previously
     fn unregister_appservice(&self, service_name: &str) -> Result<()>;
 
-    fn get_registration(&self, id: &str) -> Result<Option<serde_yaml::Value>>;
+    fn get_registration(&self, id: &str) -> Result<Option<Registration>>;
 
     fn iter_ids<'a>(&'a self) -> Result<Box<dyn Iterator<Item = Result<String>> + 'a>>;
 
-    fn all(&self) -> Result<Vec<(String, serde_yaml::Value)>>;
+    fn all(&self) -> Result<Vec<(String, Registration)>>;
 }

src/service/appservice/mod.rs

@@ -1,16 +1,113 @@
 mod data;
 
+use std::collections::HashMap;
+
 pub use data::Data;
 
-use crate::Result;
+use regex::RegexSet;
+use ruma::api::appservice::{Namespace, Registration};
+use tokio::sync::RwLock;
+
+use crate::{services, Result};
+
+/// Compiled regular expressions for a namespace
+pub struct NamespaceRegex {
+    pub exclusive: Option<RegexSet>,
+    pub non_exclusive: Option<RegexSet>,
+}
+
+impl NamespaceRegex {
+    /// Checks if this namespace has rights to a namespace
+    pub fn is_match(&self, heystack: &str) -> bool {
+        if self.is_exclusive_match(heystack) {
+            return true;
+        }
+
+        if let Some(non_exclusive) = &self.non_exclusive {
+            if non_exclusive.is_match(heystack) {
+                return true;
+            }
+        }
+        false
+    }
+
+    /// Checks if this namespace has exlusive rights to a namespace
+    pub fn is_exclusive_match(&self, heystack: &str) -> bool {
+        if let Some(exclusive) = &self.exclusive {
+            if exclusive.is_match(heystack) {
+                return true;
+            }
+        }
+        false
+    }
+}
+
+impl TryFrom<Vec<Namespace>> for NamespaceRegex {
+    fn try_from(value: Vec<Namespace>) -> Result<Self, regex::Error> {
+        let mut exclusive = vec![];
+        let mut non_exclusive = vec![];
+
+        for namespace in value {
+            if namespace.exclusive {
+                exclusive.push(namespace.regex);
+            } else {
+                non_exclusive.push(namespace.regex);
+            }
+        }
+
+        Ok(NamespaceRegex {
+            exclusive: if exclusive.is_empty() {
+                None
+            } else {
+                Some(RegexSet::new(exclusive)?)
+            },
+            non_exclusive: if non_exclusive.is_empty() {
+                None
+            } else {
+                Some(RegexSet::new(non_exclusive)?)
+            },
+        })
+    }
+
+    type Error = regex::Error;
+}
+
+/// Compiled regular expressions for an appservice
+pub struct RegistrationInfo {
+    pub registration: Registration,
+    pub users: NamespaceRegex,
+    pub aliases: NamespaceRegex,
+    pub rooms: NamespaceRegex,
+}
+
+impl TryFrom<Registration> for RegistrationInfo {
+    fn try_from(value: Registration) -> Result<RegistrationInfo, regex::Error> {
+        Ok(RegistrationInfo {
+            users: value.namespaces.users.clone().try_into()?,
+            aliases: value.namespaces.aliases.clone().try_into()?,
+            rooms: value.namespaces.rooms.clone().try_into()?,
+            registration: value,
+        })
+    }
+
+    type Error = regex::Error;
+}
 
 pub struct Service {
     pub db: &'static dyn Data,
+    pub registration_info: RwLock<HashMap<String, RegistrationInfo>>,
 }
 
 impl Service {
     /// Registers an appservice and returns the ID to the caller
-    pub fn register_appservice(&self, yaml: serde_yaml::Value) -> Result<String> {
+    pub async fn register_appservice(&self, yaml: Registration) -> Result<String> {
+        services()
+            .appservice
+            .registration_info
+            .write()
+            .await
+            .insert(yaml.id.clone(), yaml.clone().try_into()?);
+
         self.db.register_appservice(yaml)
     }
 
@@ -19,11 +116,18 @@ impl Service {
     /// # Arguments
     ///
     /// * `service_name` - the name you send to register the service previously
-    pub fn unregister_appservice(&self, service_name: &str) -> Result<()> {
+    pub async fn unregister_appservice(&self, service_name: &str) -> Result<()> {
+        services()
+            .appservice
+            .registration_info
+            .write()
+            .await
+            .remove(service_name);
+
         self.db.unregister_appservice(service_name)
     }
 
-    pub fn get_registration(&self, id: &str) -> Result<Option<serde_yaml::Value>> {
+    pub fn get_registration(&self, id: &str) -> Result<Option<Registration>> {
         self.db.get_registration(id)
     }
 
@@ -31,7 +135,7 @@ impl Service {
         self.db.iter_ids()
     }
 
-    pub fn all(&self) -> Result<Vec<(String, serde_yaml::Value)>> {
+    pub fn all(&self) -> Result<Vec<(String, Registration)>> {
         self.db.all()
     }
 }

src/service/mod.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use lru_cache::LruCache;
-use tokio::sync::Mutex;
+use tokio::sync::{Mutex, RwLock};
 
 use crate::{Config, Result};
 
@@ -56,7 +56,10 @@ impl Services {
         config: Config,
     ) -> Result<Self> {
         Ok(Self {
-            appservice: appservice::Service { db },
+            appservice: appservice::Service {
+                db,
+                registration_info: RwLock::new(HashMap::new()),
+            },
             pusher: pusher::Service { db },
             rooms: rooms::Service {
                 alias: rooms::alias::Service { db },

src/service/rooms/state_cache/data.rs

@@ -1,6 +1,6 @@
 use std::{collections::HashSet, sync::Arc};
 
-use crate::Result;
+use crate::{service::appservice::RegistrationInfo, Result};
 use ruma::{
     events::{AnyStrippedStateEvent, AnySyncStateEvent},
     serde::Raw,
@@ -22,11 +22,7 @@ pub trait Data: Send + Sync {
 
     fn get_our_real_users(&self, room_id: &RoomId) -> Result<Arc<HashSet<OwnedUserId>>>;
 
-    fn appservice_in_room(
-        &self,
-        room_id: &RoomId,
-        appservice: &(String, serde_yaml::Value),
-    ) -> Result<bool>;
+    fn appservice_in_room(&self, room_id: &RoomId, appservice: &RegistrationInfo) -> Result<bool>;
 
     /// Makes a user forget a room.
     fn forget(&self, room_id: &RoomId, user_id: &UserId) -> Result<()>;

src/service/rooms/state_cache/mod.rs

@@ -16,7 +16,7 @@ use ruma::{
 };
 use tracing::warn;
 
-use crate::{services, Error, Result};
+use crate::{service::appservice::RegistrationInfo, services, Error, Result};
 
 pub struct Service {
     pub db: &'static dyn Data,
@@ -205,7 +205,7 @@ impl Service {
     pub fn appservice_in_room(
         &self,
         room_id: &RoomId,
-        appservice: &(String, serde_yaml::Value),
+        appservice: &RegistrationInfo,
     ) -> Result<bool> {
         self.db.appservice_in_room(room_id, appservice)
     }

src/service/rooms/timeline/mod.rs

@@ -7,7 +7,7 @@ use std::{
 };
 
 pub use data::Data;
-use regex::Regex;
+
 use ruma::{
     api::{client::error::ErrorKind, federation},
     canonical_json::to_canonical_value,
@@ -21,8 +21,7 @@ use ruma::{
     },
     push::{Action, Ruleset, Tweak},
     serde::Base64,
-    state_res,
-    state_res::{Event, RoomVersion},
+    state_res::{self, Event, RoomVersion},
     uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedEventId, OwnedRoomId,
     OwnedServerName, RoomId, ServerName, UserId,
 };
@@ -33,7 +32,10 @@ use tracing::{error, info, warn};
 
 use crate::{
     api::server_server,
-    service::pdu::{EventHash, PduBuilder},
+    service::{
+        appservice::NamespaceRegex,
+        pdu::{EventHash, PduBuilder},
+    },
     services, utils, Error, PduEvent, Result,
 };
 
@@ -522,15 +524,21 @@ impl Service {
             }
         }
 
-        for appservice in services().appservice.all()? {
+        for appservice in services()
+            .appservice
+            .registration_info
+            .read()
+            .await
+            .values()
+        {
             if services()
                 .rooms
                 .state_cache
-                .appservice_in_room(&pdu.room_id, &appservice)?
+                .appservice_in_room(&pdu.room_id, appservice)?
             {
                 services()
                     .sending
-                    .send_pdu_appservice(appservice.0, pdu_id.clone())?;
+                    .send_pdu_appservice(appservice.registration.id.clone(), pdu_id.clone())?;
                 continue;
             }
 
@@ -542,73 +550,41 @@ impl Service {
                     .as_ref()
                     .and_then(|state_key| UserId::parse(state_key.as_str()).ok())
                 {
-                    if let Some(appservice_uid) = appservice
-                        .1
-                        .get("sender_localpart")
-                        .and_then(|string| string.as_str())
-                        .and_then(|string| {
-                            UserId::parse_with_server_name(string, services().globals.server_name())
-                                .ok()
-                        })
-                    {
-                        if state_key_uid == &appservice_uid {
-                            services()
-                                .sending
-                                .send_pdu_appservice(appservice.0, pdu_id.clone())?;
-                            continue;
-                        }
+                    let appservice_uid = appservice.registration.sender_localpart.as_str();
+                    if state_key_uid == appservice_uid {
+                        services().sending.send_pdu_appservice(
+                            appservice.registration.id.clone(),
+                            pdu_id.clone(),
+                        )?;
+                        continue;
                     }
                 }
             }
 
-            if let Some(namespaces) = appservice.1.get("namespaces") {
-                let users = namespaces
-                    .get("users")
-                    .and_then(|users| users.as_sequence())
-                    .map_or_else(Vec::new, |users| {
-                        users
-                            .iter()
-                            .filter_map(|users| Regex::new(users.get("regex")?.as_str()?).ok())
-                            .collect::<Vec<_>>()
-                    });
-                let aliases = namespaces
-                    .get("aliases")
-                    .and_then(|aliases| aliases.as_sequence())
-                    .map_or_else(Vec::new, |aliases| {
-                        aliases
-                            .iter()
-                            .filter_map(|aliases| Regex::new(aliases.get("regex")?.as_str()?).ok())
-                            .collect::<Vec<_>>()
-                    });
-                let rooms = namespaces
-                    .get("rooms")
-                    .and_then(|rooms| rooms.as_sequence());
+            let matching_users = |users: &NamespaceRegex| {
+                appservice.users.is_match(pdu.sender.as_str())
+                    || pdu.kind == TimelineEventType::RoomMember
+                        && pdu
+                            .state_key
+                            .as_ref()
+                            .map_or(false, |state_key| users.is_match(state_key))
+            };
+            let matching_aliases = |aliases: &NamespaceRegex| {
+                services()
+                    .rooms
+                    .alias
+                    .local_aliases_for_room(&pdu.room_id)
+                    .filter_map(|r| r.ok())
+                    .any(|room_alias| aliases.is_match(room_alias.as_str()))
+            };
 
-                let matching_users = |users: &Regex| {
-                    users.is_match(pdu.sender.as_str())
-                        || pdu.kind == TimelineEventType::RoomMember
-                            && pdu
-                                .state_key
-                                .as_ref()
-                                .map_or(false, |state_key| users.is_match(state_key))
-                };
-                let matching_aliases = |aliases: &Regex| {
-                    services()
-                        .rooms
-                        .alias
-                        .local_aliases_for_room(&pdu.room_id)
-                        .filter_map(|r| r.ok())
-                        .any(|room_alias| aliases.is_match(room_alias.as_str()))
-                };
-
-                if aliases.iter().any(matching_aliases)
-                    || rooms.map_or(false, |rooms| rooms.contains(&pdu.room_id.as_str().into()))
-                    || users.iter().any(matching_users)
-                {
-                    services()
-                        .sending
-                        .send_pdu_appservice(appservice.0, pdu_id.clone())?;
-                }
+            if matching_aliases(&appservice.aliases)
+                || appservice.rooms.is_match(pdu.room_id.as_str())
+                || matching_users(&appservice.users)
+            {
+                services()
+                    .sending
+                    .send_pdu_appservice(appservice.registration.id.clone(), pdu_id.clone())?;
             }
         }
 

src/service/sending/mod.rs

@@ -22,7 +22,7 @@ use base64::{engine::general_purpose, Engine as _};
 
 use ruma::{
     api::{
-        appservice,
+        appservice::{self, Registration},
         federation::{
             self,
             transactions::edu::{
@@ -484,7 +484,7 @@ impl Service {
 
                 let permit = services().sending.maximum_requests.acquire().await;
 
-                let response = appservice_server::send_request(
+                let response = match appservice_server::send_request(
                     services()
                         .appservice
                         .get_registration(id)
@@ -511,8 +511,12 @@ impl Service {
                     },
                 )
                 .await
-                .map(|_response| kind.clone())
-                .map_err(|e| (kind, e));
+                {
+                    None => Ok(kind.clone()),
+                    Some(op_resp) => op_resp
+                        .map(|_response| kind.clone())
+                        .map_err(|e| (kind.clone(), e)),
+                };
 
                 drop(permit);
 
@@ -698,12 +702,15 @@ impl Service {
         response
     }
 
+    /// Sends a request to an appservice
+    ///
+    /// Only returns None if there is no url specified in the appservice registration file
     #[tracing::instrument(skip(self, registration, request))]
     pub async fn send_appservice_request<T: OutgoingRequest>(
         &self,
-        registration: serde_yaml::Value,
+        registration: Registration,
         request: T,
-    ) -> Result<T::IncomingResponse>
+    ) -> Option<Result<T::IncomingResponse>>
     where
         T: Debug,
     {