Avoid crash when handling urls without domain (#4676)

* Avoid crash when handling urls without domain

* Add some extra checks

crates/apub/src/activities/block/block_user.rs

@@ -39,7 +39,10 @@ use lemmy_db_schema::{
   },
   traits::{Bannable, Crud, Followable},
 };
-use lemmy_utils::error::{LemmyError, LemmyResult};
+use lemmy_utils::{
+  error::{LemmyError, LemmyResult},
+  LemmyErrorType,
+};
 use url::Url;
 
 impl BlockUser {
@@ -129,7 +132,11 @@ impl ActivityHandler for BlockUser {
     verify_is_public(&self.to, &self.cc)?;
     match self.target.dereference(context).await? {
       SiteOrCommunity::Site(site) => {
-        let domain = self.object.inner().domain().expect("url needs domain");
+        let domain = self
+          .object
+          .inner()
+          .domain()
+          .ok_or(LemmyErrorType::UrlWithoutDomain)?;
         if context.settings().hostname == domain {
           return Err(
             anyhow!("Site bans from remote instance can't affect user's home instance").into(),

crates/apub/src/activities/create_or_update/comment.rs

@@ -19,7 +19,7 @@ use activitypub_federation::{
   config::Data,
   fetch::object_id::ObjectId,
   kinds::public,
-  protocol::verification::verify_domains_match,
+  protocol::verification::{verify_domains_match, verify_urls_match},
   traits::{ActivityHandler, Actor, Object},
 };
 use lemmy_api_common::{
@@ -133,6 +133,7 @@ impl ActivityHandler for CreateOrUpdateNote {
     verify_domains_match(self.actor.inner(), self.object.id.inner())?;
     check_community_deleted_or_removed(&community)?;
     check_post_deleted_or_removed(&post)?;
+    verify_urls_match(self.actor.inner(), self.object.attributed_to.inner())?;
 
     ApubComment::verify(&self.object, self.actor.inner(), context).await?;
     Ok(())

crates/apub/src/activities/create_or_update/private_message.rs

@@ -9,7 +9,7 @@ use crate::{
 };
 use activitypub_federation::{
   config::Data,
-  protocol::verification::verify_domains_match,
+  protocol::verification::{verify_domains_match, verify_urls_match},
   traits::{ActivityHandler, Actor, Object},
 };
 use lemmy_api_common::context::LemmyContext;
@@ -61,6 +61,7 @@ impl ActivityHandler for CreateOrUpdateChatMessage {
     verify_person(&self.actor, context).await?;
     verify_domains_match(self.actor.inner(), self.object.id.inner())?;
     verify_domains_match(self.to[0].inner(), self.object.to[0].inner())?;
+    verify_urls_match(self.actor.inner(), self.object.attributed_to.inner())?;
     ApubPrivateMessage::verify(&self.object, self.actor.inner(), context).await?;
     Ok(())
   }

crates/apub/src/lib.rs

@@ -78,7 +78,10 @@ impl UrlVerifier for VerifyUrlData {
 /// - URL not being in the blocklist (if it is active)
 #[tracing::instrument(skip(local_site_data))]
 fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> LemmyResult<()> {
-  let domain = apub_id.domain().expect("apud id has domain").to_string();
+  let domain = apub_id
+    .domain()
+    .ok_or(LemmyErrorType::UrlWithoutDomain)?
+    .to_string();
 
   if !local_site_data
     .local_site
@@ -158,7 +161,10 @@ pub(crate) async fn check_apub_id_valid_with_strictness(
   is_strict: bool,
   context: &LemmyContext,
 ) -> LemmyResult<()> {
-  let domain = apub_id.domain().expect("apud id has domain").to_string();
+  let domain = apub_id
+    .domain()
+    .ok_or(LemmyErrorType::UrlWithoutDomain)?
+    .to_string();
   let local_instance = context
     .settings()
     .get_hostname_without_port()
@@ -185,7 +191,10 @@ pub(crate) async fn check_apub_id_valid_with_strictness(
       .expect("local hostname is valid");
     allowed_and_local.push(local_instance);
 
-    let domain = apub_id.domain().expect("apud id has domain").to_string();
+    let domain = apub_id
+      .domain()
+      .ok_or(LemmyErrorType::UrlWithoutDomain)?
+      .to_string();
     if !allowed_and_local.contains(&domain) {
       Err(LemmyErrorType::FederationDisabledByStrictAllowList)?
     }

crates/apub/src/mentions.rs

@@ -54,7 +54,10 @@ pub async fn collect_non_local_mentions(
     name: Some(format!(
       "@{}@{}",
       &parent_creator.name,
-      &parent_creator.id().domain().expect("has domain")
+      &parent_creator
+        .id()
+        .domain()
+        .ok_or(LemmyErrorType::UrlWithoutDomain)?
     )),
     kind: MentionType::Mention,
   };

crates/apub/src/objects/instance.rs

@@ -45,6 +45,7 @@ use lemmy_utils::{
     markdown::markdown_to_html,
     slurs::{check_slurs, check_slurs_opt},
   },
+  LemmyErrorType,
 };
 use std::ops::Deref;
 use tracing::debug;
@@ -137,7 +138,11 @@ impl Object for ApubSite {
 
   #[tracing::instrument(skip_all)]
   async fn from_json(apub: Self::Kind, context: &Data<Self::DataType>) -> LemmyResult<Self> {
-    let domain = apub.id.inner().domain().expect("group id has domain");
+    let domain = apub
+      .id
+      .inner()
+      .domain()
+      .ok_or(LemmyErrorType::UrlWithoutDomain)?;
     let instance = DbInstance::read_or_create(&mut context.pool(), domain.to_string()).await?;
 
     let local_site = LocalSite::read(&mut context.pool()).await.ok();
@@ -210,7 +215,9 @@ pub(in crate::objects) async fn fetch_instance_actor_for_object<T: Into<Url> + C
     Err(e) => {
       // Failed to fetch instance actor, its probably not a lemmy instance
       debug!("Failed to dereference site for {}: {}", &instance_id, e);
-      let domain = instance_id.domain().expect("has domain");
+      let domain = instance_id
+        .domain()
+        .ok_or(LemmyErrorType::UrlWithoutDomain)?;
       Ok(
         DbInstance::read_or_create(&mut context.pool(), domain.to_string())
           .await?

crates/utils/src/error.rs

@@ -176,6 +176,7 @@ pub enum LemmyErrorType {
   InvalidUnixTime,
   InvalidBotAction,
   CantBlockLocalInstance,
+  UrlWithoutDomain,
   Unknown(String),
 }