Merge branch 'investigate-why-testcreateaccount-started-failing-on-main-ios-952'

Author: pinkisemils

Cargo.lock

@@ -5,6 +5,9 @@ import java.io.File
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
+import net.mullvad.mullvadvpn.lib.endpoint.ApiEndpointFromIntentHolder
+import net.mullvad.mullvadvpn.lib.endpoint.ApiEndpointOverride
+import net.mullvad.mullvadvpn.service.BuildConfig
 
 const val PROBLEM_REPORT_LOGS_FILE = "problem_report.txt"
 
@@ -21,7 +24,12 @@ sealed interface SendProblemReportResult {
 
 data class UserReport(val email: String?, val description: String)
 
-class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher = Dispatchers.IO) {
+class MullvadProblemReport(
+    context: Context,
+    private val apiEndpointOverride: ApiEndpointOverride?,
+    private val apiEndpointFromIntentHolder: ApiEndpointFromIntentHolder,
+    val dispatcher: CoroutineDispatcher = Dispatchers.IO,
+) {
 
     private val cacheDirectory = File(context.cacheDir.toURI())
     private val logDirectory = File(context.filesDir.toURI())
@@ -47,11 +55,20 @@ class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher
 
         val sentSuccessfully =
             withContext(dispatcher) {
+                val intentApiOverride = apiEndpointFromIntentHolder.apiEndpointOverride
+                val apiOverride =
+                    if (BuildConfig.DEBUG && intentApiOverride != null) {
+                        intentApiOverride
+                    } else {
+                        apiEndpointOverride
+                    }
+
                 sendProblemReport(
                     userReport.email ?: "",
                     userReport.description,
                     logsPath.absolutePath,
                     cacheDirectory.absolutePath,
+                    apiOverride,
                 )
             }
 
@@ -89,5 +106,6 @@ class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher
         userMessage: String,
         reportPath: String,
         cacheDirectory: String,
+        apiEndpointOverride: ApiEndpointOverride?,
     ): Boolean
 }

android/app/src/main/kotlin/net/mullvad/mullvadvpn/dataproxy/MullvadProblemReport.kt

@@ -33,6 +33,7 @@ import net.mullvad.mullvadvpn.repository.UserPreferencesMigration
 import net.mullvad.mullvadvpn.repository.UserPreferencesRepository
 import net.mullvad.mullvadvpn.repository.UserPreferencesSerializer
 import net.mullvad.mullvadvpn.repository.WireguardConstraintsRepository
+import net.mullvad.mullvadvpn.service.DaemonConfig
 import net.mullvad.mullvadvpn.ui.MainActivity
 import net.mullvad.mullvadvpn.ui.serviceconnection.AppVersionInfoRepository
 import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager
