Skip to content

Commit

Permalink
Replace footgunny From implementations for RelayQuery
Browse files Browse the repository at this point in the history
  • Loading branch information
@dlon
dlon committed Aug 26, 2024
1 parent 156410e commit 161996d
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 184 deletions.
10 changes: 5 additions & 5 deletions mullvad-relay-selector/src/relay_selector/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,14 @@ impl<'a> From<NormalSelectorConfig<'a>> for RelayQuery {
bridge_settings: match bridge_state {
BridgeState::On => match bridge_settings.bridge_type {
mullvad_types::relay_constraints::BridgeType::Normal => {
Constraint::Only(BridgeQuery::Normal(bridge_settings.normal.clone()))
BridgeQuery::Normal(bridge_settings.normal.clone())
}
mullvad_types::relay_constraints::BridgeType::Custom => {
Constraint::Only(BridgeQuery::Custom(bridge_settings.custom.clone()))
BridgeQuery::Custom(bridge_settings.custom.clone())
}
},
BridgeState::Auto => Constraint::Only(BridgeQuery::Auto),
BridgeState::Off => Constraint::Only(BridgeQuery::Off),
BridgeState::Auto => BridgeQuery::Auto,
BridgeState::Off => BridgeQuery::Off,
},
}
}
Expand Down Expand Up @@ -910,7 +910,7 @@ impl RelaySelector {
if !BridgeQuery::should_use_bridge(&query.openvpn_constraints.bridge_settings) {
Ok(None)
} else {
let bridge_query = &query.openvpn_constraints.bridge_settings.clone().unwrap();
let bridge_query = &query.openvpn_constraints.bridge_settings;
let custom_lists = &custom_lists;
match protocol {
TransportProtocol::Udp => {
Expand Down
176 changes: 104 additions & 72 deletions mullvad-relay-selector/src/relay_selector/query.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ use crate::AdditionalWireguardConstraints;
use mullvad_types::{
constraints::Constraint,
relay_constraints::{
BridgeConstraints, LocationConstraint, ObfuscationSettings, OpenVpnConstraints, Ownership,
Providers, RelayConstraints, SelectedObfuscation, ShadowsocksSettings, TransportPort,
Udp2TcpObfuscationSettings, WireguardConstraints,
BridgeConstraints, BridgeSettings, BridgeState, BridgeType, LocationConstraint,
ObfuscationSettings, OpenVpnConstraints, Ownership, Providers, RelayConstraints,
SelectedObfuscation, ShadowsocksSettings, TransportPort, Udp2TcpObfuscationSettings,
WireguardConstraints,
},
Intersection,
};
Expand Down Expand Up @@ -108,41 +109,35 @@ impl RelayQuery {
}
}

/// The mapping from [`RelayQuery`] to [`RelayConstraints`]. Note that this does not contain
/// obfuscation or bridge settings.
pub fn into_relay_constraints(self) -> RelayConstraints {
RelayConstraints {
/// The mapping from [`RelayQuery`] to all underlying settings types.
pub fn into_settings(
self,
) -> (
RelayConstraints,
BridgeState,
BridgeSettings,
ObfuscationSettings,
) {
let (bridge_state, bridge_settings) = self
.openvpn_constraints
.bridge_settings
.clone()
.into_settings();
let obfuscation = self
.wireguard_constraints
.obfuscation
.clone()
.into_settings();
let constraints = RelayConstraints {
location: self.location,
providers: self.providers,
ownership: self.ownership,
tunnel_protocol: self.tunnel_protocol,
wireguard_constraints: WireguardConstraints::from(self.wireguard_constraints),
openvpn_constraints: OpenVpnConstraints::from(self.openvpn_constraints),
}
}
wireguard_constraints: self.wireguard_constraints.into_constraints(),
openvpn_constraints: self.openvpn_constraints.into_constraints(),
};

/// The mapping from [`RelayQuery`] to [`ObfuscationSettings`]
pub fn into_obfuscation_settings(self) -> ObfuscationSettings {
match self.wireguard_constraints.obfuscation {
ObfuscationQuery::Auto => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Auto,
..Default::default()
},
ObfuscationQuery::Off => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Off,
..Default::default()
},
ObfuscationQuery::Udp2tcp(settings) => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Udp2Tcp,
udp2tcp: settings,
..Default::default()
},
ObfuscationQuery::Shadowsocks(settings) => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Shadowsocks,
shadowsocks: settings,
..Default::default()
},
}
(constraints, bridge_state, bridge_settings, obfuscation)
}
}

