vincenzopalazzo
/
arti
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chanmgr: clean up some TODO pt-client items and documentation.

This commit is contained in:
Nick Mathewson 2022-10-12 10:36:14 -04:00
parent d759489530
commit 69b64a2795
3 changed files with 23 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/tor-chanmgr/src/err.rs
View File

@ -149,10 +149,6 @@ impl tor_error::HasRetryTime for Error {
// This one can't succeed until the bridge, or our set of
// transports, is reconfigured.
//
// TODO pt-client: Double-check this, to make sure that it doesn't
// cause us to reject a multi-transport bridge if only one of its
// transports fails.
E::NoSuchTransport(_) => RT::Never,
// These aren't recoverable at all.

92
crates/tor-chanmgr/src/factory.rs
View File

@ -3,11 +3,12 @@
use std::sync::Arc;
use crate::Error;
use async_trait::async_trait;
use futures::{AsyncRead, AsyncWrite};
use tor_linkspec::{HasChanMethod, OwnedChanTarget, TransportId};
use tor_proto::channel::Channel;
use tor_rtcompat::Runtime;
/// An object that knows how to build Channels to ChanTargets.
///
@ -16,11 +17,12 @@ use tor_rtcompat::Runtime;
pub trait ChannelFactory {
/// Open an authenticated channel to `target`.
///
/// We need this method to take a dyn ChanTarget so it is
/// object-safe.
//
// TODO pt-client: How does this handle multiple addresses? Do we
// parallelize here, or at a higher level?
/// This method does does not necessarily handle retries or timeouts,
/// although some of its implementations may.
///
/// This method does not necessarily handle every kind of transport.
/// If the caller provides a target with the wrong [`TransportId`], this
/// method should return [`Error::NoSuchTransport`].
async fn connect_via_transport(&self, target: &OwnedChanTarget) -> crate::Result<Channel>;
}
@ -31,10 +33,8 @@ pub trait ChannelFactory {
/// This is the trait you should probably implement if you want to define a new
/// [`ChannelFactory`] that performs Tor over TLS over some stream-like type,
/// and you only want to define the stream-like type.
//
// TODO pt-client: I originally had this parameterized on a Runtime. But I
// think instead we should have individual TransportHelper implementations be
// parameterized on a Runtime.
///
/// To convert a [`TransportHelper`] into a [`ChannelFactory`], wrap it in a ChannelBuilder.
#[async_trait]
pub trait TransportHelper {
/// The type of the resulting stream.
@ -42,69 +42,19 @@ pub trait TransportHelper {
/// Implements the transport: makes a TCP connection (possibly
/// tunneled over whatever protocol) if possible.
//
// TODO pt-client: How does this handle multiple addresses? Do we
// parallelize here, or at a higher level?
//
// TODO pt-client: We could make the address an associated type: would that
// help anything?
///
/// This method does does not necessarily handle retries or timeouts,
/// although some of its implementations may.
///
/// This method does not necessarily handle every kind of transport.
/// If the caller provides a target with the wrong [`TransportId`], this
/// method should return [`Error::NoSuchTransport`].
async fn connect(
&self,
target: &OwnedChanTarget,
) -> crate::Result<(OwnedChanTarget, Self::Stream)>;
}
// We define an implementation so that every TransportHelper
// can be wrapped as a ChannelFactory...
#[async_trait]
impl<H> ChannelFactory for H
where
H: TransportHelper + Sync,
{
async fn connect_via_transport(&self, target: &OwnedChanTarget) -> crate::Result<Channel> {
let _stream = self.connect(target).await?;
// Now do the logic from
// `tor_chanmgr::builder::ChanBuilder::build_channel_no_timeout`:
// Negotiate TLS, call tor_proto::ChannelBuilder::build, ...
// TODO: Hang on, where do we get a pre-built TlsConnector in
// this method? We may need a different signature, or some
// kind of wrapper type.
//
// TODO: We may also need access to other stuff, like the contents
// of `ChanBuilder`.
todo!("TODO pt-client: implement this")
}
}
/// A ChannelFactory implementing Tor's default channel protocol.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct DefaultChannelFactory<R: Runtime> {
/// The runtime that we use to make connections.
#[allow(dead_code)] // TODO pt-client: this will be removed.
runtime: R,
}
#[async_trait]
impl<R: Runtime> TransportHelper for DefaultChannelFactory<R> {
type Stream = R::TcpStream;
async fn connect(
&self,
_target: &OwnedChanTarget,
) -> crate::Result<(OwnedChanTarget, Self::Stream)> {
// Call connect_one() as in `build_channel_no_timeout`.
// TODO pt-client: This is another place where we need to figure out
// multiple addresses and "happy eyeballs".
// Call restrict_addr() as in `build_channel_no_timeout`.
todo!("TODO pt-client: implement this")
}
}
/// An object that knows about one or more ChannelFactories.
pub trait TransportRegistry {
/// Return a ChannelFactory that can make connections via a chosen
@ -114,9 +64,6 @@ pub trait TransportRegistry {
fn get_factory(&self, transport: &TransportId) -> Option<&(dyn ChannelFactory + Sync)>;
}
// TODO pt-client: implement a DefaultTransportRegistry that returns a
// DefaultChannelFactory for TransportId::Builtin, and nothing otherwise.
/// Helper type: Wrap a `TransportRegistry` so that it can be used as a
/// `ChannelFactory`.
///
@ -130,10 +77,7 @@ impl<R: TransportRegistry + Sync> ChannelFactory for RegistryAsFactory<R> {
async fn connect_via_transport(&self, target: &OwnedChanTarget) -> crate::Result<Channel> {
let method = target.chan_method();
let id = method.transport_id();
let factory = self
.0
.get_factory(&id)
.ok_or(crate::Error::NoSuchTransport(id))?;
let factory = self.0.get_factory(&id).ok_or(Error::NoSuchTransport(id))?;
factory.connect_via_transport(target).await
}

7
crates/tor-chanmgr/src/lib.rs
View File

@ -313,16 +313,19 @@ impl<R: Runtime> ChanMgr<R> {
/// This method can be used to e.g. tell Arti to use a proxy for
/// outgoing connections.
pub fn set_default_transport(&self, _factory: impl factory::ChannelFactory) {
// TODO pt-client: Perhaps we actually want to remove this and have it
// be part of the constructor? The only way to actually implement it is
// to make the channel factory in AbstractChanMgr mutable, which seels a
// little ugly. Do we ever want to change this on a _running_ ChanMgr?
#![allow(clippy::missing_panics_doc, clippy::needless_pass_by_value)]
todo!("TODO pt-client: implement this.")
}
/// Replace the transport registry with one that may know about
/// more transports.
//
// TODO::pt_client (Alternatively, move this functionality into ChanMgr::new?)
#[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.")
}