diff --git a/crates/tor-chanmgr/src/err.rs b/crates/tor-chanmgr/src/err.rs index 73945b411..a7c237cb2 100644 --- a/crates/tor-chanmgr/src/err.rs +++ b/crates/tor-chanmgr/src/err.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use futures::task::SpawnError; use thiserror::Error; +use crate::factory::AbstractPtError; use tor_error::{internal, ErrorKind}; use tor_linkspec::{ChanTarget, OwnedChanTarget, PtTargetAddr}; use tor_proto::ClockSkew; @@ -107,6 +108,15 @@ pub enum Error { #[error("Problem while connecting to Tor via a proxy")] Proxy(#[from] ProxyError), + /// An error occurred in a pluggable transport manager. + /// + /// We can't know the type, because any pluggable transport manager implementing + /// `AbstractPtMgr` can be used. + /// However, if you're using Arti in the standard configuration, this will be + /// `tor-ptmgr`'s `PtError`. + #[error("Pluggable transport error: {0}")] + Pt(#[source] Arc), + /// An internal error of some kind that should never occur. #[error("Internal error")] Internal(#[from] tor_error::Bug), @@ -146,6 +156,7 @@ impl tor_error::HasKind for Error { E::ChannelBuild { .. } => EK::TorAccessFailed, E::RequestCancelled => EK::TransientFailure, E::Proxy(e) => e.kind(), + E::Pt(e) => e.kind(), } } } @@ -166,6 +177,7 @@ impl tor_error::HasRetryTime for Error { // Delegate. E::Proxy(e) => e.retry_time(), + E::Pt(e) => e.retry_time(), // This error reflects multiple attempts, but every failure is an IO // error, so we can also retry this after a delay. diff --git a/crates/tor-chanmgr/src/factory.rs b/crates/tor-chanmgr/src/factory.rs index 870a30b54..092b33f7d 100644 --- a/crates/tor-chanmgr/src/factory.rs +++ b/crates/tor-chanmgr/src/factory.rs @@ -1,16 +1,13 @@ //! Traits and code to define different mechanisms for building Channels to //! different kinds of targets. -pub(crate) mod registry; - use std::sync::Arc; use async_trait::async_trait; -use tor_linkspec::OwnedChanTarget; +use tor_error::{HasKind, HasRetryTime}; +use tor_linkspec::{OwnedChanTarget, PtTransportName}; use tor_proto::channel::Channel; -pub use registry::TransportRegistry; - /// An object that knows how to build `Channels` to `ChanTarget`s. /// /// This trait must be object-safe. @@ -61,3 +58,19 @@ where self.connect_via_transport(target).await } } + +/// The error type returned by a pluggable transport manager. +pub trait AbstractPtError: std::error::Error + HasKind + HasRetryTime + Send + Sync {} + +/// A pluggable transport manager. +/// +/// We can't directly reference the `PtMgr` type from `tor-ptmgr`, because of dependency resolution +/// constraints, so this defines the interface for what one should look like. +#[async_trait] +pub trait AbstractPtMgr { + /// Get a `ChannelFactory` for the provided `PtTransportName`. + async fn factory_for_transport( + &self, + transport: &PtTransportName, + ) -> Result>, Arc>; +} diff --git a/crates/tor-chanmgr/src/factory/registry.rs b/crates/tor-chanmgr/src/factory/registry.rs deleted file mode 100644 index e71192aad..000000000 --- a/crates/tor-chanmgr/src/factory/registry.rs +++ /dev/null @@ -1,34 +0,0 @@ -//! Implement a registry for different kinds of transports. - -use async_trait::async_trait; -use tor_linkspec::{HasChanMethod, OwnedChanTarget, TransportId}; -use tor_proto::channel::Channel; - -use crate::Error; - -use super::ChannelFactory; - -/// An object that knows about one or more [`ChannelFactory`]s. -/// -/// It can be used itself as a `ChannelFactory`, to open connections to a given -/// channel target depending on its configured [`TransportId`]. -// -// TODO pt-client: Turn this into a concrete type? -pub trait TransportRegistry { - /// Return a ChannelFactory that can make connections via a chosen - /// transport, if we know one. - // - // TODO pt-client: This might need to return an Arc instead of a reference - fn get_factory(&self, transport: &TransportId) -> Option<&(dyn ChannelFactory + Sync)>; -} - -#[async_trait] -impl ChannelFactory for R { - async fn connect_via_transport(&self, target: &OwnedChanTarget) -> crate::Result { - let method = target.chan_method(); - let id = method.transport_id(); - let factory = self.get_factory(&id).ok_or(Error::NoSuchTransport(id))?; - - factory.connect_via_transport(target).await - } -} diff --git a/crates/tor-chanmgr/src/lib.rs b/crates/tor-chanmgr/src/lib.rs index 1b86d1fc0..b2f3e4e89 100644 --- a/crates/tor-chanmgr/src/lib.rs +++ b/crates/tor-chanmgr/src/lib.rs @@ -280,14 +280,14 @@ impl ChanMgr { todo!("TODO pt-client: implement this.") } + /* + TODO pt-client: use AbstractPtMgr instead /// Replace the transport registry with one that may know about /// more transports. #[cfg(feature = "pt-client")] pub fn set_transport_registry(&self, _registry: impl factory::TransportRegistry) { - // TODO pt-client: See set_default_transport above. - #![allow(clippy::missing_panics_doc, clippy::needless_pass_by_value)] - todo!("TODO pt-client: implement this.") } + */ /// Watch for things that ought to change the configuration of all channels in the client /// diff --git a/crates/tor-ptmgr/src/lib.rs b/crates/tor-ptmgr/src/lib.rs index 179b666b6..05c679b41 100644 --- a/crates/tor-ptmgr/src/lib.rs +++ b/crates/tor-ptmgr/src/lib.rs @@ -58,6 +58,7 @@ use tor_rtcompat::Runtime; #[derive(Clone, Debug)] pub struct PtMgr { /// An underlying `Runtime`, used to spawn background tasks. + #[allow(dead_code)] runtime: R, } @@ -83,35 +84,3 @@ impl PtMgr { // TODO pt-client: Possibly, this should have a separate function to launch // its background tasks. } - -#[cfg(feature = "tor-channel-factory")] -#[allow(clippy::missing_panics_doc)] -impl tor_chanmgr::factory::TransportRegistry for PtMgr { - // There is going to be a lot happening "under the hood" here. - // - // When we are asked to get a ChannelFactory for a given - // connection, we will need to: - // - launch the binary for that transport if it is not already running*. - // - If we launched the binary, talk to it and see which ports it - // is listening on. - // - Return a ChannelFactory that connects via one of those ports, - // using the appropriate version of SOCKS, passing K=V parameters - // encoded properly. - // - // * As in other managers, we'll need to avoid trying to launch the same - // transport twice if we get two concurrent requests. - // - // Later if the binary crashes, we should detect that. We should relaunch - // it on demand. - // - // On reconfigure, we should shut down any no-longer-used transports. - // - // Maybe, we should shut down transports that haven't been used - // for a long time. - - fn get_factory(&self, transport: &TransportId) -> Option<&(dyn ChannelFactory + Sync)> { - let _ = transport; - let _ = &self.runtime; - todo!("TODO pt-client") - } -}