From: nutomic Date: Tue, 11 Aug 2020 14:31:05 +0000 (+0000) Subject: Remove usage of Option::unwrap() in activitypub code (#80) X-Git-Url: http://these/git/%7BlocalRss%7D?a=commitdiff_plain;h=3da47352befd37fe93c6d3e14874b5b90d8b8694;p=lemmy.git Remove usage of Option::unwrap() in activitypub code (#80) Remove remaining usages of unwrap() from activitypub code Remove most usage of Option::unwrap() from activitypub code Co-authored-by: Felix Ableitner Reviewed-on: https://yerbamate.dev/LemmyNet/lemmy/pulls/80 --- diff --git a/server/lemmy_utils/src/lib.rs b/server/lemmy_utils/src/lib.rs index 6d851b03..fc50e199 100644 --- a/server/lemmy_utils/src/lib.rs +++ b/server/lemmy_utils/src/lib.rs @@ -31,6 +31,18 @@ use regex::{Regex, RegexBuilder}; use std::io::{Error, ErrorKind}; use url::Url; +#[macro_export] +macro_rules! location_info { + () => { + format!( + "None value at {}:{}, column {}", + file!(), + line!(), + column!() + ) + }; +} + pub fn to_datetime_utc(ndt: NaiveDateTime) -> DateTime { DateTime::::from_utc(ndt, Utc) } diff --git a/server/src/api/community.rs b/server/src/api/community.rs index c7160848..fb153895 100644 --- a/server/src/api/community.rs +++ b/server/src/api/community.rs @@ -605,11 +605,11 @@ impl Perform for Oper { // Dont actually add to the community followers here, because you need // to wait for the accept user - .send_follow(&community.actor_id, &self.client, pool) + .send_follow(&community.actor_id()?, &self.client, pool) .await?; } else { user - .send_unfollow(&community.actor_id, &self.client, pool) + .send_unfollow(&community.actor_id()?, &self.client, pool) .await?; let unfollow = move |conn: &'_ _| CommunityFollower::unfollow(conn, &community_follower_form); if blocking(pool, unfollow).await?.is_err() { diff --git a/server/src/apub/activities.rs b/server/src/apub/activities.rs index b5d6ce46..6b6b17b8 100644 --- a/server/src/apub/activities.rs +++ b/server/src/apub/activities.rs @@ -21,7 +21,7 @@ use uuid::Uuid; pub async fn send_activity_to_community( creator: &User_, community: &Community, - to: Vec, + to: Vec, activity: AnyBase, client: &Client, pool: &DbPool, @@ -43,17 +43,18 @@ pub async fn send_activity( client: &Client, activity: &AnyBase, actor: &dyn ActorType, - to: Vec, + to: Vec, ) -> Result<(), LemmyError> { let activity = serde_json::to_string(&activity)?; debug!("Sending activitypub activity {} to {:?}", activity, to); - for t in to { - let to_url = Url::parse(&t)?; + for to_url in to { check_is_apub_id_valid(&to_url)?; let res = retry_custom(|| async { - let request = client.post(&t).header("Content-Type", "application/json"); + let request = client + .post(to_url.as_str()) + .header("Content-Type", "application/json"); match sign(request, actor, activity.clone()).await { Ok(signed) => Ok(signed.send().await), diff --git a/server/src/apub/comment.rs b/server/src/apub/comment.rs index fbec5905..d90d8227 100644 --- a/server/src/apub/comment.rs +++ b/server/src/apub/comment.rs @@ -41,6 +41,7 @@ use activitystreams::{ public, }; use actix_web::{body::Body, client::Client, web::Path, HttpResponse}; +use anyhow::Context; use itertools::Itertools; use lemmy_db::{ comment::{Comment, CommentForm}, @@ -49,7 +50,13 @@ use lemmy_db::{ user::User_, Crud, }; -use lemmy_utils::{convert_datetime, remove_slurs, scrape_text_for_mentions, MentionData}; +use lemmy_utils::{ + convert_datetime, + location_info, + remove_slurs, + scrape_text_for_mentions, + MentionData, +}; use log::debug; use serde::Deserialize; use serde_json::Error; @@ -136,21 +143,21 @@ impl FromApub for CommentForm { ) -> Result { let creator_actor_id = ¬e .attributed_to() - .unwrap() + .context(location_info!())? .as_single_xsd_any_uri() - .unwrap(); + .context(location_info!())?; let creator = get_or_fetch_and_upsert_user(creator_actor_id, client, pool).await?; let mut in_reply_tos = note .in_reply_to() .as_ref() - .unwrap() + .context(location_info!())? .as_many() - .unwrap() + .context(location_info!())? .iter() - .map(|i| i.as_xsd_any_uri().unwrap()); - let post_ap_id = in_reply_tos.next().unwrap(); + .map(|i| i.as_xsd_any_uri().context("")); + let post_ap_id = in_reply_tos.next().context(location_info!())??; // This post, or the parent comment might not yet exist on this server yet, fetch them. let post = get_or_fetch_and_insert_post(&post_ap_id, client, pool).await?; @@ -159,7 +166,7 @@ impl FromApub for CommentForm { // For deeply nested comments, FromApub automatically gets called recursively let parent_id: Option = match in_reply_tos.next() { Some(parent_comment_uri) => { - let parent_comment_ap_id = &parent_comment_uri; + let parent_comment_ap_id = &parent_comment_uri?; let parent_comment = get_or_fetch_and_insert_comment(&parent_comment_ap_id, client, pool).await?; @@ -169,9 +176,9 @@ impl FromApub for CommentForm { }; let content = note .content() - .unwrap() + .context(location_info!())? .as_single_xsd_string() - .unwrap() + .context(location_info!())? .to_string(); let content_slurs_removed = remove_slurs(&content); @@ -295,7 +302,7 @@ impl ApubObjectType for Comment { send_activity_to_community( &creator, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], delete.into_any_base()?, client, pool, @@ -337,7 +344,7 @@ impl ApubObjectType for Comment { send_activity_to_community( &creator, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], undo.into_any_base()?, client, pool, @@ -370,7 +377,7 @@ impl ApubObjectType for Comment { send_activity_to_community( &mod_, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], remove.into_any_base()?, client, pool, @@ -412,7 +419,7 @@ impl ApubObjectType for Comment { send_activity_to_community( &mod_, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], undo.into_any_base()?, client, pool, @@ -448,7 +455,7 @@ impl ApubLikeableType for Comment { send_activity_to_community( &creator, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], like.into_any_base()?, client, pool, @@ -481,7 +488,7 @@ impl ApubLikeableType for Comment { send_activity_to_community( &creator, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], dislike.into_any_base()?, client, pool, @@ -522,7 +529,7 @@ impl ApubLikeableType for Comment { send_activity_to_community( &creator, &community, - vec![community.get_shared_inbox_url()], + vec![community.get_shared_inbox_url()?], undo.into_any_base()?, client, pool, @@ -534,7 +541,7 @@ impl ApubLikeableType for Comment { struct MentionsAndAddresses { addressed_ccs: Vec, - inboxes: Vec, + inboxes: Vec, tags: Vec, } @@ -569,7 +576,7 @@ async fn collect_non_local_mentions_and_addresses( .filter(|m| !m.is_local()) .collect::>(); - let mut mention_inboxes = Vec::new(); + let mut mention_inboxes: Vec = Vec::new(); for mention in &mentions { // TODO should it be fetching it every time? if let Ok(actor_id) = fetch_webfinger_url(mention, client).await { @@ -577,7 +584,7 @@ async fn collect_non_local_mentions_and_addresses( addressed_ccs.push(actor_id.to_owned().to_string()); let mention_user = get_or_fetch_and_upsert_user(&actor_id, client, pool).await?; - let shared_inbox = mention_user.get_shared_inbox_url(); + let shared_inbox = mention_user.get_shared_inbox_url()?; mention_inboxes.push(shared_inbox); let mut mention_tag = Mention::new(); @@ -586,7 +593,7 @@ async fn collect_non_local_mentions_and_addresses( } } - let mut inboxes = vec![community.get_shared_inbox_url()]; + let mut inboxes = vec![community.get_shared_inbox_url()?]; inboxes.extend(mention_inboxes); inboxes = inboxes.into_iter().unique().collect(); diff --git a/server/src/apub/community.rs b/server/src/apub/community.rs index 8b522b44..fb8ef9bc 100644 --- a/server/src/apub/community.rs +++ b/server/src/apub/community.rs @@ -7,8 +7,7 @@ use crate::{ create_apub_tombstone_response, create_tombstone, extensions::group_extensions::GroupExtension, - fetcher::get_or_fetch_and_upsert_user, - get_shared_inbox, + fetcher::{get_or_fetch_and_upsert_actor, get_or_fetch_and_upsert_user}, insert_activity, ActorType, FromApub, @@ -40,6 +39,7 @@ use activitystreams::{ }; use activitystreams_ext::Ext2; use actix_web::{body::Body, client::Client, web, HttpResponse}; +use anyhow::Context; use itertools::Itertools; use lemmy_db::{ community::{Community, CommunityForm}, @@ -48,7 +48,7 @@ use lemmy_db::{ post::Post, user::User_, }; -use lemmy_utils::convert_datetime; +use lemmy_utils::{convert_datetime, get_apub_protocol_string, location_info}; use serde::Deserialize; use url::Url; @@ -99,7 +99,7 @@ impl ToApub for Community { .set_following(self.get_following_url().parse()?) .set_liked(self.get_liked_url().parse()?) .set_endpoints(Endpoints { - shared_inbox: Some(self.get_shared_inbox_url().parse()?), + shared_inbox: Some(self.get_shared_inbox_url()?), ..Default::default() }); @@ -113,7 +113,7 @@ impl ToApub for Community { Ok(Ext2::new( ap_actor, group_extension, - self.get_public_key_ext(), + self.get_public_key_ext()?, )) } @@ -128,11 +128,11 @@ impl ActorType for Community { self.actor_id.to_owned() } - fn public_key(&self) -> String { - self.public_key.to_owned().unwrap() + fn public_key(&self) -> Option { + self.public_key.to_owned() } - fn private_key(&self) -> String { - self.private_key.to_owned().unwrap() + fn private_key(&self) -> Option { + self.private_key.to_owned() } /// As a local community, accept the follow request from a remote user. @@ -142,10 +142,14 @@ impl ActorType for Community { client: &Client, pool: &DbPool, ) -> Result<(), LemmyError> { - let actor_uri = follow.actor()?.as_single_xsd_any_uri().unwrap().to_string(); + let actor_uri = follow + .actor()? + .as_single_xsd_any_uri() + .context(location_info!())?; + let actor = get_or_fetch_and_upsert_actor(actor_uri, client, pool).await?; let mut accept = Accept::new(self.actor_id.to_owned(), follow.into_any_base()?); - let to = format!("{}/inbox", actor_uri); + let to = actor.get_inbox_url()?; accept .set_context(context()) .set_id(generate_activity_id(AcceptType::Accept)?) @@ -279,7 +283,10 @@ impl ActorType for Community { } /// For a given community, returns the inboxes of all followers. - async fn get_follower_inboxes(&self, pool: &DbPool) -> Result, LemmyError> { + /// + /// TODO: this function is very badly implemented, we should just store shared_inbox_url in + /// CommunityFollowerView + async fn get_follower_inboxes(&self, pool: &DbPool) -> Result, LemmyError> { let id = self.id; let inboxes = blocking(pool, move |conn| { @@ -288,8 +295,22 @@ impl ActorType for Community { .await??; let inboxes = inboxes .into_iter() - .map(|c| get_shared_inbox(&Url::parse(&c.user_actor_id).unwrap())) - .filter(|s| !s.is_empty()) + .map(|u| -> Result { + let url = Url::parse(&u.user_actor_id)?; + let domain = url.domain().context(location_info!())?; + let port = if let Some(port) = url.port() { + format!(":{}", port) + } else { + "".to_string() + }; + Ok(Url::parse(&format!( + "{}://{}{}/inbox", + get_apub_protocol_string(), + domain, + port, + ))?) + }) + .filter_map(Result::ok) .unique() .collect(); @@ -298,7 +319,7 @@ impl ActorType for Community { async fn send_follow( &self, - _follow_actor_id: &str, + _follow_actor_id: &Url, _client: &Client, _pool: &DbPool, ) -> Result<(), LemmyError> { @@ -307,7 +328,7 @@ impl ActorType for Community { async fn send_unfollow( &self, - _follow_actor_id: &str, + _follow_actor_id: &Url, _client: &Client, _pool: &DbPool, ) -> Result<(), LemmyError> { @@ -330,44 +351,50 @@ impl FromApub for CommunityForm { pool: &DbPool, expected_domain: Option, ) -> Result { - let creator_and_moderator_uris = group.inner.attributed_to().unwrap(); + let creator_and_moderator_uris = group.inner.attributed_to().context(location_info!())?; let creator_uri = creator_and_moderator_uris .as_many() - .unwrap() + .context(location_info!())? .iter() .next() - .unwrap() + .context(location_info!())? .as_xsd_any_uri() - .unwrap(); + .context(location_info!())?; let creator = get_or_fetch_and_upsert_user(creator_uri, client, pool).await?; let name = group .inner .name() - .unwrap() + .context(location_info!())? .as_one() - .unwrap() + .context(location_info!())? .as_xsd_string() - .unwrap() + .context(location_info!())? + .to_string(); + let title = group + .inner + .preferred_username() + .context(location_info!())? .to_string(); - let title = group.inner.preferred_username().unwrap().to_string(); // TODO: should be parsed as html and tags like