@@ -129,7 +130,7 @@ val uiModule = module {
     single { ChangelogRepository(get()) }
     single { UserPreferencesRepository(get()) }
     single { SettingsRepository(get()) }
-    single { MullvadProblemReport(get()) }
+    single { MullvadProblemReport(get(), get<DaemonConfig>().apiEndpointOverride, get()) }
     single { RelayOverridesRepository(get()) }
     single { CustomListsRepository(get()) }
     single { RelayListRepository(get(), get()) }

android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt

@@ -7,7 +7,6 @@ import kotlinx.parcelize.Parcelize
 data class ApiEndpointOverride(
     val hostname: String,
     val port: Int = CUSTOM_ENDPOINT_HTTPS_PORT,
-    val disableAddressCache: Boolean = true,
     val disableTls: Boolean = false,
     val forceDirectConnection: Boolean = true,
 ) : Parcelable {

android/lib/endpoint/src/main/kotlin/net/mullvad/mullvadvpn/lib/endpoint/ApiEndpointOverride.kt

@@ -55,11 +55,6 @@ abstract class MockApiTest {
     }
 
     private fun createEndpoint(port: Int): ApiEndpointOverride {
-        return ApiEndpointOverride(
-            InetAddress.getLocalHost().hostName,
-            port,
-            disableAddressCache = true,
-            disableTls = true,
-        )
+        return ApiEndpointOverride(InetAddress.getLocalHost().hostName, port, disableTls = true)
     }
 }

android/test/mockapi/src/main/kotlin/net/mullvad/mullvadvpn/test/mockapi/MockApiTest.kt

@@ -56,7 +56,8 @@ class MullvadApi {
         let result = mullvad_api_client_initialize(
             &clientContext,
             apiAddress,
-            hostname
+            hostname,
+            false
         )
         try ApiError(result).throwIfErr()
     }

ios/MullvadVPNUITests/MullvadApi.swift

@@ -33,6 +33,7 @@ tokio = { workspace = true, features = ["macros", "time", "rt-multi-thread", "ne
 tokio-rustls = { version = "0.26.0", features = ["logging", "tls12", "ring"], default-features = false}
 tokio-socks = "0.5.1"
 rustls-pemfile = "2.1.3"
+uuid = { version = "1.4.1", features = ["v4"] }
 
 mullvad-encrypted-dns-proxy = { path = "../mullvad-encrypted-dns-proxy" }
 mullvad-fs = { path = "../mullvad-fs" }
@@ -45,13 +46,11 @@ shadowsocks = { workspace = true,  features = [ "stream-cipher" ] }
 [dev-dependencies]
 talpid-time = { path = "../talpid-time", features = ["test"] }
 tokio = { workspace = true, features = ["test-util", "time"] }
+mockito = "1.6.1"
 
 [build-dependencies]
 cbindgen = { version = "0.24.3", default-features = false }
 
-[target.'cfg(target_os = "ios")'.dependencies]
-uuid = { version = "1.4.1", features = ["v4"] }
-
 [lib]
 crate-type = [ "rlib", "staticlib" ]
 bench = false

mullvad-api/Cargo.toml

@@ -49,15 +49,16 @@ typedef struct MullvadApiDevice {
  *   struct.
  *
  * * `api_address`: pointer to nul-terminated UTF-8 string containing a socket address
- *   representation
- *   ("143.32.4.32:9090"), the port is mandatory.
+ *   representation ("143.32.4.32:9090"), the port is mandatory.
  *
  * * `hostname`: pointer to a null-terminated UTF-8 string representing the hostname that will be
  *   used for TLS validation.
+ * * `disable_tls`: only valid when built for tests, can be ignored when consumed by Swift.
  */
 struct MullvadApiError mullvad_api_client_initialize(struct MullvadApiClient *client_ptr,
                                                      const char *api_address_ptr,
-                                                     const char *hostname);
+                                                     const char *hostname,
+                                                     bool disable_tls);
 
 /**
  * Removes all devices from a given account
@@ -98,8 +99,8 @@ struct MullvadApiError mullvad_api_get_expiry(struct MullvadApiClient client_ptr
  * * `account_str_ptr`: pointer to nul-terminated UTF-8 string containing the account number of the
  *   account that will have all of it's devices removed.
  *
- * * `device_iter_ptr`: a pointer to a `device::MullvadApiDeviceIterator`. If this function
- *   doesn't return an error, the pointer will be initialized with a valid instance of
+ * * `device_iter_ptr`: a pointer to a `device::MullvadApiDeviceIterator`. If this function doesn't
+ *   return an error, the pointer will be initialized with a valid instance of
  *   `device::MullvadApiDeviceIterator`, which can be used to iterate through the devices.
  */
 struct MullvadApiError mullvad_api_list_devices(struct MullvadApiClient client_ptr,

mullvad-api/include/mullvad-api.h

@@ -1,7 +1,6 @@
 //! This module keeps track of the last known good API IP address and reads and stores it on disk.
 
-use super::API;
-use crate::DnsResolver;
+use crate::{ApiEndpoint, DnsResolver};
 use async_trait::async_trait;
 use std::{io, net::SocketAddr, path::Path, sync::Arc};
 use tokio::{
@@ -38,42 +37,42 @@ impl DnsResolver for AddressCache {
 
 #[derive(Clone)]
 pub struct AddressCache {
+    hostname: String,
     inner: Arc<Mutex<AddressCacheInner>>,
     write_path: Option<Arc<Path>>,
 }
 
 impl AddressCache {
     /// Initialize cache using the hardcoded address, and write changes to `write_path`.
-    pub fn new(write_path: Option<Box<Path>>) -> Self {
-        Self::new_inner(API.address(), write_path)
-    }
-
-    pub fn with_static_addr(address: SocketAddr) -> Self {
-        Self::new_inner(address, None)
+    pub fn new(endpoint: &ApiEndpoint, write_path: Option<Box<Path>>) -> Self {
+        Self::new_inner(endpoint.address(), endpoint.host().to_owned(), write_path)
     }
 
     /// Initialize cache using `read_path`, and write changes to `write_path`.
-    pub async fn from_file(read_path: &Path, write_path: Option<Box<Path>>) -> Result<Self, Error> {
+    pub async fn from_file(
+        read_path: &Path,
+        write_path: Option<Box<Path>>,
+        hostname: String,
+    ) -> Result<Self, Error> {
         log::debug!("Loading API addresses from {}", read_path.display());
-        Ok(Self::new_inner(
-            read_address_file(read_path).await?,
-            write_path,
-        ))
+        let address = read_address_file(read_path).await?;
+        Ok(Self::new_inner(address, hostname, write_path))
     }
 
-    fn new_inner(address: SocketAddr, write_path: Option<Box<Path>>) -> Self {
+    fn new_inner(address: SocketAddr, hostname: String, write_path: Option<Box<Path>>) -> Self {
         let cache = AddressCacheInner::from_address(address);
         log::debug!("Using API address: {}", cache.address);
 
         Self {
             inner: Arc::new(Mutex::new(cache)),
             write_path: write_path.map(Arc::from),
+            hostname,
         }
     }
 
     /// Returns the address if the hostname equals `API.host`. Otherwise, returns `None`.
     async fn resolve_hostname(&self, hostname: &str) -> Option<SocketAddr> {
-        if hostname.eq_ignore_ascii_case(API.host()) {
+        if hostname.eq_ignore_ascii_case(&self.hostname) {
             Some(self.get_address().await)
         } else {
             None

mullvad-api/src/address_cache.rs

@@ -2,14 +2,18 @@
 //! Used by the installer artifact packer to bundle the latest available
 //! relay list at the time of creating the installer.
 
-use mullvad_api::{proxy::ApiConnectionMode, rest::Error as RestError, RelayListProxy};
+use mullvad_api::{
+    proxy::ApiConnectionMode, rest::Error as RestError, ApiEndpoint, RelayListProxy,
+};
 use std::process;
 use talpid_types::ErrorExt;
 
 #[tokio::main]
 async fn main() {
-    let runtime = mullvad_api::Runtime::new(tokio::runtime::Handle::current())
-        .expect("Failed to load runtime");
+    let runtime = mullvad_api::Runtime::new(
+        tokio::runtime::Handle::current(),
+        &ApiEndpoint::from_env_vars(),
+    );
 
     let relay_list_request =
         RelayListProxy::new(runtime.mullvad_rest_handle(ApiConnectionMode::Direct.into_provider()))

mullvad-api/src/bin/relay_list.rs

@@ -13,6 +13,7 @@ pub enum MullvadApiErrorKind {
 
 /// MullvadApiErrorKind contains a description and an error kind. If the error kind is
 /// `MullvadApiErrorKind` is NoError, the pointer will be nil.
+#[derive(Debug)]
 #[repr(C)]
 pub struct MullvadApiError {
     description: *mut libc::c_char,
@@ -47,6 +48,13 @@ impl MullvadApiError {
         }
     }
 
+    pub fn unwrap(&self) {
+        if !matches!(self.kind, MullvadApiErrorKind::NoError) {
+            let desc = unsafe { std::ffi::CStr::from_ptr(self.description) };
+            panic!("API ERROR - {:?} - {}", self.kind, desc.to_str().unwrap());
+        }
+    }
+
     pub fn drop(self) {
         if self.description.is_null() {
             return;