From 646f521a059dcb0e739411940fcea067804e634b Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Thu, 14 Mar 2024 14:39:57 +0100 Subject: [PATCH] Introduce lifetimes in `matcher.rs` The various matcher structs are very short-lived (their life is strictly bound by the life-cycle of a query), and it makes little sense to copy data into them everytime that they are used. --- .../src/relay_selector/matcher.rs | 74 +++++++++---------- .../src/relay_selector/mod.rs | 30 ++++---- 2 files changed, 48 insertions(+), 56 deletions(-) diff --git a/mullvad-relay-selector/src/relay_selector/matcher.rs b/mullvad-relay-selector/src/relay_selector/matcher.rs index b2bcd48fc13a..bca7cb39c4c4 100644 --- a/mullvad-relay-selector/src/relay_selector/matcher.rs +++ b/mullvad-relay-selector/src/relay_selector/matcher.rs @@ -14,7 +14,6 @@ use talpid_types::net::{IpVersion, TransportProtocol, TunnelType}; use super::query::{OpenVpnRelayQuery, RelayQuery, WireguardRelayQuery}; -#[derive(Clone)] pub struct RelayMatcher { /// Locations allowed to be picked from. In the case of custom lists this may be multiple /// locations. In normal circumstances this contains only 1 location. @@ -27,29 +26,22 @@ pub struct RelayMatcher { pub endpoint_matcher: T, } -impl RelayMatcher { +impl<'a> RelayMatcher> { pub fn new( - constraints: RelayQuery, - openvpn_data: OpenVpnEndpointData, + query: RelayQuery, + openvpn_data: &'a OpenVpnEndpointData, bridge_state: BridgeState, - wireguard_data: WireguardEndpointData, + wireguard_data: &'a WireguardEndpointData, custom_lists: &CustomListsSettings, - ) -> Self { + ) -> RelayMatcher> { Self { - locations: ResolvedLocationConstraint::from_constraint( - constraints.location, - custom_lists, - ), - providers: constraints.providers, - ownership: constraints.ownership, + locations: ResolvedLocationConstraint::from_constraint(query.location, custom_lists), + providers: query.providers, + ownership: query.ownership, endpoint_matcher: AnyTunnelMatcher { - wireguard: WireguardMatcher::new(constraints.wireguard_constraints, wireguard_data), - openvpn: OpenVpnMatcher::new( - constraints.openvpn_constraints, - openvpn_data, - bridge_state, - ), - tunnel_type: constraints.tunnel_protocol, + wireguard: WireguardMatcher::new(query.wireguard_constraints, wireguard_data), + openvpn: OpenVpnMatcher::new(query.openvpn_constraints, openvpn_data, bridge_state), + tunnel_type: query.tunnel_protocol, }, } } @@ -92,24 +84,23 @@ impl RelayMatcher { /// EndpointMatcher allows to abstract over different tunnel-specific or bridge constraints. /// This enables one to not have false dependencies on OpenVpn specific constraints when /// selecting only WireGuard tunnels. -pub trait EndpointMatcher: Clone { +pub trait EndpointMatcher { /// Returns whether the relay has matching endpoints. fn is_matching_relay(&self, relay: &Relay) -> bool; } -#[derive(Clone)] -pub struct AnyTunnelMatcher { +pub struct AnyTunnelMatcher<'a> { /// The [`WireguardMatcher`] to be used in case we should filter Wireguard relays. - pub wireguard: WireguardMatcher, + pub wireguard: WireguardMatcher<'a>, /// The [`OpenVpnMatcher`] to be used in case we should filter OpenVPN relays. - pub openvpn: OpenVpnMatcher, + pub openvpn: OpenVpnMatcher<'a>, /// If the user hasn't specified a tunnel protocol the relay selector might /// still prefer a specific tunnel protocol, which is why the tunnel type /// may be specified in the `AnyTunnelMatcher`. pub tunnel_type: Constraint, } -impl EndpointMatcher for AnyTunnelMatcher { +impl EndpointMatcher for AnyTunnelMatcher<'_> { fn is_matching_relay(&self, relay: &Relay) -> bool { match self.tunnel_type { Constraint::Any => { @@ -121,16 +112,19 @@ impl EndpointMatcher for AnyTunnelMatcher { } } -#[derive(Default, Clone)] -pub struct WireguardMatcher { +#[derive(Debug)] +pub struct WireguardMatcher<'a> { pub port: Constraint, pub ip_version: Constraint, - pub data: WireguardEndpointData, + pub data: &'a WireguardEndpointData, } /// Filter suitable Wireguard relays from the relay list -impl WireguardMatcher { - pub fn new(constraints: WireguardRelayQuery, data: WireguardEndpointData) -> Self { +impl<'a> WireguardMatcher<'a> { + pub fn new( + constraints: WireguardRelayQuery, + data: &'a WireguardEndpointData, + ) -> WireguardMatcher<'a> { Self { port: constraints.port, ip_version: constraints.ip_version, @@ -140,7 +134,7 @@ impl WireguardMatcher { pub fn new_matcher( constraints: RelayQuery, - data: WireguardEndpointData, + data: &'a WireguardEndpointData, custom_lists: &CustomListsSettings, ) -> RelayMatcher { RelayMatcher { @@ -155,23 +149,23 @@ impl WireguardMatcher { } } -impl EndpointMatcher for WireguardMatcher { +impl<'a> EndpointMatcher for WireguardMatcher<'a> { fn is_matching_relay(&self, relay: &Relay) -> bool { filter_wireguard(relay) } } /// Filter suitable OpenVPN relays from the relay list -#[derive(Debug, Clone)] -pub struct OpenVpnMatcher { +#[derive(Debug)] +pub struct OpenVpnMatcher<'a> { pub constraints: OpenVpnRelayQuery, - pub data: OpenVpnEndpointData, + pub data: &'a OpenVpnEndpointData, } -impl OpenVpnMatcher { +impl<'a> OpenVpnMatcher<'a> { pub fn new( mut constraints: OpenVpnRelayQuery, - data: OpenVpnEndpointData, + data: &'a OpenVpnEndpointData, bridge_state: BridgeState, ) -> Self { // Using bridges demands the selected endpoint to use TCP. @@ -189,13 +183,13 @@ impl OpenVpnMatcher { } } -impl EndpointMatcher for OpenVpnMatcher { +impl EndpointMatcher for OpenVpnMatcher<'_> { fn is_matching_relay(&self, relay: &Relay) -> bool { - filter_openvpn(relay) && openvpn_filter_on_port(self.constraints.port, &self.data) + filter_openvpn(relay) && openvpn_filter_on_port(self.constraints.port, self.data) } } -#[derive(Clone)] +#[derive(Debug)] pub struct BridgeMatcher; impl BridgeMatcher { diff --git a/mullvad-relay-selector/src/relay_selector/mod.rs b/mullvad-relay-selector/src/relay_selector/mod.rs index deb47f43fe0c..71cec6a6f297 100644 --- a/mullvad-relay-selector/src/relay_selector/mod.rs +++ b/mullvad-relay-selector/src/relay_selector/mod.rs @@ -572,12 +572,14 @@ impl RelaySelector { entry_relay_query.location = query.wireguard_constraints.entry_location.clone(); // After we have our two queries (one for the exit relay & one for the entry relay), // we can construct our two matchers: - let wg_data = parsed_relays.parsed_list().wireguard.clone(); - let exit_matcher = - WireguardMatcher::new_matcher(query.clone(), wg_data.clone(), config.custom_lists); + let exit_matcher = WireguardMatcher::new_matcher( + query.clone(), + &parsed_relays.parsed_list().wireguard, + config.custom_lists, + ); let entry_matcher = WireguardMatcher::new_matcher( entry_relay_query.clone(), - wg_data.clone(), + &parsed_relays.parsed_list().wireguard, config.custom_lists, ); // .. and query for all exit & entry candidates! All candidates are needed for the next step. @@ -844,17 +846,13 @@ impl RelaySelector { if constraints.location.is_any() { return None; } - let (openvpn_data, wireguard_data) = ( - parsed_relays.parsed_list().openvpn.clone(), - parsed_relays.parsed_list().wireguard.clone(), - ); let matcher = RelayMatcher::new( constraints.clone(), - openvpn_data, + &parsed_relays.parsed_list().openvpn, *config.bridge_state, - wireguard_data, - &config.custom_lists.clone(), + &parsed_relays.parsed_list().wireguard, + config.custom_lists, ); Self::get_relay_midpoint_inner(parsed_relays, matcher) @@ -862,7 +860,7 @@ impl RelaySelector { fn get_relay_midpoint_inner( parsed_relays: &ParsedRelays, - matcher: RelayMatcher, + matcher: RelayMatcher>, ) -> Option { use std::ops::Not; let matching_locations: Vec = matcher @@ -906,9 +904,9 @@ impl RelaySelector { let relays = parsed_relays.relays(); let matcher = RelayMatcher::new( query.clone(), - parsed_relays.parsed_list().openvpn.clone(), + &parsed_relays.parsed_list().openvpn, *config.bridge_state, - parsed_relays.parsed_list().wireguard.clone(), + &parsed_relays.parsed_list().wireguard, config.custom_lists, ); matcher.filter_matching_relay_list(relays) @@ -924,8 +922,8 @@ impl RelaySelector { let relays = parsed_relays.relays(); let matcher = WireguardMatcher::new_matcher( query.clone(), - parsed_relays.parsed_list().wireguard.clone(), - &config.custom_lists, + &parsed_relays.parsed_list().wireguard, + config.custom_lists, ); matcher.filter_matching_relay_list(relays) }