Expand Down Expand Up @@ -180,6 +175,31 @@ pub enum ObfuscationQuery {
Shadowsocks(ShadowsocksSettings),
}

impl ObfuscationQuery {
fn into_settings(self) -> ObfuscationSettings {
match self {
ObfuscationQuery::Auto => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Auto,
..Default::default()
},
ObfuscationQuery::Off => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Off,
..Default::default()
},
ObfuscationQuery::Udp2tcp(settings) => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Udp2Tcp,
udp2tcp: settings,
..Default::default()
},
ObfuscationQuery::Shadowsocks(settings) => ObfuscationSettings {
selected_obfuscation: SelectedObfuscation::Shadowsocks,
shadowsocks: settings,
..Default::default()
},
}
}
}

impl From<ObfuscationSettings> for ObfuscationQuery {
/// A query for obfuscation settings.
///
Expand Down Expand Up @@ -230,6 +250,16 @@ impl WireguardRelayQuery {
daita: Constraint::Any,
}
}

/// The mapping from [`WireguardRelayQuery`] to [`WireguardConstraints`].
fn into_constraints(self) -> WireguardConstraints {
WireguardConstraints {
port: self.port,
ip_version: self.ip_version,
entry_location: self.entry_location,
use_multihop: self.use_multihop.unwrap_or(false),
}
}
}

impl Default for WireguardRelayQuery {
Expand All @@ -238,18 +268,6 @@ impl Default for WireguardRelayQuery {
}
}

impl From<WireguardRelayQuery> for WireguardConstraints {
/// The mapping from [`WireguardRelayQuery`] to [`WireguardConstraints`].
fn from(value: WireguardRelayQuery) -> Self {
WireguardConstraints {
port: value.port,
ip_version: value.ip_version,
entry_location: value.entry_location,
use_multihop: value.use_multihop.unwrap_or(false),
}
}
}

