Merge branch 'main' into 'retriable'

# Conflicts:
#   doc/semver_status.md

crates/tor-chanmgr/src/builder.rs

@@ -229,10 +229,10 @@ mod test {
     };
     use pk::ed25519::Ed25519Identity;
     use pk::rsa::RsaIdentity;
-    use std::net::SocketAddr;
     use std::time::{Duration, SystemTime};
+    use std::{net::SocketAddr, str::FromStr};
     use tor_proto::channel::Channel;
-    use tor_rtcompat::{test_with_one_runtime, TcpListener};
+    use tor_rtcompat::{test_with_one_runtime, SleepProviderExt, TcpListener};
     use tor_rtmock::{io::LocalStream, net::MockNetwork, MockSleepRuntime};
 
     // Make sure that the builder can build a real channel.  To test
@@ -274,7 +274,7 @@ mod test {
             // Tell the client to believe in a different timestamp.
             client_rt.jump_to(now);
 
-            // Create the channelbuilder that we want to test.
+            // Create the channel builder that we want to test.
             let (snd, _rcv) = crate::event::channel();
             let builder = ChanBuilder::new(client_rt, snd);
 
@@ -298,10 +298,100 @@ mod test {
             let chan = r1.unwrap();
             assert_eq!(chan.ident(), &ed);
             assert!(chan.is_usable());
+            // In theory, time could pass here, so we can't just use
+            // "assert_eq!(dur_unused, dur_unused2)".
+            let dur_unused = Channel::duration_unused(&chan);
+            let dur_unused_2 = AbstractChannel::duration_unused(&chan);
+            let dur_unused_3 = Channel::duration_unused(&chan);
+            assert!(dur_unused.unwrap() <= dur_unused_2.unwrap());
+            assert!(dur_unused_2.unwrap() <= dur_unused_3.unwrap());
+
             r2.unwrap();
             Ok(())
         })
     }
 
+    #[test]
+    fn test_connect_one() {
+        let client_addr = "192.0.1.16".parse().unwrap();
+        // We'll put a "relay" at this address
+        let addr1 = SocketAddr::from_str("192.0.2.17:443").unwrap();
+        // We'll put nothing at this address, to generate errors.
+        let addr2 = SocketAddr::from_str("192.0.3.18:443").unwrap();
+        // Well put a black hole at this address, to generate timeouts.
+        let addr3 = SocketAddr::from_str("192.0.4.19:443").unwrap();
+        // We'll put a "relay" at this address too
+        let addr4 = SocketAddr::from_str("192.0.9.9:443").unwrap();
+
+        test_with_one_runtime!(|rt| async move {
+            // Stub out the internet so that this connection can work.
+            let network = MockNetwork::new();
+
+            // Set up a client and server runtime with a given IP
+            let client_rt = network
+                .builder()
+                .add_address(client_addr)
+                .runtime(rt.clone());
+            let server_rt = network
+                .builder()
+                .add_address(addr1.ip())
+                .add_address(addr4.ip())
+                .runtime(rt.clone());
+            let _listener = server_rt.mock_net().listen(&addr1).await.unwrap();
+            let _listener2 = server_rt.mock_net().listen(&addr4).await.unwrap();
+            // TODO: Because this test doesn't mock time, there will actually be
+            // delays as we wait for connections to this address to time out. It
+            // would be good to use MockSleepProvider instead, once we figure
+            // out how to make it both reliable and convenient.
+            network.add_blackhole(addr3).unwrap();
+
+            // No addresses? Can't succeed.
+            let failure = connect_to_one(&client_rt, &[]).await;
+            assert!(failure.is_err());
+
+            // Connect to a set of addresses including addr1? That's a success.
+            for addresses in [
+                &[addr1][..],
+                &[addr1, addr2][..],
+                &[addr2, addr1][..],
+                &[addr1, addr3][..],
+                &[addr3, addr1][..],
+                &[addr1, addr2, addr3][..],
+                &[addr3, addr2, addr1][..],
+            ] {
+                let (_conn, addr) = connect_to_one(&client_rt, addresses).await.unwrap();
+                assert_eq!(addr, addr1);
+            }
+
+            // Connect to a set of addresses including addr2 but not addr1?
+            // That's an error of one kind or another.
+            for addresses in [
+                &[addr2][..],
+                &[addr2, addr3][..],
+                &[addr3, addr2][..],
+                &[addr3][..],
+            ] {
+                let expect_timeout = addresses.contains(&addr3);
+                let failure = rt
+                    .timeout(
+                        Duration::from_millis(300),
+                        connect_to_one(&client_rt, addresses),
+                    )
+                    .await;
+                if expect_timeout {
+                    assert!(failure.is_err());
+                } else {
+                    assert!(failure.unwrap().is_err());
+                }
+            }
+
+            // Connect to addr1 and addr4?  The first one should win.
+            let (_conn, addr) = connect_to_one(&client_rt, &[addr1, addr4]).await.unwrap();
+            assert_eq!(addr, addr1);
+            let (_conn, addr) = connect_to_one(&client_rt, &[addr4, addr1]).await.unwrap();
+            assert_eq!(addr, addr4);
+        });
+    }
+
     // TODO: Write tests for timeout logic, once there is smarter logic.
 }

