Intersection derive macro

Cargo.toml

@@ -24,6 +24,7 @@ members = [
     "mullvad-relay-selector",
     "mullvad-setup",
     "mullvad-types",
+    "mullvad-types/intersection-derive",
     "mullvad-version",
     "talpid-core",
     "talpid-dbus",

mullvad-relay-selector/Cargo.toml

@@ -22,6 +22,7 @@ serde_json = "1.0"
 
 talpid-types = { path = "../talpid-types" }
 mullvad-types = { path = "../mullvad-types" }
+intersection-derive = { path = "../mullvad-types/intersection-derive"}
 
 [dev-dependencies]
 proptest = { workspace = true }

mullvad-relay-selector/src/relay_selector/detailer.rs

@@ -48,7 +48,8 @@ pub enum Error {
 /// Constructs a [`MullvadWireguardEndpoint`] with details for how to connect to a Wireguard relay.
 ///
 /// # Returns
-/// - A configured endpoint for Wireguard relay, encapsulating either a single-hop or multi-hop connection.
+/// - A configured endpoint for Wireguard relay, encapsulating either a single-hop or multi-hop
+///   connection.
 /// - Returns [`Option::None`] if the desired port is not in a valid port range (see
 /// [`WireguardRelayQuery::port`]) or relay addresses cannot be resolved.
 pub fn wireguard_endpoint(
@@ -198,8 +199,8 @@ const fn get_public_key(relay: &Relay) -> Result<&PublicKey, Error> {
 /// - `port_ranges`: A slice of tuples, each representing a range of valid port numbers.
 ///
 /// # Returns
-/// - `Option<u16>`: A randomly selected port number within the given ranges, or `None` if
-///   the input is empty or the total number of available ports is zero.
+/// - `Option<u16>`: A randomly selected port number within the given ranges, or `None` if the input
+///   is empty or the total number of available ports is zero.
 fn select_random_port(port_ranges: &[(u16, u16)]) -> Result<u16, Error> {
     use rand::Rng;
     let get_port_amount = |range: &(u16, u16)| -> u64 { (1 + range.1 - range.0) as u64 };

mullvad-relay-selector/src/relay_selector/helpers.rs

@@ -8,24 +8,11 @@ use mullvad_types::{
     relay_constraints::Udp2TcpObfuscationSettings,
     relay_list::{BridgeEndpointData, Relay, RelayEndpointData},
 };
-use rand::{
-    seq::{IteratorRandom, SliceRandom},
-    thread_rng, Rng,
-};
+use rand::{seq::SliceRandom, thread_rng, Rng};
 use talpid_types::net::{obfuscation::ObfuscatorConfig, proxy::CustomProxy};
 
 use crate::SelectedObfuscator;
 
-/// Pick a random element out of `from`, excluding the element `exclude` from the selection.
-pub fn random<'a, A: PartialEq>(
-    from: impl IntoIterator<Item = &'a A>,
-    exclude: &A,
-) -> Option<&'a A> {
-    from.into_iter()
-        .filter(|&a| a != exclude)
-        .choose(&mut thread_rng())
-}
-
 /// Picks a relay using [pick_random_relay_fn], using the `weight` member of each relay
 /// as the weight function.
 pub fn pick_random_relay(relays: &[Relay]) -> Option<&Relay> {
@@ -49,16 +36,16 @@ pub fn pick_random_relay_weighted<RelayType>(
         // Pick a random number in the range 1..=total_weight. This choses the relay with a
         // non-zero weight.
         //
-        //                                rng(1..=total_weight)
-        //                                |
-        //                                v
-        //   _____________________________i___________________________________________________
-        // 0|_____________|__________________________|___________|_____|___________|__________| total_weight
-        //  ^             ^                          ^                             ^          ^
-        //  |             |                          |                             |          |
-        //  ------------------------------------------                             ------------
-        //         |                    |                                                |
-        //   weight(relay 0)       weight(relay 1)        ..       ..       ..     weight(relay n)
+        //                           rng(1..=total_weight)
+        //                           |
+        //                           v
+        //   ________________________i_______________________________________________
+        // 0|_____________|____________________|___________|_____|________|__________| total_weight
+        //  ^             ^                    ^                          ^          ^
+        //  |             |                    |                          |          |
+        //  ------------------------------------                          ------------
+        //         |                  |                                         |
+        //   weight(relay 0)     weight(relay 1)    ..       ..     ..    weight(relay n)
         let mut i: u64 = rng.gen_range(1..=total_weight);
         Some(
             relays

mullvad-relay-selector/src/relay_selector/matcher.rs

@@ -35,10 +35,10 @@ pub fn filter_matching_relay_list<'a, R: Iterator<Item = &'a Relay> + Clone>(
             .filter(|relay| filter_on_providers(&query.providers, relay));
 
     // The last filtering to be done is on the `include_in_country` attribute found on each
-    // relay. When the location constraint is based on country, a relay which has `include_in_country`
-    // set to true should always be prioritized over relays which has this flag set to false.
-    // We should only consider relays with `include_in_country` set to false if there are no
-    // other candidates left.
+    // relay. When the location constraint is based on country, a relay which has
+    // `include_in_country` set to true should always be prioritized over relays which has this
+    // flag set to false. We should only consider relays with `include_in_country` set to false
+    // if there are no other candidates left.
     match &locations {
         Constraint::Any => shortlist.cloned().collect(),
         Constraint::Only(locations) => {

mullvad-relay-selector/src/relay_selector/mod.rs

@@ -9,6 +9,7 @@ pub mod query;
 use chrono::{DateTime, Local};
 use itertools::Itertools;
 use once_cell::sync::Lazy;
+use rand::{seq::IteratorRandom, thread_rng};
 use std::{
     path::Path,
     sync::{Arc, Mutex},
@@ -27,7 +28,7 @@ use mullvad_types::{
     },
     relay_list::{Relay, RelayEndpointData, RelayList},
     settings::Settings,
-    CustomTunnelEndpoint,
+    CustomTunnelEndpoint, Intersection,
 };
 use talpid_types::{
     net::{
@@ -42,12 +43,12 @@ use self::{
     detailer::{openvpn_endpoint, wireguard_endpoint},
     matcher::{filter_matching_bridges, filter_matching_relay_list},
     parsed_relays::ParsedRelays,
-    query::{BridgeQuery, Intersection, OpenVpnRelayQuery, RelayQuery, WireguardRelayQuery},
+    query::{BridgeQuery, OpenVpnRelayQuery, RelayQuery, WireguardRelayQuery},
 };
 
-/// [`RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should prioritize on
-/// successive connection attempts. Note that these will *never* override user preferences.
-/// See [the documentation on `RelayQuery`][RelayQuery] for further details.
+/// [`RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
+/// prioritize on successive connection attempts. Note that these will *never* override user
+/// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
 ///
 /// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
 pub static RETRY_ORDER: Lazy<Vec<RelayQuery>> = Lazy::new(|| {
@@ -144,14 +145,16 @@ impl Default for RuntimeParameters {
 
 /// This enum exists to separate the two types of [`SelectorConfig`] that exists.
 ///
-/// The first one is a "regular" config, where [`SelectorConfig::relay_settings`] is [`RelaySettings::Normal`].
-/// This is the most common variant, and there exists a mapping from this variant to [`RelayQueryBuilder`].
-/// Being able to implement `From<NormalSelectorConfig> for RelayQueryBuilder` was the main
-/// motivator for introducing these seemingly useless derivates of [`SelectorConfig`].
+/// The first one is a "regular" config, where [`SelectorConfig::relay_settings`] is
+/// [`RelaySettings::Normal`]. This is the most common variant, and there exists a mapping from this
+/// variant to [`RelayQueryBuilder`]. Being able to implement `From<NormalSelectorConfig> for
+/// RelayQueryBuilder` was the main motivator for introducing these seemingly useless derivates of
+/// [`SelectorConfig`].
 ///
-/// The second one is a custom config, where [`SelectorConfig::relay_settings`] is [`RelaySettings::Custom`].
-/// For this variant, the endpoint where the client should connect to is already specified inside of the variant,
-/// so in practice the relay selector becomes superfluous. Also, there exists no mapping to [`RelayQueryBuilder`].
+/// The second one is a custom config, where [`SelectorConfig::relay_settings`] is
+/// [`RelaySettings::Custom`]. For this variant, the endpoint where the client should connect to is
+/// already specified inside of the variant, so in practice the relay selector becomes superfluous.
+/// Also, there exists no mapping to [`RelayQueryBuilder`].
 #[derive(Debug, Clone)]
 enum SpecializedSelectorConfig<'a> {
     // This variant implements `From<NormalSelectorConfig> for RelayQuery`
@@ -500,9 +503,9 @@ impl RelaySelector {
         self.get_relay_with_custom_params(retry_attempt, &RETRY_ORDER, runtime_params)
     }
 
-    /// Peek at which [`TunnelType`] that would be returned for a certain connection attempt for a given
-    /// [`SelectorConfig`]. Returns [`Option::None`] if the given config would return a custom
-    /// tunnel endpoint.
+    /// Peek at which [`TunnelType`] that would be returned for a certain connection attempt for a
+    /// given [`SelectorConfig`]. Returns [`Option::None`] if the given config would return a
+    /// custom tunnel endpoint.
     ///
     /// # Note
     /// This function is only really useful for testing-purposes. It is exposed to ease testing of
@@ -556,15 +559,16 @@ impl RelaySelector {
         }
     }
 
-    /// This function defines the merge between a set of pre-defined queries and `user_preferences` for the given
-    /// `retry_attempt`.
+    /// This function defines the merge between a set of pre-defined queries and `user_preferences`
+    /// for the given `retry_attempt`.
     ///
-    /// This algorithm will loop back to the start of `retry_order` if `retry_attempt < retry_order.len()`.
-    /// If `user_preferences` is not compatible with any of the pre-defined queries in `retry_order`, `user_preferences`
-    /// is returned.
+    /// This algorithm will loop back to the start of `retry_order` if `retry_attempt <
+    /// retry_order.len()`. If `user_preferences` is not compatible with any of the pre-defined
+    /// queries in `retry_order`, `user_preferences` is returned.
     ///
     /// Runtime parameters may affect which of the default queries that are considered. For example,
-    /// queries which rely on IPv6 will not be considered if working IPv6 is not available at runtime.
+    /// queries which rely on IPv6 will not be considered if working IPv6 is not available at
+    /// runtime.
     fn pick_and_merge_query(
         retry_attempt: usize,
         retry_order: &[RelayQuery],
@@ -583,15 +587,19 @@ impl RelaySelector {
             .unwrap_or(user_preferences)
     }
 
-    /// "Execute" the given query, yielding a final set of relays and/or bridges which the VPN traffic shall be routed through.
+    /// "Execute" the given query, yielding a final set of relays and/or bridges which the VPN
+    /// traffic shall be routed through.
     ///
     /// # Parameters
-    /// - `query`: Constraints that filter the available relays, such as geographic location or tunnel protocol.
-    /// - `config`: Configuration settings that influence relay selection, including bridge state and custom lists.
+    /// - `query`: Constraints that filter the available relays, such as geographic location or
+    ///   tunnel protocol.
+    /// - `config`: Configuration settings that influence relay selection, including bridge state
+    ///   and custom lists.
     /// - `parsed_relays`: The complete set of parsed relays available for selection.
     ///
     /// # Returns
-    /// * A randomly selected relay that meets the specified constraints (and a random bridge/entry relay if applicable).
+    /// * A randomly selected relay that meets the specified constraints (and a random bridge/entry
+    ///   relay if applicable).
     /// See [`GetRelay`] for more details.
     /// * An `Err` if no suitable relay is found
     /// * An `Err` if no suitable bridge is found
@@ -629,10 +637,10 @@ impl RelaySelector {
         parsed_relays: &ParsedRelays,
         config: &NormalSelectorConfig<'_>,
     ) -> Result<GetRelay, Error> {
-        // FIXME: A bit of defensive programming - calling `get_wiregurad_relay` with a query that doesn't
-        // specify Wireguard as the desired tunnel type is not valid and will lead to unwanted
-        // behavior. This should be seen as a workaround, and it would be nicer to lift this
-        // invariant to be checked by the type system instead.
+        // FIXME: A bit of defensive programming - calling `get_wiregurad_relay` with a query that
+        // doesn't specify Wireguard as the desired tunnel type is not valid and will lead
+        // to unwanted behavior. This should be seen as a workaround, and it would be nicer
+        // to lift this invariant to be checked by the type system instead.
         query.tunnel_protocol = Constraint::Only(TunnelType::Wireguard);
         Self::get_wireguard_relay(query, config, parsed_relays)
     }
@@ -712,7 +720,8 @@ impl RelaySelector {
         let mut entry_relay_query = query.clone();
         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 query for all exit & entry candidates! All candidates are needed for the next step.
+        // we can query for all exit & entry candidates! All candidates are needed for the next
+        // step.
         let exit_candidates =
             filter_matching_relay_list(query, parsed_relays.relays(), config.custom_lists);
         let entry_candidates = filter_matching_relay_list(
@@ -721,25 +730,25 @@ impl RelaySelector {
             config.custom_lists,
         );
 
-        // This algorithm gracefully handles a particular edge case that arise when a constraint on
-        // the exit relay is more specific than on the entry relay which forces the relay selector
-        // to choose one specific relay. The relay selector could end up selecting that specific
-        // relay as the entry relay, thus leaving no remaining exit relay candidates or vice versa.
+        fn pick_random_excluding<'a>(list: &'a [Relay], exclude: &'a Relay) -> Option<&'a Relay> {
+            list.iter()
+                .filter(|&a| a != exclude)
+                .choose(&mut thread_rng())
+        }
+        // We avoid picking the same relay for entry and exit by choosing one and excluding it when
+        // choosing the other.
         let (exit, entry) = match (exit_candidates.as_slice(), entry_candidates.as_slice()) {
-            ([exit], [entry]) if exit == entry => None,
+            // In the case where there is only one entry to choose from, we have to pick it before
+            // the exit
             (exits, [entry]) if exits.contains(entry) => {
-                let exit = helpers::random(exits, entry).ok_or(Error::NoRelay)?;
-                Some((exit, entry))
-            }
-            ([exit], entrys) if entrys.contains(exit) => {
-                let entry = helpers::random(entrys, exit).ok_or(Error::NoRelay)?;
-                Some((exit, entry))
+                pick_random_excluding(exits, entry).map(|exit| (exit, entry))
             }
-            (exits, entrys) => {
-                let exit = helpers::pick_random_relay(exits).ok_or(Error::NoRelay)?;
-                let entry = helpers::random(entrys, exit).ok_or(Error::NoRelay)?;
-                Some((exit, entry))
+            // Vice versa for the case of only one exit
+            ([exit], entries) if entries.contains(exit) => {
+                pick_random_excluding(entries, exit).map(|entry| (exit, entry))
             }
+            (exits, entries) => helpers::pick_random_relay(exits)
+                .and_then(|exit| pick_random_excluding(entries, exit).map(|entry| (exit, entry))),
         }
         .ok_or(Error::NoRelay)?;
 
@@ -847,17 +856,20 @@ impl RelaySelector {
         })
     }
 
-    /// Selects a suitable bridge based on the specified settings, relay information, and transport protocol.
+    /// Selects a suitable bridge based on the specified settings, relay information, and transport
+    /// protocol.
     ///
     /// # Parameters
     /// - `query`: The filter criteria for selecting a bridge.
     /// - `relay`: Information about the current relay, including its location.
     /// - `protocol`: The transport protocol (TCP or UDP) in use.
     /// - `parsed_relays`: A structured representation of all available relays.
-    /// - `custom_lists`: User-defined or application-specific settings that may influence bridge selection.
+    /// - `custom_lists`: User-defined or application-specific settings that may influence bridge
+    ///   selection.
     ///
     /// # Returns
-    /// * On success, returns an `Option` containing the selected bridge, if one is found. Returns `None` if no suitable bridge meets the criteria or bridges should not be used.
+    /// * On success, returns an `Option` containing the selected bridge, if one is found. Returns
+    ///   `None` if no suitable bridge meets the criteria or bridges should not be used.
     /// * `Error::NoBridge` if attempting to use OpenVPN bridges over UDP, as this is unsupported.
     /// * `Error::NoRelay` if `relay` does not have a location set.
     #[cfg(not(target_os = "android"))]
@@ -1010,7 +1022,8 @@ impl RelaySelector {
     }
 
     /// # Returns
-    /// A randomly selected relay that meets the specified constraints, or `None` if no suitable relay is found.
+    /// A randomly selected relay that meets the specified constraints, or `None` if no suitable
+    /// relay is found.
     #[cfg(not(target_os = "android"))]
     fn choose_openvpn_relay(
         query: &RelayQuery,