Skip to content

Commit

Permalink
Remove Option from Relay::location
Browse files Browse the repository at this point in the history
  • Loading branch information
hulthe committed Aug 19, 2024
1 parent c10d39e commit ba39531
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 88 deletions.
2 changes: 1 addition & 1 deletion mullvad-api/src/relay_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ fn into_mullvad_relay(
provider: relay.provider,
weight: relay.weight,
endpoint_data,
location: Some(location),
location,
}
}

Expand Down
23 changes: 9 additions & 14 deletions mullvad-cli/src/cmds/relay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use itertools::Itertools;
use mullvad_management_interface::MullvadProxyClient;
use mullvad_types::{
constraints::{Constraint, Match},
location::{CountryCode, Location},
location::CountryCode,
relay_constraints::{
GeographicLocationConstraint, LocationConstraint, LocationConstraintFormatter,
OpenVpnConstraints, Ownership, Provider, Providers, RelayConstraints, RelayOverride,
Expand Down Expand Up @@ -921,15 +921,11 @@ fn parse_transport_port(

fn relay_to_geographical_constraint(
relay: mullvad_types::relay_list::Relay,
) -> Option<GeographicLocationConstraint> {
relay.location.map(
|Location {
country_code,
city_code,
..
}| {
GeographicLocationConstraint::Hostname(country_code, city_code, relay.hostname)
},
) -> GeographicLocationConstraint {
GeographicLocationConstraint::Hostname(
relay.location.country_code,
relay.location.city_code,
relay.hostname,
)
}

Expand All @@ -952,10 +948,9 @@ pub async fn resolve_location_constraint(
.find(|relay| relay.hostname.to_lowercase() == location_constraint_args.country)
{
if relay_filter(&matching_relay) {
Ok(Constraint::Only(
relay_to_geographical_constraint(matching_relay)
.context("Selected relay did not contain a valid location")?,
))
Ok(Constraint::Only(relay_to_geographical_constraint(
matching_relay,
)))
} else {
bail!(
"The relay `{}` is not valid for this operation",
Expand Down
4 changes: 2 additions & 2 deletions mullvad-daemon/src/tunnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ impl ParametersGenerator {
hostname = exit.hostname.clone();
obfuscator_hostname = take_hostname(obfuscator);
bridge_hostname = None;
location = exit.location.as_ref().cloned().unwrap();
location = exit.location.clone();
}
#[cfg(not(target_os = "android"))]
LastSelectedRelays::OpenVpn { relay, bridge } => {
hostname = relay.hostname.clone();
bridge_hostname = take_hostname(bridge);
entry_hostname = None;
obfuscator_hostname = None;
location = relay.location.as_ref().cloned().unwrap();
location = relay.location.clone();
}
};

Expand Down
34 changes: 19 additions & 15 deletions mullvad-management-interface/src/types/conversions/relay_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ impl From<mullvad_types::relay_list::Relay> for proto::Relay {
)),
_ => None,
},
location: relay.location.map(|location| proto::Location {
country: location.country,
country_code: location.country_code,
city: location.city,
city_code: location.city_code,
latitude: location.latitude,
longitude: location.longitude,
location: Some(proto::Location {
country: relay.location.country,
country_code: relay.location.country_code,
city: relay.location.city,
city_code: relay.location.city_code,
latitude: relay.location.latitude,
longitude: relay.location.longitude,
}),
}
}
Expand Down Expand Up @@ -297,14 +297,18 @@ impl TryFrom<proto::Relay> for mullvad_types::relay_list::Relay {
provider: relay.provider,
weight: relay.weight,
endpoint_data,
location: relay.location.map(|location| MullvadLocation {
country: location.country,
country_code: location.country_code,
city: location.city,
city_code: location.city_code,
latitude: location.latitude,
longitude: location.longitude,
}),
location: relay
.location
.map(|location| MullvadLocation {
country: location.country,
country_code: location.country_code,
city: location.city,
city_code: location.city_code,
latitude: location.latitude,
longitude: location.longitude,
})
.ok_or("missing relay location")
.map_err(FromProtobufTypeError::InvalidArgument)?,
})
}
}
Expand Down
7 changes: 3 additions & 4 deletions mullvad-relay-selector/src/relay_selector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,10 +971,9 @@ impl RelaySelector {
Err(Error::NoBridge)
}
TransportProtocol::Tcp => {
let location = relay.location.as_ref().ok_or(Error::NoRelay)?;
Self::get_bridge_for(
bridge_query,
location,
&relay.location,
// FIXME: This is temporary while talpid-core only supports TCP proxies
TransportProtocol::Tcp,
parsed_relays,
Expand Down Expand Up @@ -1054,7 +1053,7 @@ impl RelaySelector {
let matching_bridges: Vec<RelayWithDistance> = relays
.into_iter()
.map(|relay| RelayWithDistance {
distance: relay.location.as_ref().unwrap().distance_from(&location),
distance: relay.location.distance_from(&location),
relay,
})
.sorted_unstable_by_key(|relay| relay.distance as usize)
Expand Down Expand Up @@ -1092,7 +1091,7 @@ impl RelaySelector {
let matching_locations: Vec<Location> =
filter_matching_relay_list(query, parsed_relays, custom_lists)
.into_iter()
.filter_map(|relay| relay.location)
.map(|relay| relay.location)
.unique_by(|location| location.city.clone())
.collect();

Expand Down
4 changes: 2 additions & 2 deletions mullvad-relay-selector/src/relay_selector/parsed_relays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,14 @@ impl ParsedRelays {
for city in &mut country.cities {
for relay in &mut city.relays {
// Append location data
relay.location = Some(Location {
relay.location = Location {
country: country.name.clone(),
country_code: country.code.clone(),
city: city.name.clone(),
city_code: city.code.clone(),
latitude: city.latitude,
longitude: city.longitude,
});
};

// Append overrides
if let Some(overrides) = remaining_overrides.remove(&relay.hostname) {
Expand Down
32 changes: 21 additions & 11 deletions mullvad-relay-selector/tests/relay_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use mullvad_relay_selector::{
use mullvad_types::{
constraints::Constraint,
endpoint::MullvadEndpoint,
location::Location,
relay_constraints::{
BridgeConstraints, BridgeState, GeographicLocationConstraint, LocationConstraint,
Ownership, Providers, RelayOverride, TransportPort,
Expand All @@ -32,6 +33,15 @@ use mullvad_types::{
},
};

static DUMMY_LOCATION: Lazy<Location> = Lazy::new(|| Location {
country: "Sweden".to_string(),
country_code: "se".to_string(),
city: "Gothenburg".to_string(),
city_code: "got".to_string(),
latitude: 57.71,
longitude: 11.97,
});

static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
etag: None,
countries: vec![RelayListCountry {
Expand Down Expand Up @@ -60,7 +70,7 @@ static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
daita: false,
shadowsocks_extra_addr_in: vec![],
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se10-wireguard".to_string(),
Expand All @@ -79,7 +89,7 @@ static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
daita: false,
shadowsocks_extra_addr_in: vec![],
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se-got-001".to_string(),
Expand All @@ -91,7 +101,7 @@ static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
provider: "provider2".to_string(),
weight: 1,
endpoint_data: RelayEndpointData::Openvpn,
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se-got-002".to_string(),
Expand All @@ -103,7 +113,7 @@ static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
provider: "provider0".to_string(),
weight: 1,
endpoint_data: RelayEndpointData::Openvpn,
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se-got-br-001".to_string(),
Expand All @@ -115,7 +125,7 @@ static RELAYS: Lazy<RelayList> = Lazy::new(|| RelayList {
provider: "provider3".to_string(),
weight: 1,
endpoint_data: RelayEndpointData::Bridge,
location: None,
location: DUMMY_LOCATION.clone(),
},
SHADOWSOCKS_RELAY.clone(),
],
Expand Down Expand Up @@ -476,7 +486,7 @@ fn test_wireguard_entry() {
daita: false,
shadowsocks_extra_addr_in: vec![],
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se10-wireguard".to_string(),
Expand All @@ -495,7 +505,7 @@ fn test_wireguard_entry() {
daita: false,
shadowsocks_extra_addr_in: vec![],
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
],
}],
Expand Down Expand Up @@ -1131,7 +1141,7 @@ fn test_include_in_country() {
shadowsocks_extra_addr_in: vec![],
daita: false,
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se10-wireguard".to_string(),
Expand All @@ -1150,7 +1160,7 @@ fn test_include_in_country() {
shadowsocks_extra_addr_in: vec![],
daita: false,
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
],
}],
Expand Down Expand Up @@ -1352,7 +1362,7 @@ fn test_daita() {
shadowsocks_extra_addr_in: vec![],
daita: false,
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
Relay {
hostname: "se10-wireguard".to_string(),
Expand All @@ -1371,7 +1381,7 @@ fn test_daita() {
shadowsocks_extra_addr_in: vec![],
daita: true,
}),
location: None,
location: DUMMY_LOCATION.clone(),
},
],
}],
Expand Down
21 changes: 9 additions & 12 deletions mullvad-types/src/relay_constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,21 +210,18 @@ impl GeographicLocationConstraint {
impl Match<Relay> for GeographicLocationConstraint {
fn matches(&self, relay: &Relay) -> bool {
match self {
GeographicLocationConstraint::Country(ref country) => relay
.location
.as_ref()
.map_or(false, |loc| loc.country_code == *country),
GeographicLocationConstraint::Country(ref country) => {
relay.location.country_code == *country
}
GeographicLocationConstraint::City(ref country, ref city) => {
relay.location.as_ref().map_or(false, |loc| {
loc.country_code == *country && loc.city_code == *city
})
let loc = &relay.location;
loc.country_code == *country && loc.city_code == *city
}
GeographicLocationConstraint::Hostname(ref country, ref city, ref hostname) => {
relay.location.as_ref().map_or(false, |loc| {
loc.country_code == *country
&& loc.city_code == *city
&& relay.hostname == *hostname
})
let loc = &relay.location;
loc.country_code == *country
&& loc.city_code == *city
&& relay.hostname == *hostname
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion mullvad-types/src/relay_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub struct Relay {
pub provider: String,
pub weight: u64,
pub endpoint_data: RelayEndpointData,
pub location: Option<Location>,
pub location: Location,
}

impl PartialEq for Relay {
Expand Down
2 changes: 1 addition & 1 deletion test/test-manager/src/tests/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ async fn connect_local_wg_relay(mullvad_client: &mut MullvadProxyClient) -> Resu
.set_quantum_resistant_tunnel(QuantumResistantState::Off)
.await?;
mullvad_client
.set_daita_settings(DaitaSettings { enabled: false })
.set_daita_settings(DaitaSettings::default())
.await?;

let peer_addr: SocketAddr = SocketAddr::new(
Expand Down
27 changes: 9 additions & 18 deletions test/test-manager/src/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use mullvad_relay_selector::{
};
use mullvad_types::{
constraints::Constraint,
location::Location,
relay_constraints::{
BridgeSettings, GeographicLocationConstraint, LocationConstraint, RelayConstraints,
RelaySettings,
Expand Down Expand Up @@ -696,7 +695,7 @@ pub async fn constrain_to_relay(
..
}
| GetRelay::OpenVpn { exit, .. } => {
let location = into_constraint(&exit)?;
let location = into_constraint(&exit);
let relay_constraints = RelayConstraints {
location,
..Default::default()
Expand Down Expand Up @@ -724,22 +723,14 @@ pub async fn constrain_to_relay(
/// # Panics
///
/// The relay must have a location set.
pub fn into_constraint(relay: &Relay) -> anyhow::Result<Constraint<LocationConstraint>> {
relay
.location
.as_ref()
.map(
|Location {
country_code,
city_code,
..
}| {
GeographicLocationConstraint::hostname(country_code, city_code, &relay.hostname)
},
)
.map(LocationConstraint::Location)
.map(Constraint::Only)
.ok_or(anyhow!("relay is missing location"))
pub fn into_constraint(relay: &Relay) -> Constraint<LocationConstraint> {
let constraint = GeographicLocationConstraint::hostname(
relay.location.country_code.clone(),
relay.location.city_code.clone(),
&relay.hostname,
);

Constraint::Only(LocationConstraint::Location(constraint))
}

/// Ping monitoring made easy!
Expand Down
7 changes: 1 addition & 6 deletions test/test-manager/src/tests/relay_ip_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,7 @@ async fn pick_a_relay(

let relay_ip = relay.ipv4_addr_in;
let hostname = relay.hostname.clone();
let city = relay
.location
.as_ref()
.ok_or(anyhow!("Got Relay with an unknown location"))?
.city_code
.clone();
let city = relay.location.city_code.clone();

log::info!("selected {hostname} ({relay_ip})");
let location = GeographicLocationConstraint::Hostname(country, city, hostname.clone()).into();
Expand Down
Loading

0 comments on commit ba39531

Please sign in to comment.