From 426871f5afc28ca59a61f60890b54adafd17e2fd Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 26 Jul 2023 16:26:22 +0200 Subject: [PATCH] Use anyhow::Error for UrlVerifier return type (fixes #61) (#65) * Use anyhow::Error for UrlVerifier return type (fixes #61) * fmt * uncomment --- examples/local_federation/instance.rs | 4 +-- src/config.rs | 35 +++++++++++++++------------ src/error.rs | 4 +-- src/protocol/verification.rs | 5 ++-- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/examples/local_federation/instance.rs b/examples/local_federation/instance.rs index 51311e3..5a9794c 100644 --- a/examples/local_federation/instance.rs +++ b/examples/local_federation/instance.rs @@ -49,9 +49,9 @@ struct MyUrlVerifier(); #[async_trait] impl UrlVerifier for MyUrlVerifier { - async fn verify(&self, url: &Url) -> Result<(), &'static str> { + async fn verify(&self, url: &Url) -> Result<(), anyhow::Error> { if url.domain() == Some("malicious.com") { - Err("malicious domain") + Err(anyhow!("malicious domain")) } else { Ok(()) } diff --git a/src/config.rs b/src/config.rs index bd817fa..bd2f675 100644 --- a/src/config.rs +++ b/src/config.rs @@ -21,7 +21,7 @@ use crate::{ protocol::verification::verify_domains_match, traits::{ActivityHandler, Actor}, }; -use anyhow::Context; +use anyhow::{anyhow, Context}; use async_trait::async_trait; use derive_builder::Builder; use dyn_clone::{clone_trait_object, DynClone}; @@ -114,9 +114,9 @@ impl FederationConfig { verify_domains_match(activity.id(), activity.actor())?; self.verify_url_valid(activity.id()).await?; if self.is_local_url(activity.id()) { - return Err(Error::UrlVerificationError( - "Activity was sent from local instance", - )); + return Err(Error::UrlVerificationError(anyhow!( + "Activity was sent from local instance" + ))); } Ok(()) @@ -139,12 +139,12 @@ impl FederationConfig { "https" => {} "http" => { if !self.allow_http_urls { - return Err(Error::UrlVerificationError( - "Http urls are only allowed in debug mode", - )); + return Err(Error::UrlVerificationError(anyhow!( + "Http urls are only allowed in debug mode" + ))); } } - _ => return Err(Error::UrlVerificationError("Invalid url scheme")), + _ => return Err(Error::UrlVerificationError(anyhow!("Invalid url scheme"))), }; // Urls which use our local domain are not a security risk, no further verification needed @@ -153,13 +153,15 @@ impl FederationConfig { } if url.domain().is_none() { - return Err(Error::UrlVerificationError("Url must have a domain")); + return Err(Error::UrlVerificationError(anyhow!( + "Url must have a domain" + ))); } if url.domain() == Some("localhost") && !self.debug { - return Err(Error::UrlVerificationError( - "Localhost is only allowed in debug mode", - )); + return Err(Error::UrlVerificationError(anyhow!( + "Localhost is only allowed in debug mode" + ))); } self.url_verifier @@ -258,6 +260,7 @@ impl Deref for FederationConfig { /// # use async_trait::async_trait; /// # use url::Url; /// # use activitypub_federation::config::UrlVerifier; +/// # use anyhow::anyhow; /// # #[derive(Clone)] /// # struct DatabaseConnection(); /// # async fn get_blocklist(_: &DatabaseConnection) -> Vec { @@ -270,11 +273,11 @@ impl Deref for FederationConfig { /// /// #[async_trait] /// impl UrlVerifier for Verifier { -/// async fn verify(&self, url: &Url) -> Result<(), &'static str> { +/// async fn verify(&self, url: &Url) -> Result<(), anyhow::Error> { /// let blocklist = get_blocklist(&self.db_connection).await; /// let domain = url.domain().unwrap().to_string(); /// if blocklist.contains(&domain) { -/// Err("Domain is blocked") +/// Err(anyhow!("Domain is blocked")) /// } else { /// Ok(()) /// } @@ -284,7 +287,7 @@ impl Deref for FederationConfig { #[async_trait] pub trait UrlVerifier: DynClone + Send { /// Should return Ok iff the given url is valid for processing. - async fn verify(&self, url: &Url) -> Result<(), &'static str>; + async fn verify(&self, url: &Url) -> Result<(), anyhow::Error>; } /// Default URL verifier which does nothing. @@ -293,7 +296,7 @@ struct DefaultUrlVerifier(); #[async_trait] impl UrlVerifier for DefaultUrlVerifier { - async fn verify(&self, _url: &Url) -> Result<(), &'static str> { + async fn verify(&self, _url: &Url) -> Result<(), anyhow::Error> { Ok(()) } } diff --git a/src/error.rs b/src/error.rs index 22b0401..91de96a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -16,8 +16,8 @@ pub enum Error { #[error("Object to be fetched was deleted")] ObjectDeleted, /// url verification error - #[error("{0}")] - UrlVerificationError(&'static str), + #[error("URL failed verification: {0}")] + UrlVerificationError(anyhow::Error), /// Incoming activity has invalid digest for body #[error("Incoming activity has invalid digest for body")] ActivityBodyDigestInvalid, diff --git a/src/protocol/verification.rs b/src/protocol/verification.rs index 18595b9..3383bd9 100644 --- a/src/protocol/verification.rs +++ b/src/protocol/verification.rs @@ -1,6 +1,7 @@ //! Verify that received data is valid use crate::error::Error; +use anyhow::anyhow; use url::Url; /// Check that both urls have the same domain. If not, return UrlVerificationError. @@ -15,7 +16,7 @@ use url::Url; /// ``` pub fn verify_domains_match(a: &Url, b: &Url) -> Result<(), Error> { if a.domain() != b.domain() { - return Err(Error::UrlVerificationError("Domains do not match")); + return Err(Error::UrlVerificationError(anyhow!("Domains do not match"))); } Ok(()) } @@ -32,7 +33,7 @@ pub fn verify_domains_match(a: &Url, b: &Url) -> Result<(), Error> { /// ``` pub fn verify_urls_match(a: &Url, b: &Url) -> Result<(), Error> { if a != b { - return Err(Error::UrlVerificationError("Urls do not match")); + return Err(Error::UrlVerificationError(anyhow!("Urls do not match"))); } Ok(()) }