Use boxed dyn callbacks to get rid of some type parameters (#1215)

* refactor: get rid of database type parameter on RequestAuthorizationHandler

this is step 1 of boxing this thing.

rationale: Databases are cheaply cloneable, so if you need access to a
database in your RequestAuthorizationHandler, just inject a clone during
creation.

In any case, the database you want for auth is probably not the database
that contains the blobs.

* refactor: also remove db parameter for CustomGetRequest

...and take advantage of the whole thing to remove 2 type parameters from
the builder.

we are going to need moar type parameters for something else, but here they
are not pulling their weight.

* refactor: move default impls from iroh-bytes into iroh and make them private

since we are using Arc<dyn ...> the type of the dummy handler does not need
to be public.

iroh-bytes/src/provider.rs

@@ -1,14 +1,18 @@
 //! Provider API
 
 use std::borrow::Cow;
+use std::fmt::Debug;
 use std::io;
 use std::path::{Path, PathBuf};
+use std::sync::Arc;
 
 use anyhow::{ensure, Context, Result};
 use bao_tree::io::fsm::{encode_ranges_validated, Outboard};
 use bytes::{Bytes, BytesMut};
-use futures::future::{BoxFuture, Either};
-use futures::{Future, FutureExt};
+use futures::{
+    future::{BoxFuture, Either},
+    Future,
+};
 use iroh_io::{AsyncSliceReaderExt, File};
 use serde::{Deserialize, Serialize};
 use tokio::io::AsyncWrite;
@@ -187,62 +191,26 @@ pub enum ProvideProgress {
 /// hook into the request handling to process authorization by examining
 /// the request and any given token. Any error returned will abort the request,
 /// and the error will be sent to the requester.
-pub trait RequestAuthorizationHandler<D>: Send + Sync + Clone + 'static {
+pub trait RequestAuthorizationHandler: Send + Sync + Debug + 'static {
     /// Handle the authorization request, given an opaque data blob from the requester.
     fn authorize(
         &self,
-        db: D,
         token: Option<RequestToken>,
         request: &Request,
     ) -> BoxFuture<'static, anyhow::Result<()>>;
 }
 
-/// Define RequestAuthorizationHandler for () so we can use it as a no-op default.
-impl<D> RequestAuthorizationHandler<D> for () {
-    fn authorize(
-        &self,
-        _db: D,
-        token: Option<RequestToken>,
-        _request: &Request,
-    ) -> BoxFuture<'static, anyhow::Result<()>> {
-        async move {
-            if let Some(token) = token {
-                anyhow::bail!(
-                    "no authorization handler defined, but token was provided: {:?}",
-                    token
-                );
-            }
-            Ok(())
-        }
-        .boxed()
-    }
-}
-
 /// A custom get request handler that allows the user to make up a get request
 /// on the fly.
-pub trait CustomGetHandler<D>: Send + Sync + Clone + 'static {
+pub trait CustomGetHandler: Send + Sync + Debug + 'static {
     /// Handle the custom request, given an opaque data blob from the requester.
     fn handle(
         &self,
         token: Option<RequestToken>,
         request: Bytes,
-        db: D,
     ) -> BoxFuture<'static, anyhow::Result<GetRequest>>;
 }
 
-/// Handle the custom request, given an opaque data blob from the requester.
-/// Define CustomGetHandler for () so we can use it as a no-op default.
-impl<D> CustomGetHandler<D> for () {
-    fn handle(
-        &self,
-        _token: Option<RequestToken>,
-        _request: Bytes,
-        _db: D,
-    ) -> BoxFuture<'static, anyhow::Result<GetRequest>> {
-        async move { Err(anyhow::anyhow!("no custom get handler defined")) }.boxed()
-    }
-}
-
 /// A [`Database`] entry.
 ///
 /// This is either stored externally in the file system, or internally in the database.
@@ -462,17 +430,12 @@ pub trait EventSender: Clone + Send + 'static {
 }
 
 /// Handle a single connection.
