From 42dbc6375b9692c6762714cf3b4fc912dc6f9994 Mon Sep 17 00:00:00 2001
From: Felix Ableitner <me@nutomic.com>
Date: Tue, 12 Dec 2023 00:40:57 +0100
Subject: [PATCH] Better error when receive fails

---
 src/actix_web/inbox.rs |  9 +++------
 src/axum/inbox.rs      | 13 ++++---------
 src/error.rs           |  3 +++
 src/lib.rs             | 39 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs
index e7e19e8..ac6ebb8 100644
--- a/src/actix_web/inbox.rs
+++ b/src/actix_web/inbox.rs
@@ -3,8 +3,8 @@
 use crate::{
     config::Data,
     error::Error,
-    fetch::object_id::ObjectId,
     http_signatures::{verify_body_hash, verify_signature},
+    parse_received_activity,
     traits::{ActivityHandler, Actor, Object},
 };
 use actix_web::{web::Bytes, HttpRequest, HttpResponse};
@@ -29,11 +29,7 @@ where
 {
     verify_body_hash(request.headers().get("Digest"), &body)?;
 
-    let activity: Activity = serde_json::from_slice(&body).map_err(Error::Json)?;
-    data.config.verify_url_and_domain(&activity).await?;
-    let actor = ObjectId::<ActorT>::from(activity.actor().clone())
-        .dereference(data)
-        .await?;
+    let (activity, actor) = parse_received_activity::<Activity, ActorT, _>(&body, data).await?;
 
     verify_signature(
         request.headers(),
@@ -54,6 +50,7 @@ mod test {
     use crate::{
         activity_sending::generate_request_headers,
         config::FederationConfig,
+        fetch::object_id::ObjectId,
         http_signatures::sign_request,
         traits::tests::{DbConnection, DbUser, Follow, DB_USER_KEYPAIR},
     };
diff --git a/src/axum/inbox.rs b/src/axum/inbox.rs
index c110bb6..5bb147a 100644
--- a/src/axum/inbox.rs
+++ b/src/axum/inbox.rs
@@ -5,8 +5,8 @@
 use crate::{
     config::Data,
     error::Error,
-    fetch::object_id::ObjectId,
-    http_signatures::{verify_body_hash, verify_signature},
+    http_signatures::verify_signature,
+    parse_received_activity,
     traits::{ActivityHandler, Actor, Object},
 };
 use axum::{
@@ -33,13 +33,8 @@ where
     <ActorT as Object>::Error: From<Error>,
     Datatype: Clone,
 {
-    verify_body_hash(activity_data.headers.get("Digest"), &activity_data.body)?;
-
-    let activity: Activity = serde_json::from_slice(&activity_data.body).map_err(Error::Json)?;
-    data.config.verify_url_and_domain(&activity).await?;
-    let actor = ObjectId::<ActorT>::from(activity.actor().clone())
-        .dereference(data)
-        .await?;
+    let (activity, actor) =
+        parse_received_activity::<Activity, ActorT, _>(&activity_data.body, data).await?;
 
     verify_signature(
         &activity_data.headers,
diff --git a/src/error.rs b/src/error.rs
index 2d179d1..89f6abf 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -38,6 +38,9 @@ pub enum Error {
     /// JSON Error
     #[error(transparent)]
     Json(#[from] serde_json::Error),
+    /// Failed to parse an activity received from another instance
+    #[error("Failed to parse incoming activity with id {0}: {1}")]
+    ParseReceivedActivity(Url, serde_json::Error),
     /// Reqwest Middleware Error
     #[error(transparent)]
     ReqwestMiddleware(#[from] reqwest_middleware::Error),
diff --git a/src/lib.rs b/src/lib.rs
index c660253..b28363b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -23,7 +23,46 @@ pub mod protocol;
 pub(crate) mod reqwest_shim;
 pub mod traits;
 
+use crate::{
+    config::Data,
+    error::Error,
+    fetch::object_id::ObjectId,
+    traits::{ActivityHandler, Actor, Object},
+};
 pub use activitystreams_kinds as kinds;
 
+use serde::{de::DeserializeOwned, Deserialize};
+use url::Url;
+
 /// Mime type for Activitypub data, used for `Accept` and `Content-Type` HTTP headers
 pub static FEDERATION_CONTENT_TYPE: &str = "application/activity+json";
+
+async fn parse_received_activity<Activity, ActorT, Datatype>(
+    body: &[u8],
+    data: &Data<Datatype>,
+) -> Result<(Activity, ActorT), <Activity as ActivityHandler>::Error>
+where
+    Activity: ActivityHandler<DataType = Datatype> + DeserializeOwned + Send + 'static,
+    ActorT: Object<DataType = Datatype> + Actor + Send + 'static,
+    for<'de2> <ActorT as Object>::Kind: serde::Deserialize<'de2>,
+    <Activity as ActivityHandler>::Error: From<Error> + From<<ActorT as Object>::Error>,
+    <ActorT as Object>::Error: From<Error>,
+    Datatype: Clone,
+{
+    let activity: Activity = serde_json::from_slice(&body).map_err(|e| {
+        // Attempt to parse only activity id for error message
+        #[derive(Deserialize)]
+        struct Id {
+            id: Url,
+        }
+        match serde_json::from_slice::<Id>(&body) {
+            Ok(id) => Error::ParseReceivedActivity(id.id, e),
+            Err(e) => Error::Json(e),
+        }
+    })?;
+    data.config.verify_url_and_domain(&activity).await?;
+    let actor = ObjectId::<ActorT>::from(activity.actor().clone())
+        .dereference(data)
+        .await?;
+    Ok((activity, actor))
+}