crates/tor-chanmgr/src/event.rs

@@ -351,6 +351,7 @@ mod test {
         assert_float_eq!(s1.frac(), 0.0, abs <= TOL);
         assert!(s1.eq(&s1));
         assert!(s1.blockage().is_none());
+        assert!(!s1.usable());
 
         let s2 = ConnStatus {
             online: Some(false),
@@ -365,6 +366,7 @@ mod test {
             s2.blockage().unwrap().to_string(),
             "unable to connect to the internet"
         );
+        assert!(!s2.usable());
 
         let s3 = ConnStatus {
             online: Some(true),
@@ -374,6 +376,7 @@ mod test {
         assert_float_eq!(s3.frac(), 0.5, abs <= TOL);
         assert_eq!(s3.blockage(), None);
         assert!(!s3.eq(&s1));
+        assert!(!s3.usable());
 
         let s4 = ConnStatus {
             online: Some(true),
@@ -390,6 +393,7 @@ mod test {
         assert!(!s4.eq(&s2));
         assert!(!s4.eq(&s3));
         assert!(s4.eq(&s4));
+        assert!(!s4.usable());
 
         let s5 = ConnStatus {
             online: Some(true),
@@ -400,6 +404,7 @@ mod test {
         assert!(s5.blockage().is_none());
         assert!(s5.eq(&s5));
         assert!(!s5.eq(&s4));
+        assert!(s5.usable());
     }
 
     #[test]

crates/tor-checkable/src/timed.rs

@@ -203,16 +203,44 @@ mod test {
         let eu = SystemTime::UNIX_EPOCH + one_day * 8705;
         let za = SystemTime::UNIX_EPOCH + one_day * 8882;
 
+        // check_valid_at
         let tr = TimerangeBound::new("Hello world", cz_sk..eu);
         assert!(tr.check_valid_at(&za).is_err());
 
         let tr = TimerangeBound::new("Hello world", cz_sk..za);
         assert_eq!(tr.check_valid_at(&eu), Ok("Hello world"));
 
+        // check_valid_now
         let tr = TimerangeBound::new("hello world", de..);
         assert_eq!(tr.check_valid_now(), Ok("hello world"));
 
         let tr = TimerangeBound::new("hello world", ..za);
         assert!(tr.check_valid_now().is_err());
+
+        // Now try check_valid_at_opt() api
+        let tr = TimerangeBound::new("hello world", de..);
+        assert_eq!(tr.check_valid_at_opt(None), Ok("hello world"));
+        let tr = TimerangeBound::new("hello world", de..);
+        assert_eq!(
+            tr.check_valid_at_opt(Some(SystemTime::now())),
+            Ok("hello world")
+        );
+        let tr = TimerangeBound::new("hello world", ..za);
+        assert!(tr.check_valid_at_opt(None).is_err());
+    }
+
+    #[cfg(feature = "experimental-api")]
+    #[test]
+    fn test_dangerous() {
+        let t1 = SystemTime::now();
+        let t2 = t1 + Duration::from_secs(60 * 525600);
+        let tr = TimerangeBound::new("cups of coffee", t1..=t2);
+
+        assert_eq!(tr.dangerously_peek(), &"cups of coffee");
+
+        let (a, b) = tr.dangerously_into_parts();
+        assert_eq!(a, "cups of coffee");
+        assert_eq!(b.0, Bound::Included(t1));
+        assert_eq!(b.1, Bound::Included(t2));
     }
 }

crates/tor-rtmock/src/net.rs

@@ -40,7 +40,7 @@ type ConnReceiver = mpsc::Receiver<(LocalStream, SocketAddr)>;
 /// its own view of its address(es) on the network.
 pub struct MockNetwork {
     /// A map from address to the entries about listeners there.
-    listening: Mutex<HashMap<SocketAddr, ListenerEntry>>,
+    listening: Mutex<HashMap<SocketAddr, AddrBehavior>>,
 }
 
 /// The `MockNetwork`'s view of a listener.
@@ -55,6 +55,15 @@ struct ListenerEntry {
     tls_cert: Option<Vec<u8>>,
 }
 
+/// A possible non-error behavior from an address
+#[derive(Clone)]
+enum AddrBehavior {
+    /// There's a listener at this address, which would like to reply.
+    Listener(ListenerEntry),
+    /// All connections sent to this address will time out.
+    Timeout,
+}
+
 /// A view of a single host's access to a MockNetwork.
 ///
 /// Each simulated host has its own addresses that it's allowed to listen on,
@@ -159,6 +168,16 @@ impl MockNetwork {
         }
     }
 
+    /// Add a "black hole" at the given address, where all traffic will time out.
+    pub fn add_blackhole(&self, address: SocketAddr) -> IoResult<()> {
+        let mut listener_map = self.listening.lock().expect("Poisoned lock for listener");
+        if listener_map.contains_key(&address) {
+            return Err(err(ErrorKind::AddrInUse));
+        }
+        listener_map.insert(address, AddrBehavior::Timeout);
+        Ok(())
+    }
+
     /// Tell the listener at `target_addr` (if any) about an incoming
     /// connection from `source_addr` at `peer_stream`.
     ///
@@ -177,12 +196,16 @@ impl MockNetwork {
             let listener_map = self.listening.lock().expect("Poisoned lock for listener");
             listener_map.get(&target_addr).map(Clone::clone)
         };
-        if let Some(mut entry) = entry {
-            if entry.send.send((peer_stream, source_addr)).await.is_ok() {
-                return Ok(entry.tls_cert);
+        match entry {
+            Some(AddrBehavior::Listener(mut entry)) => {
+                if entry.send.send((peer_stream, source_addr)).await.is_ok() {
+                    return Ok(entry.tls_cert);
+                }
+                Err(err(ErrorKind::ConnectionRefused))
             }
+            Some(AddrBehavior::Timeout) => futures::future::pending().await,
+            None => Err(err(ErrorKind::ConnectionRefused)),
         }
-        Err(err(ErrorKind::ConnectionRefused))
     }
 
     /// Register a listener at `addr` and return the ConnReceiver
@@ -203,7 +226,7 @@ impl MockNetwork {
 
         let entry = ListenerEntry { send, tls_cert };
 
-        listener_map.insert(addr, entry);
+        listener_map.insert(addr, AddrBehavior::Listener(entry));
 
         Ok(recv)
     }

crates/tor-socksproto/src/msg.rs

@@ -19,15 +19,6 @@ pub enum SocksVersion {
     V5,
 }
 
-impl From<SocksVersion> for u8 {
-    fn from(v: SocksVersion) -> u8 {
-        match v {
-            SocksVersion::V4 => 4,
-            SocksVersion::V5 => 5,
-        }
-    }
-}
-
 impl TryFrom<u8> for SocksVersion {
     type Error = Error;
     fn try_from(v: u8) -> Result<SocksVersion> {
@@ -184,12 +175,6 @@ impl AsRef<str> for SocksHostname {
     }
 }
 
-impl From<SocksHostname> for String {
-    fn from(s: SocksHostname) -> String {
-        s.0
-    }
-}
-
 impl SocksRequest {
     /// Create a SocksRequest with a given set of fields.
     ///

doc/semver_status.md

@@ -21,3 +21,11 @@ We can delete older sections here after we bump the releases.
 ### tor-circmgr
 
 MODIFIED: Added a new variant in tor_circmgr::Error.
+
+### tor-rtmock
+
+MODIFIED: Added add_blackhole to MockNetwork.
+
+### tor-socksproto
+
+BREAKING: Removed some unused accessors.