From d2edf25733a16817f3a6380d689cb18beadea5a8 Mon Sep 17 00:00:00 2001 From: trinity-1686a <trinity@deuxfleurs.fr> Date: Thu, 24 Mar 2022 21:12:46 +0100 Subject: [PATCH] move StreamIsolation to isolation module --- crates/arti-client/src/client.rs | 2 +- crates/tor-circmgr/src/isolation.rs | 109 ++++++++++++++++++++++++++-- crates/tor-circmgr/src/lib.rs | 3 +- crates/tor-circmgr/src/usage.rs | 103 +------------------------- 4 files changed, 108 insertions(+), 109 deletions(-) diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index d0c95183a8..69cca9d7d3 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -8,7 +8,7 @@ use crate::address::IntoTorAddr; use crate::config::{ClientAddrConfig, StreamTimeoutConfig, TorClientConfig}; use tor_circmgr::isolation::Isolation; -use tor_circmgr::{DirInfo, IsolationToken, StreamIsolationBuilder, TargetPort}; +use tor_circmgr::{isolation::StreamIsolationBuilder, DirInfo, IsolationToken, TargetPort}; use tor_config::MutCfg; use tor_dirmgr::DirEvent; use tor_persist::{FsStateMgr, StateMgr}; diff --git a/crates/tor-circmgr/src/isolation.rs b/crates/tor-circmgr/src/isolation.rs index a1ccddd4b6..3936620f26 100644 --- a/crates/tor-circmgr/src/isolation.rs +++ b/crates/tor-circmgr/src/isolation.rs @@ -340,8 +340,67 @@ tuple_impls! { } } +/// A set of information about how a stream should be isolated. +/// +/// If two streams are isolated from one another, they may not share +/// a circuit. +#[derive(Clone, Debug, derive_builder::Builder)] +pub struct StreamIsolation { + /// Any isolation token set on the stream. + #[builder(default = "Box::new(IsolationToken::no_isolation())")] + stream_token: Box<dyn Isolation>, + /// Any additional isolation token set on an object that "owns" this + /// stream. This is typically owned by a `TorClient`. + #[builder(default = "IsolationToken::no_isolation()")] + owner_token: IsolationToken, +} + +impl StreamIsolation { + /// Construct a new StreamIsolation with no isolation enabled. + pub fn no_isolation() -> Self { + StreamIsolationBuilder::new() + .build() + .expect("Bug constructing StreamIsolation") + } + + /// Return a new StreamIsolationBuilder for constructing + /// StreamIsolation objects. + pub fn builder() -> StreamIsolationBuilder { + StreamIsolationBuilder::new() + } + + /// Return true if this StreamIsolation can share a circuit with + /// `other`. + pub fn may_share_circuit(&self, other: &StreamIsolation) -> bool { + self.owner_token == other.owner_token + && self.stream_token.compatible(other.stream_token.as_ref()) + } + + /// Return a StreamIsolation that is the intersection of self and other. + /// Return None if such a merge is not possible. + pub fn join(&self, other: &StreamIsolation) -> Option<StreamIsolation> { + if self.owner_token != other.owner_token { + return None; + } + self.stream_token + .join(other.stream_token.as_ref()) + .map(|stream_token| StreamIsolation { + stream_token, + owner_token: self.owner_token, + }) + } +} + +impl StreamIsolationBuilder { + /// Construct a builder with no items set. + pub fn new() -> Self { + StreamIsolationBuilder::default() + } +} + #[cfg(test)] pub(crate) mod test { + #![allow(clippy::unwrap_used)] use super::*; /// Trait for testing use only. Much like PartialEq, but for type containing an dyn Isolation @@ -397,6 +456,13 @@ pub(crate) mod test { } } + impl IsolationTokenEq for StreamIsolation { + fn isol_eq(&self, other: &Self) -> bool { + self.stream_token.isol_eq(other.stream_token.as_ref()) + && self.owner_token == other.owner_token + } + } + #[derive(PartialEq, Clone, Copy, Debug)] struct OtherIsolation(usize); @@ -468,13 +534,9 @@ pub(crate) mod test { let mix_12: Box<dyn Isolation> = Box::new((token_1, other_2)); let revmix_11: Box<dyn Isolation> = Box::new((other_1, token_1)); - let join_token = token_12 - .join(token_12.as_ref()) - .expect("join should have returned Some"); + let join_token = token_12.join(token_12.as_ref()).unwrap(); assert!(join_token.compatible(token_12.as_ref())); - let join_mix = mix_12 - .join(mix_12.as_ref()) - .expect("join should have returned Some"); + let join_mix = mix_12.join(mix_12.as_ref()).unwrap(); assert!(join_mix.compatible(mix_12.as_ref())); let isol_list = [token_12, token_21, mix_11, mix_12, revmix_11]; @@ -485,4 +547,39 @@ pub(crate) mod test { } } } + + #[test] + fn build_isolation() { + let no_isolation = StreamIsolation::no_isolation(); + let no_isolation2 = StreamIsolation::builder() + .owner_token(IsolationToken::no_isolation()) + .stream_token(Box::new(IsolationToken::no_isolation())) + .build() + .unwrap(); + assert_eq!(no_isolation.owner_token, no_isolation2.owner_token); + assert_eq!( + no_isolation + .stream_token + .as_ref() + .as_any() + .downcast_ref::<IsolationToken>(), + no_isolation2 + .stream_token + .as_ref() + .as_any() + .downcast_ref::<IsolationToken>() + ); + assert!(no_isolation.may_share_circuit(&no_isolation2)); + + let tok = IsolationToken::new(); + let some_isolation = StreamIsolation::builder().owner_token(tok).build().unwrap(); + let some_isolation2 = StreamIsolation::builder() + .stream_token(Box::new(tok)) + .build() + .unwrap(); + assert!(!no_isolation.may_share_circuit(&some_isolation)); + assert!(!no_isolation.may_share_circuit(&some_isolation2)); + assert!(!some_isolation.may_share_circuit(&some_isolation2)); + assert!(some_isolation.may_share_circuit(&some_isolation)); + } } diff --git a/crates/tor-circmgr/src/lib.rs b/crates/tor-circmgr/src/lib.rs index 13e330b0e0..ceb60a7dab 100644 --- a/crates/tor-circmgr/src/lib.rs +++ b/crates/tor-circmgr/src/lib.rs @@ -73,13 +73,14 @@ mod usage; pub use err::Error; pub use isolation::IsolationToken; -pub use usage::{StreamIsolation, StreamIsolationBuilder, TargetPort, TargetPorts}; +pub use usage::{TargetPort, TargetPorts}; pub use config::{ CircMgrConfig, CircuitTiming, CircuitTimingBuilder, PathConfig, PathConfigBuilder, PreemptiveCircuitConfig, PreemptiveCircuitConfigBuilder, }; +use crate::isolation::StreamIsolation; use crate::preemptive::PreemptiveCircuitPredictor; use usage::TargetCircUsage; diff --git a/crates/tor-circmgr/src/usage.rs b/crates/tor-circmgr/src/usage.rs index 91975a36c4..f5e24e9406 100644 --- a/crates/tor-circmgr/src/usage.rs +++ b/crates/tor-circmgr/src/usage.rs @@ -13,7 +13,7 @@ use tor_netdir::Relay; use tor_netdoc::types::policy::PortPolicy; use tor_rtcompat::Runtime; -use crate::isolation::{Isolation, IsolationToken}; +use crate::isolation::StreamIsolation; use crate::mgr::{abstract_spec_find_supported, AbstractCirc, OpenEntry}; use crate::Result; @@ -100,64 +100,6 @@ impl Display for TargetPorts { } } -/// A set of information about how a stream should be isolated. -/// -/// If two streams are isolated from one another, they may not share -/// a circuit. -#[derive(Clone, Debug, derive_builder::Builder)] -pub struct StreamIsolation { - /// Any isolation token set on the stream. - #[builder(default = "Box::new(IsolationToken::no_isolation())")] - stream_token: Box<dyn Isolation>, - /// Any additional isolation token set on an object that "owns" this - /// stream. This is typically owned by a `TorClient`. - #[builder(default = "IsolationToken::no_isolation()")] - owner_token: IsolationToken, -} - -impl StreamIsolation { - /// Construct a new StreamIsolation with no isolation enabled. - pub fn no_isolation() -> Self { - StreamIsolationBuilder::new() - .build() - .expect("Bug constructing StreamIsolation") - } - - /// Return a new StreamIsolationBuilder for constructing - /// StreamIsolation objects. - pub fn builder() -> StreamIsolationBuilder { - StreamIsolationBuilder::new() - } - - /// Return true if this StreamIsolation can share a circuit with - /// `other`. - fn may_share_circuit(&self, other: &StreamIsolation) -> bool { - self.owner_token == other.owner_token - && self.stream_token.compatible(other.stream_token.as_ref()) - } - - /// Return a StreamIsolation that is the intersection of self and other. - /// Return None if such a merge is not possible. - pub fn join(&self, other: &StreamIsolation) -> Option<StreamIsolation> { - if self.owner_token != other.owner_token { - return None; - } - self.stream_token - .join(other.stream_token.as_ref()) - .map(|stream_token| StreamIsolation { - stream_token, - owner_token: self.owner_token, - }) - } -} - -impl StreamIsolationBuilder { - /// Construct a builder with no items set. - pub fn new() -> Self { - StreamIsolationBuilder::default() - } -} - impl ExitPolicy { /// Make a new exit policy from a given Relay. pub(crate) fn from_relay(relay: &Relay<'_>) -> Self { @@ -416,19 +358,13 @@ pub(crate) mod test { #![allow(clippy::unwrap_used)] use super::*; use crate::isolation::test::{assert_isoleq, IsolationTokenEq}; + use crate::isolation::{IsolationToken, StreamIsolationBuilder}; use crate::path::OwnedPath; use crate::test::OptDummyGuardMgr; use std::convert::TryFrom; use tor_linkspec::ChanTarget; use tor_netdir::testnet; - impl IsolationTokenEq for StreamIsolation { - fn isol_eq(&self, other: &Self) -> bool { - self.stream_token.isol_eq(other.stream_token.as_ref()) - && self.owner_token == other.owner_token - } - } - impl IsolationTokenEq for TargetCircUsage { fn isol_eq(&self, other: &Self) -> bool { use TargetCircUsage::*; @@ -813,41 +749,6 @@ pub(crate) mod test { assert_isoleq!(usage, SupportedCircUsage::NoUsage); } - #[test] - fn build_isolation() { - let no_isolation = StreamIsolation::no_isolation(); - let no_isolation2 = StreamIsolation::builder() - .owner_token(IsolationToken::no_isolation()) - .stream_token(Box::new(IsolationToken::no_isolation())) - .build() - .unwrap(); - assert_eq!(no_isolation.owner_token, no_isolation2.owner_token); - assert_eq!( - no_isolation - .stream_token - .as_ref() - .as_any() - .downcast_ref::<IsolationToken>(), - no_isolation2 - .stream_token - .as_ref() - .as_any() - .downcast_ref::<IsolationToken>() - ); - assert!(no_isolation.may_share_circuit(&no_isolation2)); - - let tok = IsolationToken::new(); - let some_isolation = StreamIsolation::builder().owner_token(tok).build().unwrap(); - let some_isolation2 = StreamIsolation::builder() - .stream_token(Box::new(tok)) - .build() - .unwrap(); - assert!(!no_isolation.may_share_circuit(&some_isolation)); - assert!(!no_isolation.may_share_circuit(&some_isolation2)); - assert!(!some_isolation.may_share_circuit(&some_isolation2)); - assert!(some_isolation.may_share_circuit(&some_isolation)); - } - #[test] fn display_target_ports() { let ports = []; -- GitLab