diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts index 12542d6d552e..21d489b63f8d 100644 --- a/gui/src/main/daemon-rpc.ts +++ b/gui/src/main/daemon-rpc.ts @@ -1277,11 +1277,11 @@ function convertFromLocationConstraint( return { customList: location.getCustomList() }; } else { const innerLocation = location.getLocation()?.toObject(); - return innerLocation && convertFromRelayLocation(innerLocation); + return innerLocation && convertFromGeographicConstraint(innerLocation); } } -function convertFromRelayLocation(location: grpcTypes.RelayLocation.AsObject): RelayLocation { +function convertFromGeographicConstraint(location: grpcTypes.GeographicLocationConstraint.AsObject): RelayLocation { if (location.hostname) { return location; } else if (location.city) { @@ -1520,15 +1520,15 @@ function convertToLocation( if (constraint && 'customList' in constraint && constraint.customList) { locationConstraint.setCustomList(constraint.customList); } else { - const location = constraint && convertToRelayLocation(constraint); + const location = constraint && convertToGeographicConstraint(constraint); locationConstraint.setLocation(location); } return locationConstraint; } -function convertToRelayLocation(location: RelayLocation): grpcTypes.RelayLocation { - const relayLocation = new grpcTypes.RelayLocation(); +function convertToGeographicConstraint(location: RelayLocation): grpcTypes.GeographicLocationConstraint { + const relayLocation = new grpcTypes.GeographicLocationConstraint(); if ('hostname' in location) { relayLocation.setCountry(location.country); relayLocation.setCity(location.city); @@ -1678,7 +1678,7 @@ function convertFromCustomLists(customLists: Array): Custo locations: list .getLocationsList() .map((location) => - convertFromRelayLocation(location.toObject()), + convertFromGeographicConstraint(location.toObject()), ) as Array, })); } @@ -1688,7 +1688,7 @@ function convertToCustomList(customList: ICustomList): grpcTypes.CustomList { grpcCustomList.setId(customList.id); grpcCustomList.setName(customList.name); - const locations = customList.locations.map(convertToRelayLocation); + const locations = customList.locations.map(convertToGeographicConstraint); grpcCustomList.setLocationsList(locations); return grpcCustomList; diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index 9617b389f404..4846f896e2a1 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -297,14 +297,14 @@ message BridgeSettings { message LocationConstraint { oneof type { string custom_list = 1; - RelayLocation location = 2; + GeographicLocationConstraint location = 2; } } -message RelayLocation { +message GeographicLocationConstraint { string country = 1; - string city = 2; - string hostname = 3; + optional string city = 2; + optional string hostname = 3; } message BridgeState { @@ -331,7 +331,7 @@ message ObfuscationSettings { message CustomList { string id = 1; string name = 2; - repeated RelayLocation locations = 3; + repeated GeographicLocationConstraint locations = 3; } message CustomListSettings { repeated CustomList custom_lists = 1; } diff --git a/mullvad-management-interface/src/types/conversions/custom_list.rs b/mullvad-management-interface/src/types/conversions/custom_list.rs index 62799168f7c1..9d4e9cee7846 100644 --- a/mullvad-management-interface/src/types/conversions/custom_list.rs +++ b/mullvad-management-interface/src/types/conversions/custom_list.rs @@ -33,7 +33,7 @@ impl From for proto::CustomList { let locations = custom_list .locations .into_iter() - .map(proto::RelayLocation::from) + .map(proto::GeographicLocationConstraint::from) .collect(); Self { id: custom_list.id.to_string(), @@ -60,39 +60,3 @@ impl TryFrom for mullvad_types::custom_list::CustomList { }) } } - -impl TryFrom for GeographicLocationConstraint { - type Error = FromProtobufTypeError; - - fn try_from(relay_location: proto::RelayLocation) -> Result { - match ( - relay_location.country.as_ref(), - relay_location.city.as_ref(), - relay_location.hostname.as_ref(), - ) { - ("", ..) => Err(FromProtobufTypeError::InvalidArgument( - "Invalid geographic relay location", - )), - (_country, "", "") => Ok(GeographicLocationConstraint::Country( - relay_location.country, - )), - (_country, _city, "") => Ok(GeographicLocationConstraint::City( - relay_location.country, - relay_location.city, - )), - (_country, city, _hostname) => { - if city.is_empty() { - Err(FromProtobufTypeError::InvalidArgument( - "Relay location must contain a city if hostname is included", - )) - } else { - Ok(GeographicLocationConstraint::Hostname( - relay_location.country, - relay_location.city, - relay_location.hostname, - )) - } - } - } - } -} diff --git a/mullvad-management-interface/src/types/conversions/mod.rs b/mullvad-management-interface/src/types/conversions/mod.rs index dd6fcd450167..4c2151270f3d 100644 --- a/mullvad-management-interface/src/types/conversions/mod.rs +++ b/mullvad-management-interface/src/types/conversions/mod.rs @@ -45,14 +45,6 @@ fn bytes_to_wg_key<'a>( <&[u8; 32]>::try_from(bytes).map_err(|_| FromProtobufTypeError::InvalidArgument(error_msg)) } -/// Returns `Option`, where an empty string represents `None`. -fn option_from_proto_string(s: String) -> Option { - match s { - s if s.is_empty() => None, - s => Some(s), - } -} - fn arg_from_str, E>( s: &str, invalid_arg_msg: &'static str, diff --git a/mullvad-management-interface/src/types/conversions/relay_constraints.rs b/mullvad-management-interface/src/types/conversions/relay_constraints.rs index b8ad37351f15..4fb96d4e0e23 100644 --- a/mullvad-management-interface/src/types/conversions/relay_constraints.rs +++ b/mullvad-management-interface/src/types/conversions/relay_constraints.rs @@ -1,10 +1,7 @@ -use crate::types::{ - conversions::{net::try_tunnel_type_from_i32, option_from_proto_string}, - proto, FromProtobufTypeError, -}; +use crate::types::{conversions::net::try_tunnel_type_from_i32, proto, FromProtobufTypeError}; use mullvad_types::{ custom_list::Id, - relay_constraints::{Constraint, RelaySettingsUpdate}, + relay_constraints::{Constraint, GeographicLocationConstraint, RelaySettingsUpdate}, }; use std::str::FromStr; use talpid_types::net::TunnelType; @@ -479,7 +476,7 @@ impl From for proto::Locat match location { LocationConstraint::Location(location) => Self { r#type: Some(proto::location_constraint::Type::Location( - proto::RelayLocation::from(location), + proto::GeographicLocationConstraint::from(location), )), }, LocationConstraint::CustomList { list_id } => Self { @@ -499,17 +496,9 @@ impl TryFrom fn try_from(location: proto::LocationConstraint) -> Result { use mullvad_types::relay_constraints::LocationConstraint; match location.r#type { - Some(proto::location_constraint::Type::Location(location)) => { - let location = Constraint::< - mullvad_types::relay_constraints::GeographicLocationConstraint, - >::from(location); - match location { - Constraint::Any => Ok(Constraint::Any), - Constraint::Only(location) => { - Ok(Constraint::Only(LocationConstraint::Location(location))) - } - } - } + Some(proto::location_constraint::Type::Location(location)) => Ok(Constraint::Only( + LocationConstraint::Location(GeographicLocationConstraint::try_from(location)?), + )), Some(proto::location_constraint::Type::CustomList(list_id)) => { let location = LocationConstraint::CustomList { list_id: Id::from_str(&list_id).map_err(|_| { @@ -523,10 +512,8 @@ impl TryFrom } } -impl From for proto::RelayLocation { +impl From for proto::GeographicLocationConstraint { fn from(location: mullvad_types::relay_constraints::GeographicLocationConstraint) -> Self { - use mullvad_types::relay_constraints::GeographicLocationConstraint; - match location { GeographicLocationConstraint::Country(country) => Self { country, @@ -534,36 +521,35 @@ impl From for pr }, GeographicLocationConstraint::City(country, city) => Self { country, - city, - ..Default::default() + city: Some(city), + hostname: None, }, GeographicLocationConstraint::Hostname(country, city, hostname) => Self { country, - city, - hostname, + city: Some(city), + hostname: Some(hostname), }, } } } -impl From - for Constraint -{ - fn from(location: proto::RelayLocation) -> Self { - use mullvad_types::relay_constraints::GeographicLocationConstraint; - - if let Some(hostname) = option_from_proto_string(location.hostname) { - Constraint::Only(GeographicLocationConstraint::Hostname( - location.country, - location.city, - hostname, - )) - } else if let Some(city) = option_from_proto_string(location.city) { - Constraint::Only(GeographicLocationConstraint::City(location.country, city)) - } else if let Some(country) = option_from_proto_string(location.country) { - Constraint::Only(GeographicLocationConstraint::Country(country)) - } else { - Constraint::Any +impl TryFrom for GeographicLocationConstraint { + type Error = FromProtobufTypeError; + + fn try_from(relay_location: proto::GeographicLocationConstraint) -> Result { + match ( + relay_location.country, + relay_location.city, + relay_location.hostname, + ) { + (country, None, None) => Ok(GeographicLocationConstraint::Country(country)), + (country, Some(city), None) => Ok(GeographicLocationConstraint::City(country, city)), + (country, Some(city), Some(hostname)) => Ok(GeographicLocationConstraint::Hostname( + country, city, hostname, + )), + (_country, None, Some(_hostname)) => Err(FromProtobufTypeError::InvalidArgument( + "Relay location contains hostname but no city", + )), } } }