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

@@ -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(())
         }

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<T: Clone> FederationConfig<T> {
         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(
+            return Err(Error::UrlVerificationError(anyhow!(
-                "Activity was sent from local instance",
+                "Activity was sent from local instance"
-            ));
+            )));
         }
 
         Ok(())
@@ -139,12 +139,12 @@ impl<T: Clone> FederationConfig<T> {
             "https" => {}
             "http" => {
                 if !self.allow_http_urls {
-                    return Err(Error::UrlVerificationError(
+                    return Err(Error::UrlVerificationError(anyhow!(
-                        "Http urls are only allowed in debug mode",
+                        "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<T: Clone> FederationConfig<T> {
         }
 
         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(
+            return Err(Error::UrlVerificationError(anyhow!(
-                "Localhost is only allowed in debug mode",
+                "Localhost is only allowed in debug mode"
-            ));
+            )));
         }
 
         self.url_verifier
@@ -258,6 +260,7 @@ impl<T: Clone> Deref for FederationConfig<T> {
 /// # 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<String> {
@@ -270,11 +273,11 @@ impl<T: Clone> Deref for FederationConfig<T> {
 ///
 /// #[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<T: Clone> Deref for FederationConfig<T> {
 #[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(())
     }
 }

src/error.rs

@@ -16,8 +16,8 @@ pub enum Error {
     #[error("Object to be fetched was deleted")]
     ObjectDeleted,
     /// url verification error
-    #[error("{0}")]
+    #[error("URL failed verification: {0}")]
-    UrlVerificationError(&'static str),
+    UrlVerificationError(anyhow::Error),
     /// Incoming activity has invalid digest for body
     #[error("Incoming activity has invalid digest for body")]
     ActivityBodyDigestInvalid,

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(())
 }