From b64f4a8f3fd15fed1a0e122eeaaffb65ea5b2ad2 Mon Sep 17 00:00:00 2001 From: phiresky Date: Thu, 29 Jun 2023 10:19:49 +0200 Subject: [PATCH] fix: make "other" error actually transparent (#51) * fix: make "other" error actually transparent * cargo fmt --- Cargo.toml | 1 - examples/live_federation/objects/post.rs | 7 ++----- examples/local_federation/activities/accept.rs | 5 +---- examples/local_federation/activities/follow.rs | 4 +--- examples/local_federation/axum/http.rs | 3 +-- src/activity_queue.rs | 9 +++++---- src/actix_web/inbox.rs | 4 +++- src/actix_web/middleware.rs | 5 +---- src/error.rs | 16 +++++++++++----- src/fetch/mod.rs | 5 +---- src/http_signatures.rs | 11 ++++++++--- 11 files changed, 34 insertions(+), 36 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2c8f302..90ef2d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,6 @@ axum = { version = "0.6.18", features = [ ], default-features = false, optional = true } tower = { version = "0.4.13", optional = true } hyper = { version = "0.14", optional = true } -displaydoc = "0.2.4" [features] default = ["actix-web", "axum"] diff --git a/examples/live_federation/objects/post.rs b/examples/live_federation/objects/post.rs index 9a08b9d..44012b1 100644 --- a/examples/live_federation/objects/post.rs +++ b/examples/live_federation/objects/post.rs @@ -1,9 +1,6 @@ use crate::{ - activities::create_post::CreatePost, - database::DatabaseHandle, - error::Error, - generate_object_id, - objects::person::DbUser, + activities::create_post::CreatePost, database::DatabaseHandle, error::Error, + generate_object_id, objects::person::DbUser, }; use activitypub_federation::{ config::Data, diff --git a/examples/local_federation/activities/accept.rs b/examples/local_federation/activities/accept.rs index c18945f..9305213 100644 --- a/examples/local_federation/activities/accept.rs +++ b/examples/local_federation/activities/accept.rs @@ -1,9 +1,6 @@ use crate::{activities::follow::Follow, instance::DatabaseHandle, objects::person::DbUser}; use activitypub_federation::{ - config::Data, - fetch::object_id::ObjectId, - kinds::activity::AcceptType, - traits::ActivityHandler, + config::Data, fetch::object_id::ObjectId, kinds::activity::AcceptType, traits::ActivityHandler, }; use serde::{Deserialize, Serialize}; use url::Url; diff --git a/examples/local_federation/activities/follow.rs b/examples/local_federation/activities/follow.rs index 865a618..51e8ee1 100644 --- a/examples/local_federation/activities/follow.rs +++ b/examples/local_federation/activities/follow.rs @@ -1,7 +1,5 @@ use crate::{ - activities::accept::Accept, - generate_object_id, - instance::DatabaseHandle, + activities::accept::Accept, generate_object_id, instance::DatabaseHandle, objects::person::DbUser, }; use activitypub_federation::{ diff --git a/examples/local_federation/axum/http.rs b/examples/local_federation/axum/http.rs index 3202117..fd3fba9 100644 --- a/examples/local_federation/axum/http.rs +++ b/examples/local_federation/axum/http.rs @@ -17,8 +17,7 @@ use axum::{ extract::{Path, Query}, response::IntoResponse, routing::{get, post}, - Json, - Router, + Json, Router, }; use axum_macros::debug_handler; use serde::Deserialize; diff --git a/src/activity_queue.rs b/src/activity_queue.rs index 6e4ad99..2c60662 100644 --- a/src/activity_queue.rs +++ b/src/activity_queue.rs @@ -10,7 +10,7 @@ use crate::{ traits::{ActivityHandler, Actor}, FEDERATION_CONTENT_TYPE, }; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use bytes::Bytes; use futures_core::Future; @@ -77,14 +77,14 @@ where .unique() .filter(|i| !config.is_local_url(i)) .collect(); - // This field is only optional to make builder work, its always present at this point let activity_queue = config .activity_queue .as_ref() .expect("Config has activity queue"); for inbox in inboxes { - if config.verify_url_valid(&inbox).await.is_err() { + if let Err(err) = config.verify_url_valid(&inbox).await { + debug!("inbox url invalid, skipping: {inbox}: {err}"); continue; } @@ -166,7 +166,8 @@ async fn sign_and_send( task.private_key.clone(), task.http_signature_compat, ) - .await?; + .await + .context("signing request")?; retry( || { diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs index e4d83e9..d4d1abf 100644 --- a/src/actix_web/inbox.rs +++ b/src/actix_web/inbox.rs @@ -8,6 +8,7 @@ use crate::{ traits::{ActivityHandler, Actor, Object}, }; use actix_web::{web::Bytes, HttpRequest, HttpResponse}; +use anyhow::Context; use serde::de::DeserializeOwned; use tracing::debug; @@ -32,7 +33,8 @@ where { verify_body_hash(request.headers().get("Digest"), &body)?; - let activity: Activity = serde_json::from_slice(&body)?; + let activity: Activity = serde_json::from_slice(&body) + .with_context(|| format!("deserializing body: {}", String::from_utf8_lossy(&body)))?; data.config.verify_url_and_domain(&activity).await?; let actor = ObjectId::::from(activity.actor().clone()) .dereference(data) diff --git a/src/actix_web/middleware.rs b/src/actix_web/middleware.rs index afa0117..2bb562b 100644 --- a/src/actix_web/middleware.rs +++ b/src/actix_web/middleware.rs @@ -1,10 +1,7 @@ use crate::config::{Data, FederationConfig, FederationMiddleware}; use actix_web::{ dev::{forward_ready, Payload, Service, ServiceRequest, ServiceResponse, Transform}, - Error, - FromRequest, - HttpMessage, - HttpRequest, + Error, FromRequest, HttpMessage, HttpRequest, }; use std::future::{ready, Ready}; diff --git a/src/error.rs b/src/error.rs index 0cb7ed1..22b0401 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,27 +1,33 @@ //! Error messages returned by this library -use displaydoc::Display; - /// Error messages returned by this library -#[derive(thiserror::Error, Debug, Display)] +#[derive(thiserror::Error, Debug)] pub enum Error { /// Object was not found in local database + #[error("Object was not found in local database")] NotFound, /// Request limit was reached during fetch + #[error("Request limit was reached during fetch")] RequestLimit, /// Response body limit was reached during fetch + #[error("Response body limit was reached during fetch")] ResponseBodyLimit, /// Object to be fetched was deleted + #[error("Object to be fetched was deleted")] ObjectDeleted, - /// {0} + /// url verification error + #[error("{0}")] UrlVerificationError(&'static str), /// Incoming activity has invalid digest for body + #[error("Incoming activity has invalid digest for body")] ActivityBodyDigestInvalid, /// Incoming activity has invalid signature + #[error("Incoming activity has invalid signature")] ActivitySignatureInvalid, /// Failed to resolve actor via webfinger + #[error("Failed to resolve actor via webfinger")] WebfingerResolveFailed, - /// Other errors which are not explicitly handled + /// other error #[error(transparent)] Other(#[from] anyhow::Error), } diff --git a/src/fetch/mod.rs b/src/fetch/mod.rs index 7a9734c..5faadcd 100644 --- a/src/fetch/mod.rs +++ b/src/fetch/mod.rs @@ -3,10 +3,7 @@ #![doc = include_str!("../../docs/07_fetching_data.md")] use crate::{ - config::Data, - error::Error, - http_signatures::sign_request, - reqwest_shim::ResponseExt, + config::Data, error::Error, http_signatures::sign_request, reqwest_shim::ResponseExt, FEDERATION_CONTENT_TYPE, }; use bytes::Bytes; diff --git a/src/http_signatures.rs b/src/http_signatures.rs index 19cf319..0b9d06c 100644 --- a/src/http_signatures.rs +++ b/src/http_signatures.rs @@ -12,6 +12,7 @@ use crate::{ protocol::public_key::main_key_id, traits::{Actor, Object}, }; +use anyhow::Context; use base64::{engine::general_purpose::STANDARD as Base64, Engine}; use bytes::Bytes; use http::{header::HeaderName, uri::PathAndQuery, HeaderValue, Method, Uri}; @@ -102,10 +103,14 @@ pub(crate) async fn sign_request( Sha256::new(), activity, move |signing_string| { - let mut signer = Signer::new(MessageDigest::sha256(), &private_key)?; - signer.update(signing_string.as_bytes())?; + let mut signer = Signer::new(MessageDigest::sha256(), &private_key) + .context("instantiating signer")?; + signer + .update(signing_string.as_bytes()) + .context("updating signer")?; - Ok(Base64.encode(signer.sign_to_vec()?)) as Result<_, anyhow::Error> + Ok(Base64.encode(signer.sign_to_vec().context("sign to vec")?)) + as Result<_, anyhow::Error> }, ) .await