From 99cdf742d4366e835fd827749eb6638d5f56ba9b Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 25 Sep 2024 13:21:20 +0200 Subject: [PATCH] Ignore NotFound errors when receiving activities (fixes #2240) --- Cargo.lock | 1 + crates/apub/src/http/mod.rs | 15 +++++++++++---- crates/utils/Cargo.toml | 2 ++ crates/utils/src/error.rs | 28 ++++++++++++++++++++-------- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a799d9d6f..c49ae5274 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2795,6 +2795,7 @@ dependencies = [ name = "lemmy_utils" version = "0.19.6-beta.7" dependencies = [ + "activitypub_federation", "actix-web", "anyhow", "cfg-if", diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index dad39881f..ed3026e6e 100644 --- a/crates/apub/src/http/mod.rs +++ b/crates/apub/src/http/mod.rs @@ -17,7 +17,7 @@ use lemmy_db_schema::{ source::{activity::SentActivity, community::Community}, CommunityVisibility, }; -use lemmy_utils::error::{LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult}; use serde::{Deserialize, Serialize}; use std::{ops::Deref, time::Duration}; use tokio::time::timeout; @@ -43,9 +43,16 @@ pub async fn shared_inbox( // avoid taking a long time to process an incoming activity when a required data fetch times out. // In this case our own instance would timeout and be marked as dead by the sender. Better to // consider the activity broken and move on. - timeout(INCOMING_ACTIVITY_TIMEOUT, receive_fut) - .await - .map_err(|_| LemmyErrorType::InboxTimeout)? + let res = timeout(INCOMING_ACTIVITY_TIMEOUT, receive_fut).await; + + match res { + // Ignore NotFound error, usually means that the sending actor was deleted and sent a delete + // activity: https://github.com/LemmyNet/lemmy/issues/2240 + Ok(Err(LemmyError {error_type: LemmyErrorType::NotFound, ..})) => Ok(HttpResponse::Ok().finish()), + // Top-level error means we hit the send timeout + Err(_) => Err(LemmyErrorType::InboxTimeout.into()), + Ok(other) => other, + } } /// Convert the data to json and turn it into an HTTP Response with the correct ActivityPub diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index f0369f7e9..6f6059160 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -47,6 +47,7 @@ full = [ "dep:uuid", "dep:itertools", "dep:markdown-it", + "dep:activitypub_federation" ] [dependencies] @@ -80,6 +81,7 @@ lettre = { version = "0.11.8", default-features = false, features = [ markdown-it = { version = "0.6.1", optional = true } ts-rs = { workspace = true, optional = true } enum-map = { workspace = true, optional = true } +activitypub_federation = { workspace = true, optional = true } cfg-if = "1" clearurls = { version = "0.0.4", features = ["linkify"] } diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index e03ff2e23..b0cfcc9ce 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -198,10 +198,14 @@ cfg_if! { { fn from(t: T) -> Self { let cause = t.into(); - let error_type = match cause.downcast_ref::() { - Some(&diesel::NotFound) => LemmyErrorType::NotFound, - _ => LemmyErrorType::Unknown(format!("{}", &cause)) - }; + let error_type = if let Some(&diesel::NotFound) = cause.downcast_ref::() { + LemmyErrorType::NotFound + } else if let Some(activitypub_federation::error::Error::NotFound) = cause.downcast_ref::() { + LemmyErrorType::NotFound + } + else { + LemmyErrorType::Unknown(format!("{}", &cause)) + }; LemmyError { error_type, inner: cause, @@ -231,11 +235,15 @@ cfg_if! { impl actix_web::error::ResponseError for LemmyError { fn status_code(&self) -> actix_web::http::StatusCode { if self.error_type == LemmyErrorType::IncorrectLogin { - return actix_web::http::StatusCode::UNAUTHORIZED; + actix_web::http::StatusCode::UNAUTHORIZED } - match self.inner.downcast_ref::() { - Some(diesel::result::Error::NotFound) => actix_web::http::StatusCode::NOT_FOUND, - _ => actix_web::http::StatusCode::BAD_REQUEST, + else if let Some(&diesel::NotFound) = self.inner.downcast_ref::() { + actix_web::http::StatusCode::NOT_FOUND + } else if let Some(activitypub_federation::error::Error::NotFound) = self.inner.downcast_ref::() { + actix_web::http::StatusCode::NOT_FOUND + } + else { + actix_web::http::StatusCode::BAD_REQUEST } } @@ -320,6 +328,10 @@ cfg_if! { assert_eq!(LemmyErrorType::NotFound, not_found_error.error_type); assert_eq!(404, not_found_error.status_code()); + let apub_not_found_error = LemmyError::from(activitypub_federation::error::Error::NotFound); + assert_eq!(LemmyErrorType::NotFound, apub_not_found_error.error_type); + assert_eq!(404, apub_not_found_error.status_code()); + let other_error = LemmyError::from(diesel::result::Error::NotInTransaction); assert!(matches!(other_error.error_type, LemmyErrorType::Unknown{..})); assert_eq!(400, other_error.status_code());