Handle invalid proxy settings

composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/ProxyModel.kt

@@ -1,5 +1,6 @@
 package org.ooni.probe.data.models
 
+import co.touchlab.kermit.Logger
 import ooniprobe.composeapp.generated.resources.Res
 import ooniprobe.composeapp.generated.resources.Settings_Proxy_Custom
 import ooniprobe.composeapp.generated.resources.Settings_Proxy_None
@@ -70,21 +71,35 @@ enum class ProxyProtocol(val value: String) {
  * ProxySettings contains the settings configured inside the proxy. Please, see the
  * documentation of proxy activity for the design rationale.
  */
-class ProxySettings {
-    /**
-     * Scheme is the proxy scheme (e.g., "psiphon", "socks5").
-     */
-    var protocol: ProxyProtocol = ProxyProtocol.NONE
-
-    /**
-     * Hostname is the hostname for custom proxies.
-     */
-    var hostname: String = ""
-
-    /**
-     * Port is the port for custom proxies.
-     */
-    var port: String = ""
+sealed interface ProxySettings {
+    fun getProxyString(): String
+
+    data object None : ProxySettings {
+        override fun getProxyString() = ""
+    }
+
+    data object Psiphon : ProxySettings {
+        override fun getProxyString() = "psiphon://"
+    }
+
+    data class Custom(
+        val protocol: ProxyProtocol,
+        val hostname: String,
+        val port: Int,
+    ) : ProxySettings {
+        override fun getProxyString(): String {
+            val formattedHost = if (isIPv6(hostname)) {
+                "[$hostname]"
+            } else {
+                hostname
+            }
+            return "${protocol.value}://$formattedHost:$port/"
+        }
+
+        private fun isIPv6(hostname: String): Boolean {
+            return IPV6_ADDRESS.toRegex().matches(hostname)
+        }
+    }
 
     companion object {
         /**
@@ -97,65 +112,32 @@ class ProxySettings {
         fun newProxySettings(
             protocol: String?,
             hostname: String?,
-            port: String?,
-        ): ProxySettings {
-            val settings = ProxySettings()
-
-            protocol?.let { protocol ->
-                when (protocol) {
-                    ProxyProtocol.NONE.value -> {
-                        settings.protocol = ProxyProtocol.NONE
-                    }
-
-                    ProxyProtocol.PSIPHON.value -> {
-                        settings.protocol = ProxyProtocol.PSIPHON
-                    }
-
-                    ProxyProtocol.SOCKS5.value,
-                    ProxyProtocol.HTTP.value,
-                    ProxyProtocol.HTTPS.value,
-                    -> {
-                        settings.protocol = ProxyProtocol.fromValue(protocol)
-                    }
-
-                    else -> {
-                        // This is where we will extend the code to add support for
-                        // more proxies, e.g., HTTP proxies.
-                        throw InvalidProxyURL("unhandled URL scheme")
-                    }
+            port: Int?,
+        ): ProxySettings =
+            when (protocol) {
+                ProxyProtocol.NONE.value -> None
+                ProxyProtocol.PSIPHON.value -> Psiphon
+
+                ProxyProtocol.SOCKS5.value,
+                ProxyProtocol.HTTP.value,
+                ProxyProtocol.HTTPS.value,
+                -> run {
+                    Custom(
+                        protocol = ProxyProtocol.fromValue(protocol),
+                        hostname = hostname ?: return@run null,
+                        port = port ?: return@run null,
+                    )
                 }
-            } ?: run {
-                settings.protocol = ProxyProtocol.NONE
-            }
-
-            settings.apply {
-                this.hostname = hostname ?: ""
-                this.port = (port as Int? ?: "").toString()
-            }
-            return settings
-        }
-    }
 
-    fun getProxyString(): String {
-        when (protocol) {
-            ProxyProtocol.NONE -> return ""
-            ProxyProtocol.PSIPHON -> return "psiphon://"
-            ProxyProtocol.SOCKS5, ProxyProtocol.HTTP, ProxyProtocol.HTTPS -> {
-                val formattedHost = if (isIPv6(hostname)) {
-                    "[$hostname]"
-                } else {
-                    hostname
-                }
-                return "${protocol.value}://$formattedHost:$port/"
+                else -> null
+            } ?: run {
+                Logger.w(
+                    "Invalid proxy settings: protocol=$protocol hostname=$hostname port=$port",
+                    InvalidSettings(),
+                )
+                None
             }
-
-            else -> return ""
-        }
-    }
-
-    private fun isIPv6(hostname: String): Boolean {
-        return IPV6_ADDRESS.toRegex().matches(hostname)
     }
 
-    class InvalidProxyURL(message: String) : Exception(message)
+    class InvalidSettings : Exception("Invalid proxy settings")
 }

composeApp/src/commonMain/kotlin/org/ooni/probe/domain/GetProxySettings.kt

@@ -12,7 +12,7 @@ class GetProxySettings(
         ProxySettings.newProxySettings(
             protocol = getValueForKey(SettingsKey.PROXY_PROTOCOL) as? String,
             hostname = getValueForKey(SettingsKey.PROXY_HOSTNAME) as? String,
-            port = getValueForKey(SettingsKey.PROXY_PORT) as? String,
+            port = getValueForKey(SettingsKey.PROXY_PORT) as? Int,
         )
 
     private suspend fun getValueForKey(settingsKey: SettingsKey) = preferencesRepository.getValueByKey(settingsKey).first()

composeApp/src/commonTest/kotlin/org/ooni/probe/data/models/ProxyModelTest.kt

@@ -0,0 +1,56 @@
+package org.ooni.probe.data.models
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class ProxyModelTest {
+    @Test
+    fun newProxySettings_none() {
+        val settings = ProxySettings.newProxySettings(
+            protocol = null,
+            hostname = null,
+            port = null,
+        )
+
+        assertTrue(settings is ProxySettings.None)
+        assertEquals("", settings.getProxyString())
+    }
+
+    @Test
+    fun newProxySettings_psiphon() {
+        val settings = ProxySettings.newProxySettings(
+            protocol = "psiphon",
+            hostname = null,
+            port = null,
+        )
+
+        assertTrue(settings is ProxySettings.Psiphon)
+        assertEquals("psiphon://", settings.getProxyString())
+    }
+
+    @Test
+    fun newProxySettings_custom() {
+        val settings = ProxySettings.newProxySettings(
+            protocol = "http",
+            hostname = "example.org",
+            port = 80,
+        )
+
+        assertTrue(settings is ProxySettings.Custom)
+        assertEquals(ProxyProtocol.HTTP, settings.protocol)
+        assertEquals("example.org", settings.hostname)
+        assertEquals(80, settings.port)
+        assertEquals("http://example.org:80/", settings.getProxyString())
+    }
+
+    @Test
+    fun newProxySettings_withInvalidPort() {
+        val settings = ProxySettings.newProxySettings(
+            protocol = "http",
+            hostname = "example.org",
+            port = null,
+        )
+        assertEquals(ProxySettings.None, settings)
+    }
+}

gradle/libs.versions.toml

@@ -55,7 +55,7 @@ sqldelight-android = { module = "app.cash.sqldelight:android-driver", version.re
 sqldelight-native = { module = "app.cash.sqldelight:native-driver", version.ref = "sqldelight" }
 
 # Files
-okio = { module = "com.squareup.okio:okio", version = "3.9.1" }
+okio = { module = "com.squareup.okio:okio", version = "3.10.2" }
 
 # Lottie animations
 kottie = { module = "io.github.alexzhirkevich:compottie", version = "2.0.0-rc01" } # 2.0.0 not supported yet