Merge branch 'display_chan_target' into 'main'

ChanTarget: Add a display function, with better output.

Closes #647

See merge request tpo/core/arti!868

crates/tor-linkspec/semver.md

@@ -6,4 +6,4 @@ MODIFIED: RelayId and RelayIdRef now implement Ord.
 MODIFIED: Added cmp_by_relay_ids() to HasRelayIds.
 BREAKING: Replaced functions to access addresses from ChanMethod.
 BREAKING: Replaced functions to strip addresses from ChanMethod.
-
+BREAKING: Remove impl Display for OwnedCircTarget.

crates/tor-linkspec/src/owned.rs

@@ -159,17 +159,7 @@ impl OwnedChanTarget {
 /// Primarily for error reporting and logging
 impl Display for OwnedChanTarget {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "[")?;
-        match &*self.addrs {
-            [] => write!(f, "?")?,
-            [a] => write!(f, "{}", a)?,
-            [a, ..] => write!(f, "{}+", a)?,
-        };
-        for ident in self.identities() {
-            write!(f, " {}", ident)?;
-        }
-        write!(f, "]")?;
-        Ok(())
+        write!(f, "{}", self.display_chan_target())
     }
 }
 
@@ -209,13 +199,6 @@ impl OwnedCircTarget {
     }
 }
 
-/// Primarily for error reporting and logging
-impl Display for OwnedCircTarget {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        Display::fmt(&self.chan_target, f)
-    }
-}
-
 impl HasAddrs for OwnedCircTarget {
     fn addrs(&self) -> &[SocketAddr] {
         self.chan_target.addrs()

crates/tor-linkspec/src/traits.rs

@@ -1,7 +1,7 @@
 //! Declare traits to be implemented by types that describe a place
 //! that Tor can connect to, directly or indirectly.
 
-use std::{iter::FusedIterator, net::SocketAddr};
+use std::{fmt, iter::FusedIterator, net::SocketAddr};
 use strum::IntoEnumIterator;
 use tor_llcrypto::pk;
 
@@ -195,7 +195,19 @@ impl<D: DirectChanMethodsHelper> HasChanMethod for D {
 /// Anything that implements 'ChanTarget' can be used as the
 /// identity of a relay for the purposes of launching a new
 /// channel.
-pub trait ChanTarget: HasRelayIds + HasAddrs + HasChanMethod {}
+pub trait ChanTarget: HasRelayIds + HasAddrs + HasChanMethod {
+    /// Return a reference to this object suitable for formatting its
+    /// [`ChanTarget`]-specific members.
+    ///
+    /// The display format is not exhaustive, but tries to give enough
+    /// information to identify which channel target we're talking about.
+    fn display_chan_target(&self) -> DisplayChanTarget<'_, Self>
+    where
+        Self: Sized,
+    {
+        DisplayChanTarget { inner: self }
+    }
+}
 
 impl<T: ChanTarget> From<&T> for OwnedChanTarget {
     fn from(target: &T) -> Self {
@@ -226,6 +238,44 @@ pub trait CircTarget: ChanTarget {
     fn protovers(&self) -> &tor_protover::Protocols;
 }
 
+/// A reference to a ChanTarget that implements Display using a hopefully useful
+/// format.
+#[derive(Debug, Clone)]
+pub struct DisplayChanTarget<'a, T> {
+    /// The ChanTarget that we're formatting.
+    inner: &'a T,
+}
+
+impl<'a, T: ChanTarget> fmt::Display for DisplayChanTarget<'a, T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "[")?;
+        // We look at the chan_method() (where we would connect to) rather than
+        // the addrs() (where the relay is, nebulously, "located").  This lets us
+        // give a less surprising description.
+        match self.inner.chan_method() {
+            ChannelMethod::Direct(v) if v.is_empty() => write!(f, "?")?,
+            ChannelMethod::Direct(v) if v.len() == 1 => write!(f, "{}", v[0])?,
+            ChannelMethod::Direct(v) => write!(f, "{}+", v[0])?,
+            #[cfg(feature = "pt-client")]
+            ChannelMethod::Pluggable(target) => {
+                match target.addr() {
+                    crate::PtTargetAddr::None => {}
+                    other => write!(f, "{} ", other)?,
+                }
+                write!(f, "via {}", target.transport())?;
+                // This deliberately doesn't include the PtTargetSettings, since
+                // they can be large, and they're typically unnecessary.
+            }
+        }
+
+        for ident in self.inner.identities() {
+            write!(f, " {}", ident)?;
+        }
+
+        write!(f, "]")
+    }
+}
+
 #[cfg(test)]
 mod test {
     #![allow(clippy::unwrap_used)]
@@ -383,4 +433,31 @@ mod test {
             b(Some(ed1), Some(rsa1)),
         ]);
     }
+
+    #[test]
+    fn display() {
+        use crate::PtTarget;
+
+        let e1 = example();
+        assert_eq!(
+            e1.display_chan_target().to_string(),
+            "[127.0.0.1:99+ ed25519:/FHNjmIYoaONpH7QAjDwWAgW7RO6MwOsXeuRFUiQgCU \
+              $1234567890abcdef12341234567890abcdef1234]"
+        );
+
+        let rsa = hex!("234461644a6f6b6523436f726e794f6e4d61696e").into();
+        let mut b = OwnedChanTarget::builder();
+        b.ids().rsa_identity(rsa);
+        let e2 = b
+            .method(ChannelMethod::Pluggable(PtTarget::new(
+                "obfs4".parse().unwrap(),
+                "127.0.0.1:99".parse().unwrap(),
+            )))
+            .build()
+            .unwrap();
+        assert_eq!(
+            e2.to_string(),
+            "[127.0.0.1:99 via obfs4 $234461644a6f6b6523436f726e794f6e4d61696e]"
+        );
+    }
 }