Skip to content

Commit c418cec

Browse files
sdsantossdsantos
authored
Merge pull request #498 from ooni/handle-proxy-settings-issues
Handle invalid proxy settings
2 parents c62082a + c7651a8 commit c418cec

File tree

3 files changed

+110
-72
lines changed

3 files changed

+110
-72
lines changed

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

Lines changed: 53 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.ooni.probe.data.models
22

3+
import co.touchlab.kermit.Logger
34
import ooniprobe.composeapp.generated.resources.Res
45
import ooniprobe.composeapp.generated.resources.Settings_Proxy_Custom
56
import ooniprobe.composeapp.generated.resources.Settings_Proxy_None
@@ -70,21 +71,35 @@ enum class ProxyProtocol(val value: String) {
7071
* ProxySettings contains the settings configured inside the proxy. Please, see the
7172
* documentation of proxy activity for the design rationale.
7273
*/
73-
class ProxySettings {
74-
/**
75-
* Scheme is the proxy scheme (e.g., "psiphon", "socks5").
76-
*/
77-
var protocol: ProxyProtocol = ProxyProtocol.NONE
78-
79-
/**
80-
* Hostname is the hostname for custom proxies.
81-
*/
82-
var hostname: String = ""
83-
84-
/**
85-
* Port is the port for custom proxies.
86-
*/
87-
var port: String = ""
74+
sealed interface ProxySettings {
75+
fun getProxyString(): String
76+
77+
data object None : ProxySettings {
78+
override fun getProxyString() = ""
79+
}
80+
81+
data object Psiphon : ProxySettings {
82+
override fun getProxyString() = "psiphon://"
83+
}
84+
85+
data class Custom(
86+
val protocol: ProxyProtocol,
87+
val hostname: String,
88+
val port: Int,
89+
) : ProxySettings {
90+
override fun getProxyString(): String {
91+
val formattedHost = if (isIPv6(hostname)) {
92+
"[$hostname]"
93+
} else {
94+
hostname
95+
}
96+
return "${protocol.value}://$formattedHost:$port/"
97+
}
98+
99+
private fun isIPv6(hostname: String): Boolean {
100+
return IPV6_ADDRESS.toRegex().matches(hostname)
101+
}
102+
}
88103

89104
companion object {
90105
/**
@@ -97,65 +112,32 @@ class ProxySettings {
97112
fun newProxySettings(
98113
protocol: String?,
99114
hostname: String?,
100-
port: String?,
101-
): ProxySettings {
102-
val settings = ProxySettings()
103-
104-
protocol?.let { protocol ->
105-
when (protocol) {
106-
ProxyProtocol.NONE.value -> {
107-
settings.protocol = ProxyProtocol.NONE
108-
}
109-
110-
ProxyProtocol.PSIPHON.value -> {
111-
settings.protocol = ProxyProtocol.PSIPHON
112-
}
113-
114-
ProxyProtocol.SOCKS5.value,
115-
ProxyProtocol.HTTP.value,
116-
ProxyProtocol.HTTPS.value,
117-
-> {
118-
settings.protocol = ProxyProtocol.fromValue(protocol)
119-
}
120-
121-
else -> {
122-
// This is where we will extend the code to add support for
123-
// more proxies, e.g., HTTP proxies.
124-
throw InvalidProxyURL("unhandled URL scheme")
125-
}
115+
port: Int?,
116+
): ProxySettings =
117+
when (protocol) {
118+
ProxyProtocol.NONE.value -> None
119+
ProxyProtocol.PSIPHON.value -> Psiphon
120+
121+
ProxyProtocol.SOCKS5.value,
122+
ProxyProtocol.HTTP.value,
123+
ProxyProtocol.HTTPS.value,
124+
-> run {
125+
Custom(
126+
protocol = ProxyProtocol.fromValue(protocol),
127+
hostname = hostname ?: return@run null,
128+
port = port ?: return@run null,
129+
)
126130
}
127-
} ?: run {
128-
settings.protocol = ProxyProtocol.NONE
129-
}
130-
131-
settings.apply {
132-
this.hostname = hostname ?: ""
133-
this.port = (port as Int? ?: "").toString()
134-
}
135-
return settings
136-
}
137-
}
138131

139-
fun getProxyString(): String {
140-
when (protocol) {
141-
ProxyProtocol.NONE -> return ""
142-
ProxyProtocol.PSIPHON -> return "psiphon://"
143-
ProxyProtocol.SOCKS5, ProxyProtocol.HTTP, ProxyProtocol.HTTPS -> {
144-
val formattedHost = if (isIPv6(hostname)) {
145-
"[$hostname]"
146-
} else {
147-
hostname
148-
}
149-
return "${protocol.value}://$formattedHost:$port/"
132+
else -> null
133+
} ?: run {
134+
Logger.w(
135+
"Invalid proxy settings: protocol=$protocol hostname=$hostname port=$port",
136+
InvalidSettings(),
137+
)
138+
None
150139
}
151-
152-
else -> return ""
153-
}
154-
}
155-
156-
private fun isIPv6(hostname: String): Boolean {
157-
return IPV6_ADDRESS.toRegex().matches(hostname)
158140
}
159141

160-
class InvalidProxyURL(message: String) : Exception(message)
142+
class InvalidSettings : Exception("Invalid proxy settings")
161143
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class GetProxySettings(
1212
ProxySettings.newProxySettings(
1313
protocol = getValueForKey(SettingsKey.PROXY_PROTOCOL) as? String,
1414
hostname = getValueForKey(SettingsKey.PROXY_HOSTNAME) as? String,
15-
port = getValueForKey(SettingsKey.PROXY_PORT) as? String,
15+
port = getValueForKey(SettingsKey.PROXY_PORT) as? Int,
1616
)
1717

1818
private suspend fun getValueForKey(settingsKey: SettingsKey) = preferencesRepository.getValueByKey(settingsKey).first()

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package org.ooni.probe.data.models
2+
3+
import kotlin.test.Test
4+
import kotlin.test.assertEquals
5+
import kotlin.test.assertTrue
6+
7+
class ProxyModelTest {
8+
@Test
9+
fun newProxySettings_none() {
10+
val settings = ProxySettings.newProxySettings(
11+
protocol = null,
12+
hostname = null,
13+
port = null,
14+
)
15+
16+
assertTrue(settings is ProxySettings.None)
17+
assertEquals("", settings.getProxyString())
18+
}
19+
20+
@Test
21+
fun newProxySettings_psiphon() {
22+
val settings = ProxySettings.newProxySettings(
23+
protocol = "psiphon",
24+
hostname = null,
25+
port = null,
26+
)
27+
28+
assertTrue(settings is ProxySettings.Psiphon)
29+
assertEquals("psiphon://", settings.getProxyString())
30+
}
31+
32+
@Test
33+
fun newProxySettings_custom() {
34+
val settings = ProxySettings.newProxySettings(
35+
protocol = "http",
36+
hostname = "example.org",
37+
port = 80,
38+
)
39+
40+
assertTrue(settings is ProxySettings.Custom)
41+
assertEquals(ProxyProtocol.HTTP, settings.protocol)
42+
assertEquals("example.org", settings.hostname)
43+
assertEquals(80, settings.port)
44+
assertEquals("http://example.org:80/", settings.getProxyString())
45+
}
46+
47+
@Test
48+
fun newProxySettings_withInvalidPort() {
49+
val settings = ProxySettings.newProxySettings(
50+
protocol = "http",
51+
hostname = "example.org",
52+
port = null,
53+
)
54+
assertEquals(ProxySettings.None, settings)
55+
}
56+
}

0 commit comments

Comments
 (0)