-pub async fn handle_connection<
-    D: BaoMap,
-    C: CustomGetHandler<D>,
-    E: EventSender,
-    A: RequestAuthorizationHandler<D>,
->(
+pub async fn handle_connection<D: BaoMap, E: EventSender>(
     connecting: quinn::Connecting,
     db: D,
     events: E,
-    custom_get_handler: C,
-    authorization_handler: A,
+    custom_get_handler: Arc<dyn CustomGetHandler>,
+    authorization_handler: Arc<dyn RequestAuthorizationHandler>,
     rt: crate::runtime::Handle,
 ) {
     let remote_addr = connecting.remote_address();
@@ -526,8 +489,8 @@ async fn handle_stream<D: BaoMap, E: EventSender>(
     db: D,
     reader: quinn::RecvStream,
     writer: ResponseWriter<E>,
-    custom_get_handler: impl CustomGetHandler<D>,
-    authorization_handler: impl RequestAuthorizationHandler<D>,
+    custom_get_handler: Arc<dyn CustomGetHandler>,
+    authorization_handler: Arc<dyn RequestAuthorizationHandler>,
 ) -> Result<()> {
     let mut in_buffer = BytesMut::with_capacity(1024);
 
@@ -544,7 +507,7 @@ async fn handle_stream<D: BaoMap, E: EventSender>(
     // 2. Authorize the request (may be a no-op)
     debug!("authorizing request");
     if let Err(e) = authorization_handler
-        .authorize(db.clone(), request.token().cloned(), &request)
+        .authorize(request.token().cloned(), &request)
         .await
     {
         writer.notify_transfer_aborted();
@@ -562,7 +525,7 @@ async fn handle_custom_get<E: EventSender, D: BaoMap>(
     db: D,
     request: CustomGetRequest,
     mut writer: ResponseWriter<E>,
-    custom_get_handler: impl CustomGetHandler<D>,
+    custom_get_handler: Arc<dyn CustomGetHandler>,
 ) -> Result<()> {
     let _ = writer.events.send(Event::CustomGetRequestReceived {
         len: request.data.len(),
@@ -572,7 +535,7 @@ async fn handle_custom_get<E: EventSender, D: BaoMap>(
     });
     // try to make a GetRequest from the custom bytes
     let request = custom_get_handler
-        .handle(request.token, request.data, db.clone())
+        .handle(request.token, request.data)
         .await?;
     // write it to the requester as the first thing
     let data = postcard::to_stdvec(&request)?;

iroh/Cargo.toml

@@ -16,6 +16,7 @@ rust-version = "1.65"
 anyhow = { version = "1", features = ["backtrace"] }
 bao-tree = { version = "0.5.0", features = ["tokio_fsm"], default-features = false }
 blake3 = "1.3.3"
+bytes = "1"
 derive_more = { package = "derive_more_preview", version = "0.1.0", features = ["debug", "display", "from", "try_into"] }
 flume = "0.10.14"
 futures = "0.3.25"

iroh/src/commands/provide.rs

@@ -3,6 +3,7 @@ use std::{
     net::{Ipv4Addr, SocketAddr, SocketAddrV4},
     path::PathBuf,
     str::FromStr,
+    sync::Arc,
 };
 
 use anyhow::{ensure, Context, Result};
@@ -127,7 +128,7 @@ async fn provide(
     let keypair = get_keypair(key).await?;
 
     let mut builder = Node::builder(db)
-        .custom_auth_handler(StaticTokenAuthHandler::new(opts.request_token))
+        .custom_auth_handler(Arc::new(StaticTokenAuthHandler::new(opts.request_token)))
         .keylog(opts.keylog);
     if let Some(dm) = opts.derp_map {
         builder = builder.derp_map(dm);

iroh/src/node.rs

@@ -16,8 +16,10 @@ use std::task::Poll;
 use std::time::Duration;
 
 use anyhow::{Context, Result};
+use bytes::Bytes;
 use futures::future::{BoxFuture, Shared};
 use futures::{FutureExt, Stream, StreamExt, TryFutureExt};
+use iroh_bytes::protocol::GetRequest;
 use iroh_bytes::{
     blobs::Collection,
     protocol::{Closed, Request, RequestToken},
@@ -70,26 +72,63 @@ const ENDPOINT_WAIT: Duration = Duration::from_secs(5);
 /// The returned [`Node`] is awaitable to know when it finishes.  It can be terminated
 /// using [`Node::shutdown`].
 #[derive(Debug)]
-pub struct Builder<D = Database, E = DummyServerEndpoint, C = (), A = ()>
+pub struct Builder<D = Database, E = DummyServerEndpoint>
 where
     D: BaoMap,
     E: ServiceEndpoint<ProviderService>,
-    C: CustomGetHandler<D>,
-    A: RequestAuthorizationHandler<D>,
 {
     bind_addr: SocketAddr,
     keypair: Keypair,
     rpc_endpoint: E,
     db: D,
     keylog: bool,
-    custom_get_handler: C,
-    auth_handler: A,
+    custom_get_handler: Arc<dyn CustomGetHandler>,
+    auth_handler: Arc<dyn RequestAuthorizationHandler>,
     derp_map: Option<DerpMap>,
     rt: Option<runtime::Handle>,
 }
 
 const PROTOCOLS: [&[u8]; 1] = [&iroh_bytes::protocol::ALPN];
 
+/// A noop authorization handler that does not do any authorization.
+///
+/// This is the default. It does not have to be pub, since it is going to be
+/// boxed.
+#[derive(Debug)]
+struct NoopRequestAuthorizationHandler;
+
+impl RequestAuthorizationHandler for NoopRequestAuthorizationHandler {
+    fn authorize(
+        &self,
+        token: Option<RequestToken>,
+        _request: &Request,
+    ) -> BoxFuture<'static, anyhow::Result<()>> {
+        async move {
+            if let Some(token) = token {
+                anyhow::bail!(
+                    "no authorization handler defined, but token was provided: {:?}",
+                    token
+                );
+            }
+            Ok(())
+        }
+        .boxed()
+    }
+}
+
+#[derive(Debug)]
+struct NoopCustomGetHandler;
+
+impl CustomGetHandler for NoopCustomGetHandler {
+    fn handle(
+        &self,
+        _token: Option<RequestToken>,
+        _request: Bytes,
+    ) -> BoxFuture<'static, anyhow::Result<GetRequest>> {
+        async move { Err(anyhow::anyhow!("no custom get handler defined")) }.boxed()
+    }
+}
+
 impl<D: BaoMap> Builder<D> {
     /// Creates a new builder for [`Node`] using the given [`Database`].
     pub fn with_db(db: D) -> Self {
@@ -100,25 +139,20 @@ impl<D: BaoMap> Builder<D> {
             keylog: false,
             derp_map: None,
             rpc_endpoint: Default::default(),
-            custom_get_handler: Default::default(),
-            auth_handler: Default::default(),
+            custom_get_handler: Arc::new(NoopCustomGetHandler),
+            auth_handler: Arc::new(NoopRequestAuthorizationHandler),
             rt: None,
         }
     }
 }
 
-impl<E, C, A, D> Builder<D, E, C, A>
+impl<D, E> Builder<D, E>
 where
     D: BaoReadonlyDb,
     E: ServiceEndpoint<ProviderService>,
-    C: CustomGetHandler<D>,
-    A: RequestAuthorizationHandler<D>,
 {
     /// Configure rpc endpoint, changing the type of the builder to the new endpoint type.
-    pub fn rpc_endpoint<E2: ServiceEndpoint<ProviderService>>(
-        self,
-        value: E2,
-    ) -> Builder<D, E2, C, A> {
+    pub fn rpc_endpoint<E2: ServiceEndpoint<ProviderService>>(self, value: E2) -> Builder<D, E2> {
         // we can't use ..self here because the return type is different
         Builder {
             bind_addr: self.bind_addr,
@@ -139,41 +173,19 @@ where
         self
     }
 
-    /// Configure the custom get handler, changing the type of the builder to the new handler type.
-    pub fn custom_get_handler<C2: CustomGetHandler<D>>(
-        self,
-        custom_handler: C2,
-    ) -> Builder<D, E, C2, A> {
-        // we can't use ..self here because the return type is different
-        Builder {
-            bind_addr: self.bind_addr,
-            keypair: self.keypair,
-            db: self.db,
-            keylog: self.keylog,
-            rpc_endpoint: self.rpc_endpoint,
-            custom_get_handler: custom_handler,
-            auth_handler: self.auth_handler,
-            derp_map: self.derp_map,
-            rt: self.rt,
+    /// Configure the custom get handler.
+    pub fn custom_get_handler(self, custom_get_handler: Arc<dyn CustomGetHandler>) -> Self {
+        Self {
+            custom_get_handler,
+            ..self
         }
     }
 
     /// Configures a custom authorization handler.
-    pub fn custom_auth_handler<A2: RequestAuthorizationHandler<D>>(
-        self,
-        auth_handler: A2,
-    ) -> Builder<D, E, C, A2> {
-        // we can't use ..self here because the return type is different
-        Builder {
-            bind_addr: self.bind_addr,
-            keypair: self.keypair,
-            db: self.db,
-            keylog: self.keylog,
-            rpc_endpoint: self.rpc_endpoint,
-            custom_get_handler: self.custom_get_handler,
+    pub fn custom_auth_handler(self, auth_handler: Arc<dyn RequestAuthorizationHandler>) -> Self {
+        Self {
             auth_handler,
-            derp_map: self.derp_map,
-            rt: self.rt,
+            ..self
         }
     }
 
@@ -303,8 +315,8 @@ where
         handler: RpcHandler<D>,
         rpc: E,
         internal_rpc: impl ServiceEndpoint<ProviderService>,
-        custom_get_handler: C,
-        auth_handler: A,
+        custom_get_handler: Arc<dyn CustomGetHandler>,
+        auth_handler: Arc<dyn RequestAuthorizationHandler>,
         rt: runtime::Handle,
     ) {
         let rpc = RpcServer::new(rpc);
@@ -778,10 +790,9 @@ impl StaticTokenAuthHandler {
     }
 }
 
-impl<D> RequestAuthorizationHandler<D> for StaticTokenAuthHandler {
+impl RequestAuthorizationHandler for StaticTokenAuthHandler {
     fn authorize(
         &self,
-        _db: D,
         token: Option<RequestToken>,
         _request: &Request,
     ) -> BoxFuture<'static, anyhow::Result<()>> {