From 8ea21c39b7d25bf39e9d333040674b55cf640b50 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Sat, 6 Nov 2021 14:25:34 +0100 Subject: [PATCH] Reduce stack memory usage in apub code - use our own, smaller Endpoints struct - wrap ObjectId.url in Box - adjust usage of Box in different places --- crates/api/src/post.rs | 2 +- crates/api_crud/src/post/create.rs | 2 +- crates/apub/src/activities/community/update.rs | 16 +++++++--------- crates/apub/src/activities/deletion/mod.rs | 12 ++++++------ .../apub/src/activities/post/create_or_update.rs | 2 +- crates/apub/src/activities/voting/undo_vote.rs | 3 +-- crates/apub/src/activities/voting/vote.rs | 3 +-- crates/apub/src/activity_lists.rs | 10 +++++----- crates/apub/src/fetcher/post_or_comment.rs | 16 ++++++++-------- crates/apub/src/http/community.rs | 2 +- crates/apub/src/http/mod.rs | 2 +- crates/apub/src/objects/community.rs | 8 ++------ crates/apub/src/objects/person.rs | 8 +++++--- .../src/protocol/activities/community/update.rs | 2 +- crates/apub/src/protocol/objects/group.rs | 10 +++------- crates/apub/src/protocol/objects/mod.rs | 10 ++++++++++ crates/apub/src/protocol/objects/person.rs | 6 +++--- crates/apub_lib/src/object_id.rs | 9 +++++---- crates/apub_lib/src/signatures.rs | 4 ++-- crates/apub_lib/src/traits.rs | 2 +- 20 files changed, 65 insertions(+), 64 deletions(-) diff --git a/crates/api/src/post.rs b/crates/api/src/post.rs index c7acb08fb..f22104eb7 100644 --- a/crates/api/src/post.rs +++ b/crates/api/src/post.rs @@ -73,7 +73,7 @@ impl Perform for CreatePostLike { .await??; let community_id = post.community_id; - let object = PostOrComment::Post(Box::new(post)); + let object = PostOrComment::Post(post); // Only add the like if the score isnt 0 let do_add = like_form.score != 0 && (like_form.score == 1 || like_form.score == -1); diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 248adee6c..1147c1a32 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -146,7 +146,7 @@ impl PerformCrud for CreatePost { context, ) .await?; - let object = PostOrComment::Post(Box::new(apub_post)); + let object = PostOrComment::Post(apub_post); Vote::send( &object, &local_user_view.person.clone().into(), diff --git a/crates/apub/src/activities/community/update.rs b/crates/apub/src/activities/community/update.rs index 688722479..f5d7e5651 100644 --- a/crates/apub/src/activities/community/update.rs +++ b/crates/apub/src/activities/community/update.rs @@ -9,7 +9,7 @@ use crate::{ }, activity_lists::AnnouncableActivities, objects::{community::ApubCommunity, person::ApubPerson}, - protocol::{activities::community::update::UpdateCommunity, objects::group::Group}, + protocol::activities::community::update::UpdateCommunity, }; use activitystreams::{activity::kind::UpdateType, public}; use lemmy_api_common::blocking; @@ -38,14 +38,14 @@ impl UpdateCommunity { let update = UpdateCommunity { actor: ObjectId::new(actor.actor_id()), to: vec![public()], - object: community.clone().into_apub(context).await?, + object: Box::new(community.clone().into_apub(context).await?), cc: vec![community.actor_id()], kind: UpdateType::Update, id: id.clone(), unparsed: Default::default(), }; - let activity = AnnouncableActivities::UpdateCommunity(Box::new(update)); + let activity = AnnouncableActivities::UpdateCommunity(update); send_to_community(activity, &id, actor, &community, vec![], context).await } } @@ -73,12 +73,10 @@ impl ActivityHandler for UpdateCommunity { ) -> Result<(), LemmyError> { let community = self.get_community(context, request_counter).await?; - let updated_community = Group::into_form( - self.object, - &community.actor_id.clone().into(), - &context.settings(), - ) - .await?; + let updated_community = self + .object + .into_form(&community.actor_id.clone().into(), &context.settings()) + .await?; let cf = CommunityForm { name: updated_community.name, title: updated_community.title, diff --git a/crates/apub/src/activities/deletion/mod.rs b/crates/apub/src/activities/deletion/mod.rs index ddd607a7c..1d5a3836f 100644 --- a/crates/apub/src/activities/deletion/mod.rs +++ b/crates/apub/src/activities/deletion/mod.rs @@ -53,9 +53,9 @@ pub async fn send_apub_remove( } pub enum DeletableObjects { - Community(Box), - Comment(Box), - Post(Box), + Community(ApubCommunity), + Comment(ApubComment), + Post(ApubPost), } impl DeletableObjects { @@ -64,13 +64,13 @@ impl DeletableObjects { context: &LemmyContext, ) -> Result { if let Some(c) = ApubCommunity::read_from_apub_id(ap_id.clone(), context).await? { - return Ok(DeletableObjects::Community(Box::new(c))); + return Ok(DeletableObjects::Community(c)); } if let Some(p) = ApubPost::read_from_apub_id(ap_id.clone(), context).await? { - return Ok(DeletableObjects::Post(Box::new(p))); + return Ok(DeletableObjects::Post(p)); } if let Some(c) = ApubComment::read_from_apub_id(ap_id.clone(), context).await? { - return Ok(DeletableObjects::Comment(Box::new(c))); + return Ok(DeletableObjects::Comment(c)); } Err(diesel::NotFound.into()) } diff --git a/crates/apub/src/activities/post/create_or_update.rs b/crates/apub/src/activities/post/create_or_update.rs index 982f90ccc..1c71c8ec1 100644 --- a/crates/apub/src/activities/post/create_or_update.rs +++ b/crates/apub/src/activities/post/create_or_update.rs @@ -61,7 +61,7 @@ impl CreateOrUpdatePost { .into(); let create_or_update = CreateOrUpdatePost::new(post, actor, &community, kind, context).await?; let id = create_or_update.id.clone(); - let activity = AnnouncableActivities::CreateOrUpdatePost(Box::new(create_or_update)); + let activity = AnnouncableActivities::CreateOrUpdatePost(create_or_update); send_to_community(activity, &id, actor, &community, vec![], context).await } } diff --git a/crates/apub/src/activities/voting/undo_vote.rs b/crates/apub/src/activities/voting/undo_vote.rs index 0fe40b628..6d6f3eaff 100644 --- a/crates/apub/src/activities/voting/undo_vote.rs +++ b/crates/apub/src/activities/voting/undo_vote.rs @@ -26,7 +26,6 @@ use lemmy_apub_lib::{ use lemmy_db_schema::{newtypes::CommunityId, source::community::Community, traits::Crud}; use lemmy_utils::LemmyError; use lemmy_websocket::LemmyContext; -use std::ops::Deref; impl UndoVote { pub async fn send( @@ -90,7 +89,7 @@ impl ActivityHandler for UndoVote { .dereference(context, request_counter) .await?; match object { - PostOrComment::Post(p) => undo_vote_post(actor, p.deref(), context).await, + PostOrComment::Post(p) => undo_vote_post(actor, &p, context).await, PostOrComment::Comment(c) => undo_vote_comment(actor, &c, context).await, } } diff --git a/crates/apub/src/activities/voting/vote.rs b/crates/apub/src/activities/voting/vote.rs index 6b9bfbd2f..89e88fe64 100644 --- a/crates/apub/src/activities/voting/vote.rs +++ b/crates/apub/src/activities/voting/vote.rs @@ -26,7 +26,6 @@ use lemmy_db_schema::{ }; use lemmy_utils::LemmyError; use lemmy_websocket::LemmyContext; -use std::ops::Deref; impl Vote { pub(in crate::activities::voting) fn new( @@ -90,7 +89,7 @@ impl ActivityHandler for Vote { let actor = self.actor.dereference(context, request_counter).await?; let object = self.object.dereference(context, request_counter).await?; match object { - PostOrComment::Post(p) => vote_post(&self.kind, actor, p.deref(), context).await, + PostOrComment::Post(p) => vote_post(&self.kind, actor, &p, context).await, PostOrComment::Comment(c) => vote_comment(&self.kind, actor, &c, context).await, } } diff --git a/crates/apub/src/activity_lists.rs b/crates/apub/src/activity_lists.rs index 6c866b83b..2b695111f 100644 --- a/crates/apub/src/activity_lists.rs +++ b/crates/apub/src/activity_lists.rs @@ -38,7 +38,7 @@ pub enum SharedInboxActivities { GroupInboxActivities(GroupInboxActivities), // Note, pm activities need to be at the end, otherwise comments will end up here. We can probably // avoid this problem by replacing createpm.object with our own struct, instead of NoteExt. - PersonInboxActivities(PersonInboxActivities), + PersonInboxActivities(Box), } #[derive(Clone, Debug, Deserialize, Serialize, ActivityHandler)] @@ -47,7 +47,7 @@ pub enum SharedInboxActivities { pub enum GroupInboxActivities { FollowCommunity(FollowCommunity), UndoFollowCommunity(UndoFollowCommunity), - AnnouncableActivities(AnnouncableActivities), + AnnouncableActivities(Box), Report(Report), } @@ -61,7 +61,7 @@ pub enum PersonInboxActivities { CreateOrUpdatePrivateMessage(CreateOrUpdatePrivateMessage), DeletePrivateMessage(DeletePrivateMessage), UndoDeletePrivateMessage(UndoDeletePrivateMessage), - AnnounceActivity(Box), + AnnounceActivity(AnnounceActivity), } #[derive(Clone, Debug, Deserialize, Serialize, ActivityHandler)] @@ -69,12 +69,12 @@ pub enum PersonInboxActivities { #[activity_handler(LemmyContext)] pub enum AnnouncableActivities { CreateOrUpdateComment(CreateOrUpdateComment), - CreateOrUpdatePost(Box), + CreateOrUpdatePost(CreateOrUpdatePost), Vote(Vote), UndoVote(UndoVote), Delete(Delete), UndoDelete(UndoDelete), - UpdateCommunity(Box), + UpdateCommunity(UpdateCommunity), BlockUserFromCommunity(BlockUserFromCommunity), UndoBlockUserFromCommunity(UndoBlockUserFromCommunity), AddMod(AddMod), diff --git a/crates/apub/src/fetcher/post_or_comment.rs b/crates/apub/src/fetcher/post_or_comment.rs index 075b018e3..dc1546f9c 100644 --- a/crates/apub/src/fetcher/post_or_comment.rs +++ b/crates/apub/src/fetcher/post_or_comment.rs @@ -11,15 +11,15 @@ use url::Url; #[derive(Clone, Debug)] pub enum PostOrComment { - Post(Box), + Post(ApubPost), Comment(ApubComment), } #[derive(Deserialize)] #[serde(untagged)] pub enum PageOrNote { - Page(Box), - Note(Box), + Page(Page), + Note(Note), } #[async_trait::async_trait(?Send)] @@ -39,7 +39,7 @@ impl ApubObject for PostOrComment { ) -> Result, LemmyError> { let post = ApubPost::read_from_apub_id(object_id.clone(), data).await?; Ok(match post { - Some(o) => Some(PostOrComment::Post(Box::new(o))), + Some(o) => Some(PostOrComment::Post(o)), None => ApubComment::read_from_apub_id(object_id, data) .await? .map(PostOrComment::Comment), @@ -68,11 +68,11 @@ impl ApubObject for PostOrComment { request_counter: &mut i32, ) -> Result { Ok(match apub { - PageOrNote::Page(p) => PostOrComment::Post(Box::new( - ApubPost::from_apub(*p, context, expected_domain, request_counter).await?, - )), + PageOrNote::Page(p) => PostOrComment::Post( + ApubPost::from_apub(p, context, expected_domain, request_counter).await?, + ), PageOrNote::Note(n) => PostOrComment::Comment( - ApubComment::from_apub(*n, context, expected_domain, request_counter).await?, + ApubComment::from_apub(n, context, expected_domain, request_counter).await?, ), }) } diff --git a/crates/apub/src/http/community.rs b/crates/apub/src/http/community.rs index 10794d993..1854da98b 100644 --- a/crates/apub/src/http/community.rs +++ b/crates/apub/src/http/community.rs @@ -85,7 +85,7 @@ pub(in crate::http) async fn receive_group_inbox( let community = announcable.get_community(context, &mut 0).await?; verify_person_in_community(&actor_id, &community, context, &mut 0).await?; if community.local { - AnnounceActivity::send(announcable, &community, vec![], context).await?; + AnnounceActivity::send(*announcable, &community, vec![], context).await?; } } diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index 03b68b94b..b3288b0d9 100644 --- a/crates/apub/src/http/mod.rs +++ b/crates/apub/src/http/mod.rs @@ -52,7 +52,7 @@ pub async fn shared_inbox( receive_group_inbox(g, activity_data, request, &context).await } SharedInboxActivities::PersonInboxActivities(p) => { - receive_person_inbox(p, activity_data, request, &context).await + receive_person_inbox(*p, activity_data, request, &context).await } } } diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index 1eb58995c..35a19353f 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -4,15 +4,12 @@ use crate::{ generate_moderators_url, generate_outbox_url, protocol::{ - objects::{group::Group, tombstone::Tombstone}, + objects::{group::Group, tombstone::Tombstone, Endpoints}, ImageObject, Source, }, }; -use activitystreams::{ - actor::{kind::GroupType, Endpoints}, - object::kind::ImageType, -}; +use activitystreams::{actor::kind::GroupType, object::kind::ImageType}; use chrono::NaiveDateTime; use itertools::Itertools; use lemmy_api_common::blocking; @@ -111,7 +108,6 @@ impl ApubObject for ApubCommunity { followers: self.followers_url.clone().into(), endpoints: Endpoints { shared_inbox: self.shared_inbox_url.clone().map(|s| s.into()), - ..Default::default() }, public_key: self.get_public_key()?, published: Some(convert_datetime(self.published)), diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index dcaa9cd6d..5f494b70f 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -3,12 +3,15 @@ use crate::{ generate_outbox_url, objects::get_summary_from_string_or_source, protocol::{ - objects::person::{Person, UserTypes}, + objects::{ + person::{Person, UserTypes}, + Endpoints, + }, ImageObject, Source, }, }; -use activitystreams::{actor::Endpoints, object::kind::ImageType}; +use activitystreams::object::kind::ImageType; use chrono::NaiveDateTime; use lemmy_api_common::blocking; use lemmy_apub_lib::{ @@ -109,7 +112,6 @@ impl ApubObject for ApubPerson { outbox: generate_outbox_url(&self.actor_id)?.into(), endpoints: Endpoints { shared_inbox: self.shared_inbox_url.clone().map(|s| s.into()), - ..Default::default() }, public_key: self.get_public_key()?, updated: self.updated.map(convert_datetime), diff --git a/crates/apub/src/protocol/activities/community/update.rs b/crates/apub/src/protocol/activities/community/update.rs index b9877338a..9a831a1f8 100644 --- a/crates/apub/src/protocol/activities/community/update.rs +++ b/crates/apub/src/protocol/activities/community/update.rs @@ -12,7 +12,7 @@ pub struct UpdateCommunity { pub(crate) actor: ObjectId, pub(crate) to: Vec, // TODO: would be nice to use a separate struct here, which only contains the fields updated here - pub(crate) object: Group, + pub(crate) object: Box, pub(crate) cc: Vec, #[serde(rename = "type")] pub(crate) kind: UpdateType, diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs index acaf59092..a761a4425 100644 --- a/crates/apub/src/protocol/objects/group.rs +++ b/crates/apub/src/protocol/objects/group.rs @@ -5,12 +5,9 @@ use crate::{ community_outbox::ApubCommunityOutbox, }, objects::{community::ApubCommunity, get_summary_from_string_or_source}, - protocol::{ImageObject, Source}, -}; -use activitystreams::{ - actor::{kind::GroupType, Endpoints}, - unparsed::Unparsed, + protocol::{objects::Endpoints, ImageObject, Source}, }; +use activitystreams::{actor::kind::GroupType, unparsed::Unparsed}; use chrono::{DateTime, FixedOffset}; use lemmy_apub_lib::{object_id::ObjectId, signatures::PublicKey, verify::verify_domains_match}; use lemmy_db_schema::{naive_now, source::community::CommunityForm}; @@ -46,7 +43,7 @@ pub struct Group { pub(crate) inbox: Url, pub(crate) outbox: ObjectId, pub(crate) followers: Url, - pub(crate) endpoints: Endpoints, + pub(crate) endpoints: Endpoints, pub(crate) public_key: PublicKey, pub(crate) published: Option>, pub(crate) updated: Option>, @@ -72,7 +69,6 @@ impl Group { check_slurs(&title, slur_regex)?; check_slurs_opt(&description, slur_regex)?; - // TODO: test_parse_lemmy_community_moderators() keeps failing here with stack overflow Ok(CommunityForm { name, title, diff --git a/crates/apub/src/protocol/objects/mod.rs b/crates/apub/src/protocol/objects/mod.rs index 529ee6370..bf53d5c99 100644 --- a/crates/apub/src/protocol/objects/mod.rs +++ b/crates/apub/src/protocol/objects/mod.rs @@ -1,3 +1,6 @@ +use serde::{Deserialize, Serialize}; +use url::Url; + pub(crate) mod chat_message; pub(crate) mod group; pub(crate) mod note; @@ -5,6 +8,13 @@ pub(crate) mod page; pub(crate) mod person; pub(crate) mod tombstone; +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Endpoints { + #[serde(skip_serializing_if = "Option::is_none")] + pub shared_inbox: Option, +} + #[cfg(test)] mod tests { use crate::protocol::{ diff --git a/crates/apub/src/protocol/objects/person.rs b/crates/apub/src/protocol/objects/person.rs index cf9c10b1f..e45ea78c7 100644 --- a/crates/apub/src/protocol/objects/person.rs +++ b/crates/apub/src/protocol/objects/person.rs @@ -1,8 +1,8 @@ use crate::{ objects::person::ApubPerson, - protocol::{ImageObject, Source}, + protocol::{objects::Endpoints, ImageObject, Source}, }; -use activitystreams::{actor::Endpoints, unparsed::Unparsed, url::Url}; +use activitystreams::{unparsed::Unparsed, url::Url}; use chrono::{DateTime, FixedOffset}; use lemmy_apub_lib::{object_id::ObjectId, signatures::PublicKey}; use serde::{Deserialize, Serialize}; @@ -35,7 +35,7 @@ pub struct Person { pub(crate) inbox: Url, /// mandatory field in activitypub, currently empty in lemmy pub(crate) outbox: Url, - pub(crate) endpoints: Endpoints, + pub(crate) endpoints: Endpoints, pub(crate) public_key: PublicKey, pub(crate) published: Option>, pub(crate) updated: Option>, diff --git a/crates/apub_lib/src/object_id.rs b/crates/apub_lib/src/object_id.rs index 52fe5b470..8e4b70a23 100644 --- a/crates/apub_lib/src/object_id.rs +++ b/crates/apub_lib/src/object_id.rs @@ -28,9 +28,10 @@ lazy_static! { .unwrap(); } +/// We store Url on the heap because it is quite large (88 bytes). #[derive(Clone, PartialEq, Serialize, Deserialize, Debug)] #[serde(transparent)] -pub struct ObjectId(Url, #[serde(skip)] PhantomData) +pub struct ObjectId(Box, #[serde(skip)] PhantomData) where Kind: ApubObject + Send + 'static, for<'de2> ::ApubType: serde::Deserialize<'de2>; @@ -44,7 +45,7 @@ where where T: Into, { - ObjectId(url.into(), PhantomData::) + ObjectId(Box::new(url.into()), PhantomData::) } pub fn inner(&self) -> &Url { @@ -103,7 +104,7 @@ where data: &::DataType, ) -> Result, LemmyError> { let id = self.0.clone(); - ApubObject::read_from_apub_id(id, data).await + ApubObject::read_from_apub_id(*id, data).await } async fn dereference_from_http( @@ -181,7 +182,7 @@ where for<'de2> ::ApubType: serde::Deserialize<'de2>, { fn from(id: ObjectId) -> Self { - id.0 + *id.0 } } diff --git a/crates/apub_lib/src/signatures.rs b/crates/apub_lib/src/signatures.rs index df9590685..ccc720815 100644 --- a/crates/apub_lib/src/signatures.rs +++ b/crates/apub_lib/src/signatures.rs @@ -91,7 +91,7 @@ pub fn verify_signature(request: &HttpRequest, public_key: &str) -> Result<(), L #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct PublicKey { - pub id: String, - pub owner: Url, + pub(crate) id: String, + pub(crate) owner: Box, pub public_key_pem: String, } diff --git a/crates/apub_lib/src/traits.rs b/crates/apub_lib/src/traits.rs index 38b2d685d..18b33ba65 100644 --- a/crates/apub_lib/src/traits.rs +++ b/crates/apub_lib/src/traits.rs @@ -82,7 +82,7 @@ pub trait ActorType { fn get_public_key(&self) -> Result { Ok(PublicKey { id: format!("{}#main-key", self.actor_id()), - owner: self.actor_id(), + owner: Box::new(self.actor_id()), public_key_pem: self.public_key().context(location_info!())?, }) }