From 1081354f65e0fc589a40efa10f3cbed110ef0d3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Sat, 23 Sep 2023 22:03:09 +0200 Subject: [PATCH] Use optional modifier for optional primitives in protobuf messages --- gui/src/main/daemon-rpc.ts | 93 +++++----- .../proto/management_interface.proto | 52 +++--- .../src/types/conversions/custom_list.rs | 38 +--- .../src/types/conversions/custom_tunnel.rs | 8 +- .../src/types/conversions/location.rs | 35 ++-- .../src/types/conversions/mod.rs | 8 - .../src/types/conversions/net.rs | 19 +- .../types/conversions/relay_constraints.rs | 164 +++++++----------- .../src/types/conversions/relay_list.rs | 10 +- .../src/types/conversions/settings.rs | 16 +- .../src/types/conversions/states.rs | 16 +- .../src/types/conversions/version.rs | 8 +- 12 files changed, 175 insertions(+), 292 deletions(-) diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts index 608bf82241f1..288bbfd7fc70 100644 --- a/gui/src/main/daemon-rpc.ts +++ b/gui/src/main/daemon-rpc.ts @@ -306,9 +306,9 @@ export class DaemonRpc { if (settingsUpdate.tunnelProtocol) { const tunnelTypeUpdate = new grpcTypes.TunnelTypeUpdate(); - tunnelTypeUpdate.setTunnelType( - convertToTunnelTypeConstraint(settingsUpdate.tunnelProtocol), - ); + if (settingsUpdate.tunnelProtocol !== 'any') { + tunnelTypeUpdate.setTunnelType(convertToTunnelType(settingsUpdate.tunnelProtocol.only)); + } normalUpdate.setTunnelType(tunnelTypeUpdate); } @@ -416,11 +416,9 @@ export class DaemonRpc { if (obfuscationSettings.udp2tcpSettings) { const grpcUdp2tcpSettings = new grpcTypes.Udp2TcpObfuscationSettings(); - grpcUdp2tcpSettings.setPort( - obfuscationSettings.udp2tcpSettings.port === 'any' - ? 0 - : obfuscationSettings.udp2tcpSettings.port.only, - ); + if (obfuscationSettings.udp2tcpSettings.port !== 'any') { + grpcUdp2tcpSettings.setPort(obfuscationSettings.udp2tcpSettings.port.only); + } grpcObfuscationSettings.setUdp2tcp(grpcUdp2tcpSettings); } @@ -995,7 +993,7 @@ function convertFromBlockingError( return { type: FirewallPolicyErrorType.generic }; case grpcTypes.ErrorState.FirewallPolicyError.ErrorType.LOCKED: { const pid = error.lockPid; - const name = error.lockName; + const name = error.lockName!; return { type: FirewallPolicyErrorType.locked, pid, name }; } } @@ -1151,7 +1149,10 @@ function convertFromRelaySettings( const normal = relaySettings.getNormal()!; const locationConstraint = convertFromLocationConstraint(normal.getLocation()); const location = locationConstraint ? { only: locationConstraint } : 'any'; - const tunnelProtocol = convertFromTunnelTypeConstraint(normal.getTunnelType()!); + // `getTunnelType()` is not falsy if type is 'any' + const tunnelProtocol = convertFromTunnelTypeConstraint( + normal.hasTunnelType() ? normal.getTunnelType() : undefined, + ); const providers = normal.getProvidersList(); const ownership = convertFromOwnership(normal.getOwnership()); const openvpnConstraints = convertFromOpenVpnConstraints(normal.getOpenvpnConstraints()!); @@ -1276,11 +1277,13 @@ 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) { @@ -1356,10 +1359,9 @@ function convertFromObfuscationSettings( return { selectedObfuscation: selectedObfuscationType, - udp2tcpSettings: - obfuscationSettings?.udp2tcp && obfuscationSettings.udp2tcp.port !== 0 - ? { port: { only: obfuscationSettings.udp2tcp.port } } - : { port: 'any' }, + udp2tcpSettings: obfuscationSettings?.udp2tcp + ? { port: convertFromConstraint(obfuscationSettings.udp2tcp.port) } + : { port: 'any' }, }; } @@ -1458,14 +1460,16 @@ function convertFromWireguardConstraints( result.port = { only: port }; } - const ipVersion = constraints.getIpVersion()?.getProtocol(); - switch (ipVersion) { - case grpcTypes.IpVersion.V4: - result.ipVersion = { only: 'ipv4' }; - break; - case grpcTypes.IpVersion.V6: - result.ipVersion = { only: 'ipv6' }; - break; + // `getIpVersion()` is not falsy if type is 'any' + if (constraints.hasIpVersion()) { + switch (constraints.getIpVersion()) { + case grpcTypes.IpVersion.V4: + result.ipVersion = { only: 'ipv4' }; + break; + case grpcTypes.IpVersion.V6: + result.ipVersion = { only: 'ipv6' }; + break; + } } const entryLocation = constraints.getEntryLocation(); @@ -1478,9 +1482,9 @@ function convertFromWireguardConstraints( } function convertFromTunnelTypeConstraint( - constraint: grpcTypes.TunnelTypeConstraint | undefined, + constraint: grpcTypes.TunnelType | undefined, ): Constraint { - switch (constraint?.getTunnelType()) { + switch (constraint) { case grpcTypes.TunnelType.WIREGUARD: { return { only: 'wireguard' }; } @@ -1518,15 +1522,17 @@ 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); @@ -1541,22 +1547,13 @@ function convertToRelayLocation(location: RelayLocation): grpcTypes.RelayLocatio return relayLocation; } -function convertToTunnelTypeConstraint( - constraint: Constraint, -): grpcTypes.TunnelTypeConstraint | undefined { - const grpcConstraint = new grpcTypes.TunnelTypeConstraint(); - - if (constraint !== undefined && constraint !== 'any' && 'only' in constraint) { - switch (constraint.only) { - case 'wireguard': - grpcConstraint.setTunnelType(grpcTypes.TunnelType.WIREGUARD); - return grpcConstraint; - case 'openvpn': - grpcConstraint.setTunnelType(grpcTypes.TunnelType.OPENVPN); - return grpcConstraint; - } +function convertToTunnelType(tunnelProtocol: TunnelProtocol): grpcTypes.TunnelType { + switch (tunnelProtocol) { + case 'wireguard': + return grpcTypes.TunnelType.WIREGUARD; + case 'openvpn': + return grpcTypes.TunnelType.OPENVPN; } - return undefined; } function convertToOpenVpnConstraints( @@ -1595,9 +1592,7 @@ function convertToWireguardConstraints( if (ipVersion) { const ipVersionProtocol = ipVersion === 'ipv4' ? grpcTypes.IpVersion.V4 : grpcTypes.IpVersion.V6; - const ipVersionConstraints = new grpcTypes.IpVersionConstraint(); - ipVersionConstraints.setProtocol(ipVersionProtocol); - wireguardConstraints.setIpVersion(ipVersionConstraints); + wireguardConstraints.setIpVersion(ipVersionProtocol); } if (constraint.useMultihop) { @@ -1687,7 +1682,7 @@ function convertFromCustomLists(customLists: Array): Custo locations: list .getLocationsList() .map((location) => - convertFromRelayLocation(location.toObject()), + convertFromGeographicConstraint(location.toObject()), ) as Array, })); } @@ -1697,7 +1692,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 24bfe2284b7f..4846f896e2a1 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -161,7 +161,7 @@ message ErrorState { // LOCKED uint32 lock_pid = 2; - string lock_name = 3; + optional string lock_name = 3; } Cause cause = 1; @@ -240,17 +240,17 @@ message ProxyEndpoint { } message GeoIpLocation { - string ipv4 = 1; - string ipv6 = 2; + optional string ipv4 = 1; + optional string ipv6 = 2; string country = 3; - string city = 4; + optional string city = 4; double latitude = 5; double longitude = 6; bool mullvad_exit_ip = 7; - string hostname = 8; - string bridge_hostname = 9; - string entry_hostname = 10; - string obfuscator_hostname = 11; + optional string hostname = 8; + optional string bridge_hostname = 9; + optional string entry_hostname = 10; + optional string obfuscator_hostname = 11; } message TunnelMetadata { string tunnel_interface = 1; } @@ -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 { @@ -316,7 +316,7 @@ message BridgeState { State state = 1; } -message Udp2TcpObfuscationSettings { uint32 port = 1; } +message Udp2TcpObfuscationSettings { optional uint32 port = 1; } message ObfuscationSettings { enum SelectedObfuscation { @@ -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; } @@ -410,12 +410,10 @@ message RelaySettings { } } -message TunnelTypeConstraint { TunnelType tunnel_type = 1; } - message NormalRelaySettings { LocationConstraint location = 1; repeated string providers = 2; - TunnelTypeConstraint tunnel_type = 3; + optional TunnelType tunnel_type = 3; WireguardConstraints wireguard_constraints = 4; OpenvpnConstraints openvpn_constraints = 5; Ownership ownership = 6; @@ -433,11 +431,11 @@ message NormalRelaySettingsUpdate { message ProviderUpdate { repeated string providers = 1; } -message TunnelTypeUpdate { TunnelTypeConstraint tunnel_type = 2; } +message TunnelTypeUpdate { optional TunnelType tunnel_type = 2; } message TransportPort { TransportProtocol protocol = 1; - uint32 port = 2; + optional uint32 port = 2; } message OpenvpnConstraints { TransportPort port = 1; } @@ -449,11 +447,9 @@ enum IpVersion { V6 = 1; } -message IpVersionConstraint { IpVersion protocol = 1; } - message WireguardConstraints { - uint32 port = 1; - IpVersionConstraint ip_version = 2; + optional uint32 port = 1; + optional IpVersion ip_version = 2; bool use_multihop = 3; LocationConstraint entry_location = 4; } @@ -484,7 +480,7 @@ message ConnectionConfig { TunnelConfig tunnel = 1; PeerConfig peer = 2; string ipv4_gateway = 3; - string ipv6_gateway = 4; + optional string ipv6_gateway = 4; } oneof config { @@ -503,9 +499,9 @@ message QuantumResistantState { } message TunnelOptions { - message OpenvpnOptions { uint32 mssfix = 1; } + message OpenvpnOptions { optional uint32 mssfix = 1; } message WireguardOptions { - uint32 mtu = 1; + optional uint32 mtu = 1; google.protobuf.Duration rotation_interval = 2; QuantumResistantState quantum_resistant = 4; } @@ -555,7 +551,7 @@ message AppVersionInfo { bool supported = 1; string latest_stable = 2; string latest_beta = 3; - string suggested_upgrade = 4; + optional string suggested_upgrade = 4; } message RelayListCountry { @@ -581,7 +577,7 @@ message Relay { string hostname = 1; string ipv4_addr_in = 2; - string ipv6_addr_in = 3; + optional string ipv6_addr_in = 3; bool include_in_country = 4; bool active = 5; bool owned = 6; 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/custom_tunnel.rs b/mullvad-management-interface/src/types/conversions/custom_tunnel.rs index 146799b47382..8a4408ec83d3 100644 --- a/mullvad-management-interface/src/types/conversions/custom_tunnel.rs +++ b/mullvad-management-interface/src/types/conversions/custom_tunnel.rs @@ -1,5 +1,5 @@ use crate::types::{ - conversions::{bytes_to_privkey, bytes_to_pubkey, option_from_proto_string}, + conversions::{bytes_to_privkey, bytes_to_pubkey}, proto, FromProtobufTypeError, }; use talpid_types::net::wireguard; @@ -51,7 +51,8 @@ impl TryFrom for mullvad_types::ConnectionConfig { let ipv4_gateway = config.ipv4_gateway.parse().map_err(|_err| { FromProtobufTypeError::InvalidArgument("invalid IPv4 gateway") })?; - let ipv6_gateway = option_from_proto_string(config.ipv6_gateway) + let ipv6_gateway = config + .ipv6_gateway .map(|addr| { addr.parse().map_err(|_err| { FromProtobufTypeError::InvalidArgument("invalid IPv6 gateway") @@ -144,8 +145,7 @@ impl From for proto::ConnectionConfig { ipv6_gateway: config .ipv6_gateway .as_ref() - .map(|address| address.to_string()) - .unwrap_or_default(), + .map(|address| address.to_string()), }) } }), diff --git a/mullvad-management-interface/src/types/conversions/location.rs b/mullvad-management-interface/src/types/conversions/location.rs index 65fa8256386f..b8225f2ac0f0 100644 --- a/mullvad-management-interface/src/types/conversions/location.rs +++ b/mullvad-management-interface/src/types/conversions/location.rs @@ -1,22 +1,19 @@ -use crate::types::{ - conversions::{arg_from_str, option_from_proto_string}, - proto, FromProtobufTypeError, -}; +use crate::types::{conversions::arg_from_str, proto, FromProtobufTypeError}; impl From for proto::GeoIpLocation { fn from(geoip: mullvad_types::location::GeoIpLocation) -> proto::GeoIpLocation { proto::GeoIpLocation { - ipv4: geoip.ipv4.map(|ip| ip.to_string()).unwrap_or_default(), - ipv6: geoip.ipv6.map(|ip| ip.to_string()).unwrap_or_default(), + ipv4: geoip.ipv4.map(|ip| ip.to_string()), + ipv6: geoip.ipv6.map(|ip| ip.to_string()), country: geoip.country, - city: geoip.city.unwrap_or_default(), + city: geoip.city, latitude: geoip.latitude, longitude: geoip.longitude, mullvad_exit_ip: geoip.mullvad_exit_ip, - hostname: geoip.hostname.unwrap_or_default(), - bridge_hostname: geoip.bridge_hostname.unwrap_or_default(), - entry_hostname: geoip.entry_hostname.unwrap_or_default(), - obfuscator_hostname: geoip.obfuscator_hostname.unwrap_or_default(), + hostname: geoip.hostname, + bridge_hostname: geoip.bridge_hostname, + entry_hostname: geoip.entry_hostname, + obfuscator_hostname: geoip.obfuscator_hostname, } } } @@ -26,21 +23,23 @@ impl TryFrom for mullvad_types::location::GeoIpLocation { fn try_from(geoip: proto::GeoIpLocation) -> Result { Ok(mullvad_types::location::GeoIpLocation { - ipv4: option_from_proto_string(geoip.ipv4) + ipv4: geoip + .ipv4 .map(|addr| arg_from_str(&addr, "invalid IPv4 address")) .transpose()?, - ipv6: option_from_proto_string(geoip.ipv6) + ipv6: geoip + .ipv6 .map(|addr| arg_from_str(&addr, "invalid IPv6 address")) .transpose()?, country: geoip.country, - city: option_from_proto_string(geoip.city), + city: geoip.city, latitude: geoip.latitude, longitude: geoip.longitude, mullvad_exit_ip: geoip.mullvad_exit_ip, - hostname: option_from_proto_string(geoip.hostname), - bridge_hostname: option_from_proto_string(geoip.bridge_hostname), - entry_hostname: option_from_proto_string(geoip.entry_hostname), - obfuscator_hostname: option_from_proto_string(geoip.obfuscator_hostname), + hostname: geoip.hostname, + bridge_hostname: geoip.bridge_hostname, + entry_hostname: geoip.entry_hostname, + obfuscator_hostname: geoip.obfuscator_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/net.rs b/mullvad-management-interface/src/types/conversions/net.rs index ea5dcf99a53d..7b24a8f2b4ca 100644 --- a/mullvad-management-interface/src/types/conversions/net.rs +++ b/mullvad-management-interface/src/types/conversions/net.rs @@ -1,5 +1,4 @@ use crate::types::{conversions::arg_from_str, proto, FromProtobufTypeError}; -use mullvad_types::relay_constraints::Constraint; use std::net::SocketAddr; impl From for proto::TunnelEndpoint { @@ -155,21 +154,11 @@ impl From for talpid_types::net::TransportProtocol { } } -impl TryFrom for Constraint { - type Error = FromProtobufTypeError; - - fn try_from( - tunnel_type: proto::TunnelTypeConstraint, - ) -> Result, Self::Error> { - let tunnel_type = try_tunnel_type_from_i32(tunnel_type.tunnel_type)?; - Ok(Constraint::Only(tunnel_type)) - } -} - -impl From for proto::IpVersionConstraint { +impl From for talpid_types::net::IpVersion { fn from(version: proto::IpVersion) -> Self { - Self { - protocol: i32::from(version), + match version { + proto::IpVersion::V4 => talpid_types::net::IpVersion::V4, + proto::IpVersion::V6 => talpid_types::net::IpVersion::V6, } } } diff --git a/mullvad-management-interface/src/types/conversions/relay_constraints.rs b/mullvad-management-interface/src/types/conversions/relay_constraints.rs index b7b5f0060bdf..4fb96d4e0e23 100644 --- a/mullvad-management-interface/src/types/conversions/relay_constraints.rs +++ b/mullvad-management-interface/src/types/conversions/relay_constraints.rs @@ -1,7 +1,7 @@ -use crate::types::{conversions::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; @@ -17,25 +17,17 @@ impl TryFrom<&proto::WireguardConstraints> use mullvad_types::relay_constraints as mullvad_constraints; use talpid_types::net; - let ip_version = match &constraints.ip_version { - Some(constraint) => match proto::IpVersion::try_from(constraint.protocol) { - Ok(proto::IpVersion::V4) => Some(net::IpVersion::V4), - Ok(proto::IpVersion::V6) => Some(net::IpVersion::V6), - Err(_) => { - return Err(FromProtobufTypeError::InvalidArgument( - "invalid ip protocol version", - )) - } - }, + let ip_version = match constraints.ip_version { + Some(version) => Some(net::IpVersion::from( + proto::IpVersion::try_from(version).map_err(|_| { + FromProtobufTypeError::InvalidArgument("invalid IP protocol version") + })?, + )), None => None, }; Ok(mullvad_constraints::WireguardConstraints { - port: if constraints.port == 0 { - Constraint::Any - } else { - Constraint::Only(constraints.port as u16) - }, + port: Constraint::from(constraints.port.map(|port| port as u16)), ip_version: Constraint::from(ip_version), use_multihop: constraints.use_multihop, entry_location: constraints @@ -76,7 +68,6 @@ impl TryFrom for mullvad_types::relay_constraints::RelaySe settings: proto::RelaySettings, ) -> Result { use mullvad_types::{relay_constraints as mullvad_constraints, CustomTunnelEndpoint}; - use talpid_types::net; let update_value = settings .endpoint @@ -107,11 +98,12 @@ impl TryFrom for mullvad_types::relay_constraints::RelaySe .unwrap_or(Constraint::Any); let providers = try_providers_constraint_from_proto(&settings.providers)?; let ownership = try_ownership_constraint_from_i32(settings.ownership)?; - let tunnel_protocol = settings - .tunnel_type - .map(Constraint::::try_from) - .transpose()? - .unwrap_or(Constraint::Any); + let tunnel_protocol = Constraint::from( + settings + .tunnel_type + .map(try_tunnel_type_from_i32) + .transpose()?, + ); let openvpn_constraints = mullvad_constraints::OpenVpnConstraints::try_from( &settings.openvpn_constraints.ok_or( @@ -165,29 +157,23 @@ impl From for proto::RelaySettingsUpdate { }), tunnel_type: constraints.tunnel_protocol.map(|protocol| { proto::TunnelTypeUpdate { - tunnel_type: match protocol { - Constraint::Any => None, - Constraint::Only(protocol) => { - Some(proto::TunnelTypeConstraint { - tunnel_type: i32::from(match protocol { - TunnelType::Wireguard => { - proto::TunnelType::Wireguard - } - TunnelType::OpenVpn => proto::TunnelType::Openvpn, - }), + tunnel_type: protocol + .map(|protocol| { + i32::from(match protocol { + TunnelType::Wireguard => proto::TunnelType::Wireguard, + TunnelType::OpenVpn => proto::TunnelType::Openvpn, }) - } - }, + }) + .option(), } }), wireguard_constraints: constraints.wireguard_constraints.map( |wireguard_constraints| proto::WireguardConstraints { - port: u32::from(wireguard_constraints.port.unwrap_or(0)), + port: wireguard_constraints.port.map(u32::from).option(), ip_version: wireguard_constraints .ip_version .option() - .map(proto::IpVersion::from) - .map(proto::IpVersionConstraint::from), + .map(|ipv| i32::from(proto::IpVersion::from(ipv))), use_multihop: wireguard_constraints.use_multihop, entry_location: wireguard_constraints .entry_location @@ -225,7 +211,6 @@ impl TryFrom for mullvad_types::relay_constraints::R settings: proto::RelaySettingsUpdate, ) -> Result { use mullvad_types::{relay_constraints as mullvad_constraints, CustomTunnelEndpoint}; - use talpid_types::net; let update_value = settings .r#type @@ -276,13 +261,12 @@ impl TryFrom for mullvad_types::relay_constraints::R None }; let tunnel_protocol = if let Some(update) = settings.tunnel_type { - Some( + Some(Constraint::from( update .tunnel_type - .map(Constraint::::try_from) - .transpose()? - .unwrap_or(Constraint::Any), - ) + .map(try_tunnel_type_from_i32) + .transpose()?, + )) } else { None }; @@ -352,7 +336,7 @@ impl From<&mullvad_types::relay_constraints::Udp2TcpObfuscationSettings> { fn from(settings: &mullvad_types::relay_constraints::Udp2TcpObfuscationSettings) -> Self { Self { - port: u32::from(settings.port.unwrap_or(0)), + port: settings.port.map(u32::from).option(), } } } @@ -439,18 +423,19 @@ impl From for proto::RelaySetti Some(proto::TunnelType::Openvpn) } } - .map(|tunnel_type| proto::TunnelTypeConstraint { - tunnel_type: i32::from(tunnel_type), - }), + .map(i32::from), wireguard_constraints: Some(proto::WireguardConstraints { - port: u32::from(constraints.wireguard_constraints.port.unwrap_or(0)), + port: constraints + .wireguard_constraints + .port + .map(u32::from) + .option(), ip_version: constraints .wireguard_constraints .ip_version .option() - .map(proto::IpVersion::from) - .map(proto::IpVersionConstraint::from), + .map(|ipv| i32::from(proto::IpVersion::from(ipv))), use_multihop: constraints.wireguard_constraints.use_multihop, entry_location: constraints .wireguard_constraints @@ -480,7 +465,7 @@ impl From for proto::TransportP fn from(port: mullvad_types::relay_constraints::TransportPort) -> Self { proto::TransportPort { protocol: proto::TransportProtocol::from(port.protocol) as i32, - port: port.port.map(u32::from).unwrap_or(0), + port: port.port.map(u32::from).option(), } } } @@ -491,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 { @@ -511,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(|_| { @@ -535,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, @@ -546,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", + )), } } } @@ -699,11 +673,7 @@ impl TryFrom<&proto::Udp2TcpObfuscationSettings> fn try_from(settings: &proto::Udp2TcpObfuscationSettings) -> Result { Ok(Self { - port: if settings.port == 0 { - Constraint::Any - } else { - Constraint::Only(settings.port as u16) - }, + port: Constraint::from(settings.port.map(|port| port as u16)), }) } } @@ -735,11 +705,7 @@ impl TryFrom for mullvad_types::relay_constraints::Transpo fn try_from(port: proto::TransportPort) -> Result { Ok(mullvad_types::relay_constraints::TransportPort { protocol: super::net::try_transport_protocol_from_i32(port.protocol)?, - port: if port.port == 0 { - Constraint::Any - } else { - Constraint::Only(port.port as u16) - }, + port: Constraint::from(port.port.map(|port| port as u16)), }) } } diff --git a/mullvad-management-interface/src/types/conversions/relay_list.rs b/mullvad-management-interface/src/types/conversions/relay_list.rs index 32541239298f..32aee834afe1 100644 --- a/mullvad-management-interface/src/types/conversions/relay_list.rs +++ b/mullvad-management-interface/src/types/conversions/relay_list.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::types::{ - conversions::{bytes_to_pubkey, option_from_proto_string, to_proto_any, try_from_proto_any}, + conversions::{bytes_to_pubkey, to_proto_any, try_from_proto_any}, proto, FromProtobufTypeError, }; @@ -106,10 +106,7 @@ impl From for proto::Relay { Self { hostname: relay.hostname, ipv4_addr_in: relay.ipv4_addr_in.to_string(), - ipv6_addr_in: relay - .ipv6_addr_in - .map(|addr| addr.to_string()) - .unwrap_or_default(), + ipv6_addr_in: relay.ipv6_addr_in.map(|addr| addr.to_string()), include_in_country: relay.include_in_country, active: relay.active, owned: relay.owned, @@ -249,7 +246,8 @@ impl TryFrom for mullvad_types::relay_list::Relay { } }; - let ipv6_addr_in = option_from_proto_string(relay.ipv6_addr_in) + let ipv6_addr_in = relay + .ipv6_addr_in .map(|addr| { addr.parse().map_err(|_err| { FromProtobufTypeError::InvalidArgument("invalid relay IPv6 address") diff --git a/mullvad-management-interface/src/types/conversions/settings.rs b/mullvad-management-interface/src/types/conversions/settings.rs index 98c195d93562..c18e7aab57c0 100644 --- a/mullvad-management-interface/src/types/conversions/settings.rs +++ b/mullvad-management-interface/src/types/conversions/settings.rs @@ -82,10 +82,10 @@ impl From<&mullvad_types::settings::TunnelOptions> for proto::TunnelOptions { fn from(options: &mullvad_types::settings::TunnelOptions) -> Self { Self { openvpn: Some(proto::tunnel_options::OpenvpnOptions { - mssfix: u32::from(options.openvpn.mssfix.unwrap_or_default()), + mssfix: options.openvpn.mssfix.map(u32::from), }), wireguard: Some(proto::tunnel_options::WireguardOptions { - mtu: u32::from(options.wireguard.mtu.unwrap_or_default()), + mtu: options.wireguard.mtu.map(u32::from), rotation_interval: options.wireguard.rotation_interval.map(|ivl| { prost_types::Duration::try_from(std::time::Duration::from(ivl)) .expect("Failed to convert std::time::Duration to prost_types::Duration for tunnel_options.wireguard.rotation_interval") @@ -247,18 +247,10 @@ impl TryFrom for mullvad_types::settings::TunnelOptions { Ok(Self { openvpn: net::openvpn::TunnelOptions { - mssfix: if openvpn_options.mssfix != 0 { - Some(openvpn_options.mssfix as u16) - } else { - None - }, + mssfix: openvpn_options.mssfix.map(|mssfix| mssfix as u16), }, wireguard: mullvad_types::wireguard::TunnelOptions { - mtu: if wireguard_options.mtu != 0 { - Some(wireguard_options.mtu as u16) - } else { - None - }, + mtu: wireguard_options.mtu.map(|mtu| mtu as u16), rotation_interval: wireguard_options .rotation_interval .map(std::time::Duration::try_from) diff --git a/mullvad-management-interface/src/types/conversions/states.rs b/mullvad-management-interface/src/types/conversions/states.rs index ff8de455ab0a..b742aa7b004c 100644 --- a/mullvad-management-interface/src/types/conversions/states.rs +++ b/mullvad-management-interface/src/types/conversions/states.rs @@ -1,5 +1,3 @@ -#[cfg(windows)] -use crate::types::conversions::option_from_proto_string; use crate::types::{proto, FromProtobufTypeError}; impl From for proto::TunnelState { @@ -21,8 +19,8 @@ impl From for proto::TunnelState { #[cfg(windows)] talpid_tunnel::FirewallPolicyError::Locked(blocking_app) => { let (lock_pid, lock_name) = match blocking_app { - Some(app) => (app.pid, app.name.clone()), - None => (0, "".to_string()), + Some(app) => (app.pid, Some(app.name.clone())), + None => (0, None), }; FirewallPolicyError { @@ -330,7 +328,7 @@ impl TryFrom for mullvad_types::states::TunnelState { fn try_firewall_policy_error_from_i32( policy_error: i32, lock_pid: u32, - lock_name: String, + lock_name: Option, ) -> Result { match proto::error_state::firewall_policy_error::ErrorType::try_from(policy_error) { Ok(proto::error_state::firewall_policy_error::ErrorType::Generic) => { @@ -338,11 +336,9 @@ fn try_firewall_policy_error_from_i32( } #[cfg(windows)] Ok(proto::error_state::firewall_policy_error::ErrorType::Locked) => { - let blocking_app = option_from_proto_string(lock_name).map(|name| { - talpid_types::tunnel::BlockingApplication { - pid: lock_pid, - name, - } + let blocking_app = lock_name.map(|name| talpid_types::tunnel::BlockingApplication { + pid: lock_pid, + name, }); Ok(talpid_types::tunnel::FirewallPolicyError::Locked( blocking_app, diff --git a/mullvad-management-interface/src/types/conversions/version.rs b/mullvad-management-interface/src/types/conversions/version.rs index 5ded9e9f0f37..b2ab05420641 100644 --- a/mullvad-management-interface/src/types/conversions/version.rs +++ b/mullvad-management-interface/src/types/conversions/version.rs @@ -6,7 +6,7 @@ impl From for proto::AppVersionInfo { supported: version_info.supported, latest_stable: version_info.latest_stable, latest_beta: version_info.latest_beta, - suggested_upgrade: version_info.suggested_upgrade.unwrap_or_default(), + suggested_upgrade: version_info.suggested_upgrade, } } } @@ -17,11 +17,7 @@ impl From for mullvad_types::version::AppVersionInfo { supported: version_info.supported, latest_stable: version_info.latest_stable, latest_beta: version_info.latest_beta, - suggested_upgrade: if version_info.suggested_upgrade.is_empty() { - None - } else { - Some(version_info.suggested_upgrade) - }, + suggested_upgrade: version_info.suggested_upgrade, } } }