From aa72f3d8c8456534e247c513461c56be74b1374e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 13:52:05 -0500 Subject: [PATCH 1/7] Add a test for ByRelayIds::remove_by_all_ids --- crates/tor-linkspec/src/ids/by_id.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/tor-linkspec/src/ids/by_id.rs b/crates/tor-linkspec/src/ids/by_id.rs index 0c76edbb8..dd9f5afdd 100644 --- a/crates/tor-linkspec/src/ids/by_id.rs +++ b/crates/tor-linkspec/src/ids/by_id.rs @@ -116,7 +116,7 @@ impl ByRelayIds { } } - /// Remove the single value in this set (if any) that has all the same relay ids that + /// Remove the single value in this set (if any) that has all the same /// relay IDs that `key` does. pub fn remove_by_all_ids(&mut self, key: &T) -> Option where @@ -133,6 +133,7 @@ impl ByRelayIds { None } } + /// Return a reference to every element in this set that shares _any_ ID /// with `key`. /// @@ -268,7 +269,7 @@ mod test { let mut set = ByRelayIds::new(); set.insert(keys1.clone()); - set.insert(keys2); + set.insert(keys2.clone()); assert_eq!(set.len(), 2); let removed = set.remove_exact(&keys1); @@ -277,9 +278,27 @@ mod test { { let search = RelayIdsBuilder::default().ed_identity(ed2).build().unwrap(); + // We're calling remove_exact, but we did not list _all_ the keys in keys2. let removed = set.remove_exact(&search); assert_eq!(removed, None); assert_eq!(set.len(), 1); + + // If we were to use `remove_by_all_ids` with a search that didn't + // match, it wouldn't work. + let no_match = RelayIdsBuilder::default() + .ed_identity(ed2) + .rsa_identity(rsa1) + .build() + .unwrap(); + let removed = set.remove_by_all_ids(&no_match); + assert_eq!(removed, None); + assert_eq!(set.len(), 1); + + // If we use `remove_by_all_ids` with the original search, though, + // it will remove the element. + let removed = set.remove_by_all_ids(&search); + assert_eq!(removed, Some(keys2)); + assert_eq!(set.len(), 0); } } } From 145ab3c49baa7ef17beed970574690513f265f49 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 13:55:11 -0500 Subject: [PATCH 2/7] linkspec: Add test for lookup on no-ids. --- crates/tor-linkspec/src/ids/by_id.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/tor-linkspec/src/ids/by_id.rs b/crates/tor-linkspec/src/ids/by_id.rs index dd9f5afdd..b833df5df 100644 --- a/crates/tor-linkspec/src/ids/by_id.rs +++ b/crates/tor-linkspec/src/ids/by_id.rs @@ -202,6 +202,7 @@ mod test { // Try exact lookup assert_eq!(set.by_all_ids(&keys1), Some(&keys1)); assert_eq!(set.by_all_ids(&keys2), Some(&keys2)); + assert_eq!(set.by_all_ids(&RelayIds::empty()), None); { let search = RelayIdsBuilder::default() .rsa_identity(rsa1) From d990a239483d5a860118be6992035f84309f98f1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 14:45:05 -0500 Subject: [PATCH 3/7] linkspec: Add tests for has_any_identity. --- crates/tor-linkspec/src/traits.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/tor-linkspec/src/traits.rs b/crates/tor-linkspec/src/traits.rs index e22d3f070..8a3736a4d 100644 --- a/crates/tor-linkspec/src/traits.rs +++ b/crates/tor-linkspec/src/traits.rs @@ -457,4 +457,11 @@ mod test { ); } } + + #[test] + fn has_id() { + use crate::RelayIds; + assert!(example().has_any_identity()); + assert!(!RelayIds::empty().has_any_identity()); + } } From 119e5f6f754251e0d2db7731f9a7044764f4653e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 15:18:59 -0500 Subject: [PATCH 4/7] PtTransportName: Remove unused accessors. --- crates/tor-linkspec/src/transport.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/crates/tor-linkspec/src/transport.rs b/crates/tor-linkspec/src/transport.rs index 679ddfd83..7278cf6d6 100644 --- a/crates/tor-linkspec/src/transport.rs +++ b/crates/tor-linkspec/src/transport.rs @@ -82,19 +82,6 @@ impl TryFrom for PtTransportName { } } -impl AsRef for PtTransportName { - fn as_ref(&self) -> &str { - &self.0 - } -} - -impl PtTransportName { - /// Return the name as a `String` - pub fn into_inner(self) -> String { - self.0 - } -} - impl Display for PtTransportName { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { Display::fmt(&self.0, f) From 60bc17901912dc2aa636a2732d46718cb052288f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 15:36:08 -0500 Subject: [PATCH 5/7] linkspec: Tests for PtTargetSettings --- crates/tor-linkspec/src/transport.rs | 46 ++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crates/tor-linkspec/src/transport.rs b/crates/tor-linkspec/src/transport.rs index 7278cf6d6..8f5fb734e 100644 --- a/crates/tor-linkspec/src/transport.rs +++ b/crates/tor-linkspec/src/transport.rs @@ -668,4 +668,50 @@ mod test { assert!(matches!(e, BridgeAddrError::BadAddress(_))); } } + + #[test] + fn transport_id() { + let id1: TransportId = "".parse().unwrap(); + assert!(id1.is_builtin()); + assert_eq!(id1.to_string(), "".to_string()); + + #[cfg(feature = "pt-client")] + { + let id2: TransportId = "obfs4".parse().unwrap(); + assert_ne!(id2, id1); + assert!(!id2.is_builtin()); + assert_eq!(id2.to_string(), "obfs4"); + + assert!(matches!( + TransportId::from_str("==="), + Err(TransportIdError::BadId(_)) + )); + } + + #[cfg(not(feature = "pt-client"))] + { + assert!(matches!( + TransportId::from_str("obfs4"), + Err(TransportIdError::NoSupport) + )) + } + } + + #[test] + fn settings() { + let s = PtTargetSettings::try_from(vec![]).unwrap(); + assert_eq!(Vec::<_>::from(s), vec![]); + + let v = vec![("abc".into(), "def".into()), ("ghi".into(), "jkl".into())]; + let s = PtTargetSettings::try_from(v.clone()).unwrap(); + assert_eq!(Vec::<_>::from(s), v); + + let v = vec![("a=b".into(), "def".into())]; + let s = PtTargetSettings::try_from(v); + assert!(matches!(s, Err(PtTargetInvalidSetting::Key(_)))); + + let v = vec![("abc".into(), "d ef".into())]; + let s = PtTargetSettings::try_from(v); + assert!(matches!(s, Err(PtTargetInvalidSetting::Value(_)))); + } } From 777b6bee3d6e66c0c1339b82a47364dc827eed73 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 16 Nov 2022 16:00:15 -0500 Subject: [PATCH 6/7] linkspec: Add some tests for ChannelMethod --- Cargo.lock | 1 + crates/tor-linkspec/Cargo.toml | 1 + crates/tor-linkspec/src/transport.rs | 84 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index c9a90646d..f695991a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3884,6 +3884,7 @@ dependencies = [ "educe", "hex", "hex-literal", + "itertools", "safelog", "serde", "serde_test", diff --git a/crates/tor-linkspec/Cargo.toml b/crates/tor-linkspec/Cargo.toml index 56b217831..90ca70938 100644 --- a/crates/tor-linkspec/Cargo.toml +++ b/crates/tor-linkspec/Cargo.toml @@ -38,6 +38,7 @@ tor-protover = { path = "../tor-protover", version = "0.3.0" } [dev-dependencies] hex-literal = "0.3" +itertools = "0.10.1" serde_test = "1.0.124" [package.metadata.docs.rs] diff --git a/crates/tor-linkspec/src/transport.rs b/crates/tor-linkspec/src/transport.rs index 8f5fb734e..df7405a46 100644 --- a/crates/tor-linkspec/src/transport.rs +++ b/crates/tor-linkspec/src/transport.rs @@ -714,4 +714,88 @@ mod test { let s = PtTargetSettings::try_from(v); assert!(matches!(s, Err(PtTargetInvalidSetting::Value(_)))); } + + #[test] + fn chanmethod_direct() { + let a1 = "127.0.0.1:8080".parse().unwrap(); + let a2 = "127.0.0.2:8181".parse().unwrap(); + let a3 = "127.0.0.3:8282".parse().unwrap(); + + let m = ChannelMethod::Direct(vec![a1, a2]); + assert_eq!(m.socket_addrs(), Some(&[a1, a2][..])); + assert_eq!((m.target_addr()), Some(PtTargetAddr::IpPort(a1))); + assert!(m.is_direct()); + assert_eq!(m.transport_id(), TransportId::default()); + + let m2 = ChannelMethod::Direct(vec![a1, a2, a3]); + assert!(m.contained_by(&m)); + assert!(m.contained_by(&m2)); + assert!(!m2.contained_by(&m)); + + let mut m3 = m2.clone(); + m3.retain_addrs(|a| a.port() != 8282).unwrap(); + assert_eq!(m3, m); + assert_ne!(m3, m2); + } + + #[test] + #[cfg(feature = "pt-client")] + fn chanmethod_pt() { + use itertools::Itertools; + + let transport = "giraffe".parse().unwrap(); + let addr1 = PtTargetAddr::HostPort("pt.example.com".into(), 1234); + let target1 = PtTarget::new("giraffe".parse().unwrap(), addr1.clone()); + let m1 = ChannelMethod::Pluggable(target1); + + let addr2 = PtTargetAddr::IpPort("127.0.0.1:567".parse().unwrap()); + let target2 = PtTarget::new("giraffe".parse().unwrap(), addr2.clone()); + let m2 = ChannelMethod::Pluggable(target2); + + let addr3 = PtTargetAddr::None; + let target3 = PtTarget::new("giraffe".parse().unwrap(), addr3.clone()); + let m3 = ChannelMethod::Pluggable(target3); + + assert_eq!(m1.socket_addrs(), None); + assert_eq!( + m2.socket_addrs(), + Some(&["127.0.0.1:567".parse().unwrap()][..]) + ); + assert_eq!(m3.socket_addrs(), None); + + assert_eq!(m1.target_addr(), Some(addr1)); + assert_eq!(m2.target_addr(), Some(addr2)); + assert_eq!(m3.target_addr(), Some(addr3)); + + assert!(!m1.is_direct()); + assert!(!m2.is_direct()); + assert!(!m3.is_direct()); + + assert_eq!(m1.transport_id(), transport); + assert_eq!(m2.transport_id(), transport); + assert_eq!(m3.transport_id(), transport); + + for v in [&m1, &m2, &m3].iter().combinations(2) { + let first = v[0]; + let second = v[1]; + assert_eq!(first.contained_by(second), first == second); + } + + let mut m1new = m1.clone(); + let mut m2new = m2.clone(); + let mut m3new = m3.clone(); + // this will retain the IpPort target, and ignore the other targets. + m1new.retain_addrs(|a| a.port() == 567).unwrap(); + m2new.retain_addrs(|a| a.port() == 567).unwrap(); + m3new.retain_addrs(|a| a.port() == 567).unwrap(); + assert_eq!(m1new, m1); + assert_eq!(m2new, m2); + assert_eq!(m3new, m3); + + // But if we try to remove the ipport target, we get an error. + assert!(matches!( + m2new.retain_addrs(|a| a.port() == 999), + Err(RetainAddrsError::NoAddrsLeft) + )); + } } From 45c4beff7a31acf9569a810ec77080810dfd0482 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 28 Nov 2022 08:29:33 -0500 Subject: [PATCH 7/7] Fix up compatibility issues between linkspec tests and other patches --- crates/tor-guardmgr/src/bridge/config.rs | 2 +- crates/tor-linkspec/src/transport.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/tor-guardmgr/src/bridge/config.rs b/crates/tor-guardmgr/src/bridge/config.rs index af30513d8..8fce2e077 100644 --- a/crates/tor-guardmgr/src/bridge/config.rs +++ b/crates/tor-guardmgr/src/bridge/config.rs @@ -367,7 +367,7 @@ impl FromStr for BridgeConfigBuilder { #[cfg(feature = "pt-client")] ChannelMethod::Pluggable(target) => { let (transport, addr, settings) = target.into_parts(); - (transport.into_inner(), vec![addr], settings.into_inner()) + (transport.to_string(), vec![addr], settings.into_inner()) } other => { return Err(BridgeParseError::UnsupportedChannelMethod { diff --git a/crates/tor-linkspec/src/transport.rs b/crates/tor-linkspec/src/transport.rs index df7405a46..21361578e 100644 --- a/crates/tor-linkspec/src/transport.rs +++ b/crates/tor-linkspec/src/transport.rs @@ -673,7 +673,7 @@ mod test { fn transport_id() { let id1: TransportId = "".parse().unwrap(); assert!(id1.is_builtin()); - assert_eq!(id1.to_string(), "".to_string()); + assert_eq!(id1.to_string(), "-".to_string()); #[cfg(feature = "pt-client")] { @@ -723,7 +723,7 @@ mod test { let m = ChannelMethod::Direct(vec![a1, a2]); assert_eq!(m.socket_addrs(), Some(&[a1, a2][..])); - assert_eq!((m.target_addr()), Some(PtTargetAddr::IpPort(a1))); + assert_eq!((m.target_addr()), Some(BridgeAddr::IpPort(a1))); assert!(m.is_direct()); assert_eq!(m.transport_id(), TransportId::default()); @@ -744,15 +744,15 @@ mod test { use itertools::Itertools; let transport = "giraffe".parse().unwrap(); - let addr1 = PtTargetAddr::HostPort("pt.example.com".into(), 1234); + let addr1 = BridgeAddr::HostPort("pt.example.com".into(), 1234); let target1 = PtTarget::new("giraffe".parse().unwrap(), addr1.clone()); let m1 = ChannelMethod::Pluggable(target1); - let addr2 = PtTargetAddr::IpPort("127.0.0.1:567".parse().unwrap()); + let addr2 = BridgeAddr::IpPort("127.0.0.1:567".parse().unwrap()); let target2 = PtTarget::new("giraffe".parse().unwrap(), addr2.clone()); let m2 = ChannelMethod::Pluggable(target2); - let addr3 = PtTargetAddr::None; + let addr3 = BridgeAddr::None; let target3 = PtTarget::new("giraffe".parse().unwrap(), addr3.clone()); let m3 = ChannelMethod::Pluggable(target3);