impl From<WireguardRelayQuery> for AdditionalWireguardConstraints {
/// The mapping from [`WireguardRelayQuery`] to [`AdditionalWireguardConstraints`].
fn from(value: WireguardRelayQuery) -> Self {
Expand All @@ -270,16 +288,21 @@ impl From<WireguardRelayQuery> for AdditionalWireguardConstraints {
#[derive(Debug, Clone, Eq, PartialEq, Intersection)]
pub struct OpenVpnRelayQuery {
pub port: Constraint<TransportPort>,
pub bridge_settings: Constraint<BridgeQuery>,
pub bridge_settings: BridgeQuery,
}

impl OpenVpnRelayQuery {
pub const fn new() -> OpenVpnRelayQuery {
OpenVpnRelayQuery {
port: Constraint::Any,
bridge_settings: Constraint::Any,
bridge_settings: BridgeQuery::Auto,
}
}

/// The mapping from [`OpenVpnRelayQuery`] to [`OpenVpnConstraints`].
fn into_constraints(self) -> OpenVpnConstraints {
OpenVpnConstraints { port: self.port }
}
}

impl Default for OpenVpnRelayQuery {
Expand All @@ -293,14 +316,15 @@ impl Default for OpenVpnRelayQuery {
///
/// [`BridgeState`]: mullvad_types::relay_constraints::BridgeState
/// [`BridgeSettings`]: mullvad_types::relay_constraints::BridgeSettings
#[derive(Debug, Clone, Eq, PartialEq)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub enum BridgeQuery {
/// Bridges should not be used.
Off,
/// Don't care, let the relay selector choose!
///
/// If this variant is intersected with another [`BridgeQuery`] `bq`,
/// `bq` is always preferred.
#[default]
Auto,
/// Bridges should be used.
Normal(BridgeConstraints),
Expand All @@ -309,22 +333,38 @@ pub enum BridgeQuery {
}

impl BridgeQuery {
/// If `bridge_constraints` is `Any`, bridges should not be used due to
/// latency concerns.
///
/// If `bridge_constraints` is `Only(settings)`, then `settings` will be
/// used to decide if bridges should be used. See [`BridgeQuery`] for more
/// details, but the algorithm beaks down to this:
/// `settings` will be used to decide if bridges should be used. See [`BridgeQuery`]
/// for more details, but the algorithm beaks down to this:
///
/// * `BridgeQuery::Off`: bridges will not be used
/// * otherwise: bridges should be used
pub const fn should_use_bridge(bridge_constraints: &Constraint<BridgeQuery>) -> bool {
match bridge_constraints {
Constraint::Only(settings) => match settings {
BridgeQuery::Normal(_) | BridgeQuery::Custom(_) => true,
BridgeQuery::Off | BridgeQuery::Auto => false,
},
Constraint::Any => false,
pub const fn should_use_bridge(settings: &BridgeQuery) -> bool {
match settings {
BridgeQuery::Normal(_) | BridgeQuery::Custom(_) => true,
BridgeQuery::Off | BridgeQuery::Auto => false,
}
}

fn into_settings(self) -> (BridgeState, BridgeSettings) {
match self {
BridgeQuery::Off => (BridgeState::Off, Default::default()),
BridgeQuery::Auto => (BridgeState::Auto, Default::default()),
BridgeQuery::Normal(constraints) => (
BridgeState::On,
BridgeSettings {
bridge_type: BridgeType::Normal,
normal: constraints,
custom: None,
},
),
BridgeQuery::Custom(custom) => (
BridgeState::On,
BridgeSettings {
bridge_type: BridgeType::Normal,
normal: Default::default(),
custom,
},
),
}
}
}
Expand All @@ -347,13 +387,6 @@ impl Intersection for BridgeQuery {
}
}

impl From<OpenVpnRelayQuery> for OpenVpnConstraints {
/// The mapping from [`OpenVpnRelayQuery`] to [`OpenVpnConstraints`].
fn from(value: OpenVpnRelayQuery) -> Self {
OpenVpnConstraints { port: value.port }
}
}

#[allow(unused)]
pub mod builder {
//! Strongly typed Builder pattern for of relay constraints though the use of the Typestate
Expand Down Expand Up @@ -674,8 +707,7 @@ pub mod builder {
bridge_settings: bridge_settings.clone(),
};

self.query.openvpn_constraints.bridge_settings =
Constraint::Only(BridgeQuery::Normal(bridge_settings));
self.query.openvpn_constraints.bridge_settings = BridgeQuery::Normal(bridge_settings);

let builder = RelayQueryBuilder {
query: self.query,
Expand All @@ -692,14 +724,14 @@ pub mod builder {
self.protocol.bridge_settings.location =
Constraint::Only(LocationConstraint::from(location));
self.query.openvpn_constraints.bridge_settings =
Constraint::Only(BridgeQuery::Normal(self.protocol.bridge_settings.clone()));
BridgeQuery::Normal(self.protocol.bridge_settings.clone());
self
}
/// Constrain the [`Providers`] of the selected bridge.
pub fn bridge_providers(mut self, providers: Providers) -> Self {
self.protocol.bridge_settings.providers = Constraint::Only(providers);
self.query.openvpn_constraints.bridge_settings =
Constraint::Only(BridgeQuery::Normal(self.protocol.bridge_settings.clone()));
BridgeQuery::Normal(self.protocol.bridge_settings.clone());
self
}
/// Constrain the [`Ownership`] of the selected bridge.
Expand Down
7 changes: 3 additions & 4 deletions mullvad-relay-selector/tests/relay_selector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1277,8 +1277,7 @@ fn openvpn_handle_bridge_settings() {
}
// Tweaking the query just slightly to try to enable bridge mode, while sill using UDP,
// should fail.
query.openvpn_constraints.bridge_settings =
Constraint::Only(BridgeQuery::Normal(BridgeConstraints::default()));
query.openvpn_constraints.bridge_settings = BridgeQuery::Normal(BridgeConstraints::default());
let relay = relay_selector.get_relay_by_query(query.clone());
assert!(relay.is_err());

Expand Down Expand Up @@ -1514,10 +1513,10 @@ fn valid_user_setting_should_yield_relay() {
// Make a valid user relay constraint
let location = GeographicLocationConstraint::hostname("se", "got", "se9-wireguard");
let user_query = RelayQueryBuilder::new().location(location.clone()).build();
let user_constraints = RelayQueryBuilder::new()
let (user_constraints, ..) = RelayQueryBuilder::new()
.location(location.clone())
.build()
.into_relay_constraints();
.into_settings();

let config = SelectorConfig {
relay_settings: user_constraints.into(),
Expand Down
Loading

0 comments on commit 161996d

Please sign in to comment.