diff --git a/network/data/src/main/kotlin/me/proton/core/network/data/ApiManagerFactory.kt b/network/data/src/main/kotlin/me/proton/core/network/data/ApiManagerFactory.kt index a0a45bf93..bc7bc599e 100644 --- a/network/data/src/main/kotlin/me/proton/core/network/data/ApiManagerFactory.kt +++ b/network/data/src/main/kotlin/me/proton/core/network/data/ApiManagerFactory.kt @@ -96,6 +96,7 @@ class ApiManagerFactory( private val clientVersionValidator: ClientVersionValidator, private val dohAlternativesListener: DohAlternativesListener?, private val dohProviderUrls: Array = Constants.DOH_PROVIDERS_URLS, + private val alternativeDohProviderUrls: List = Constants.ALTERNATIVE_DOH_PROVIDERS_URLS, private val okHttpClient: OkHttpClient, private val interceptors: Set>, ) { @@ -218,6 +219,8 @@ class ApiManagerFactory( baseUrl.toString(), apiClient, dohServices, + alternativeDohProviderUrls, + { serviceUrl -> DnsOverHttpsProviderRFC8484(baseOkHttpClient, serviceUrl, apiClient, networkManager) }, protonDohService, mainScope, prefs, diff --git a/network/data/src/main/kotlin/me/proton/core/network/data/NetworkPrefsImpl.kt b/network/data/src/main/kotlin/me/proton/core/network/data/NetworkPrefsImpl.kt index 23d04a8d2..66d3860c9 100644 --- a/network/data/src/main/kotlin/me/proton/core/network/data/NetworkPrefsImpl.kt +++ b/network/data/src/main/kotlin/me/proton/core/network/data/NetworkPrefsImpl.kt @@ -34,6 +34,7 @@ class NetworkPrefsImpl(context: Context) : NetworkPrefs, PreferencesProvider { override var activeAltBaseUrl: String? by string() override var lastPrimaryApiFail: Long by long(default = Long.MIN_VALUE) override var alternativeBaseUrls: List? by list() + override var successfulSecondaryDohServiceUrl: String? by string() companion object { private const val PREFS_NAME = "me.proton.core.network" diff --git a/network/data/src/main/kotlin/me/proton/core/network/data/di/Constants.kt b/network/data/src/main/kotlin/me/proton/core/network/data/di/Constants.kt index 5c0758330..6e2386eb9 100644 --- a/network/data/src/main/kotlin/me/proton/core/network/data/di/Constants.kt +++ b/network/data/src/main/kotlin/me/proton/core/network/data/di/Constants.kt @@ -49,4 +49,58 @@ object Constants { */ val DOH_PROVIDERS_URLS = arrayOf("https://dns11.quad9.net/dns-query/", "https://dns.google/dns-query/") + + /** + * Alternative DNS over HTTPS services urls. DohProvider will randomly pick from this list for additional resolvers. + */ + val ALTERNATIVE_DOH_PROVIDERS_URLS = listOf( + "https://anycast.dns.nextdns.io/dns-query/", + "https://130.59.31.248/dns-query/", + "https://dns-doh.dnsforfamily.com/dns-query/", + "https://94.140.14.14/dns-query/", + "https://1.1.1.2/dns-query/", + "https://pluton.plan9-dns.com/dns-query/", + "https://syd.adfilter.net/dns-query/", + "https://dnspub.restena.lu/dns-query/", + "https://blank.dnsforge.de/dns-query/", + "https://dns.mullvad.net/dns-query/", + "https://dns.digitale-gesellschaft.ch/dns-query/", + "https://dns.brahma.world/dns-query/", + "https://94.140.14.140/dns-query/", + "https://base.dns.mullvad.net/dns-query/", + "https://per.adfilter.net/dns-query/", + "https://adl.adfilter.net/dns-query/", + "https://family.dns.mullvad.net/dns-query/", + "https://dns.digitalsize.net/dns-query/", + "https://unicast.uncensoreddns.org/dns-query/", + "https://dns.circl.lu/dns-query/", + "https://dns12.quad9.net/dns-query/", + "https://all.dns.mullvad.net/dns-query/", + "https://dns.aa.net.uk/dns-query/", + "https://doh.libredns.gr/dns-query/", + "https://extended.dns.mullvad.net/dns-query/", + "https://dns.njal.la/dns-query/", + "https://dns9.quad9.net/dns-query/", + "https://149.112.112.9/dns-query/", + "https://freedns.controld.com/dns-query/", + "https://76.76.2.11/dns-query/", + "https://149.112.112.12/dns-query/", + "https://dnsforge.de/dns-query/", + "https://94.140.15.16/dns-query/", + "https://doq.dns4all.eu/dns-query/", + "https://ns1.fdn.fr/dns-query/", + "https://dns10.quad9.net/dns-query/", + "https://9.9.9.10/dns-query/", + "https://149.112.112.11/dns-query/", + "https://doh.ffmuc.net/dns-query/", + "https://odvr.nic.cz/dns-query/", + "https://ibksturm.synology.me/dns-query/", + "https://1.0.0.3/dns-query/", + "https://adblock.dns.mullvad.net/dns-query/", + "https://dns-doh-no-safe-search.dnsforfamily.com/dns-query/", + "https://dns1.dnscrypt.ca/dns-query/", + "https://cloudflare-dns.com/dns-query/", + "https://anycast.uncensoreddns.org/dns-query/", + "https://dns.quad9.net/dns-query/", + ) } diff --git a/network/data/src/test/java/me/proton/core/network/data/ApiManagerTests.kt b/network/data/src/test/java/me/proton/core/network/data/ApiManagerTests.kt index 0b9b64452..d0e2f08a0 100644 --- a/network/data/src/test/java/me/proton/core/network/data/ApiManagerTests.kt +++ b/network/data/src/test/java/me/proton/core/network/data/ApiManagerTests.kt @@ -143,7 +143,7 @@ internal class ApiManagerTests { if (firstArg() != null) session.sessionId else null } coEvery { sessionProvider.getSession(any()) } returns session - + DohProvider.lastAlternativesRefresh = Long.MIN_VALUE networkManager = MockNetworkManager() networkManager.networkStatus = NetworkStatus.Unmetered @@ -177,6 +177,8 @@ internal class ApiManagerTests { baseUrl, apiClient, listOf(dohService), + emptyList(), + { DohService { _, _ -> emptyList() } }, protonDohService, testScope, prefs, @@ -414,6 +416,8 @@ internal class ApiManagerTests { baseUrl, apiClient, listOf(dohService), + emptyList(), + { DohService { _, _ -> emptyList() } }, protonDohService, testScope, prefs, @@ -457,6 +461,8 @@ internal class ApiManagerTests { baseUrl, apiClient, listOf(dohService), + emptyList(), + { DohService { _, _ -> emptyList() } }, protonDohService, testScope, prefs, diff --git a/network/data/src/test/java/me/proton/core/network/data/DohProviderTests.kt b/network/data/src/test/java/me/proton/core/network/data/DohProviderTests.kt index 13cc48f61..b291dd0b8 100644 --- a/network/data/src/test/java/me/proton/core/network/data/DohProviderTests.kt +++ b/network/data/src/test/java/me/proton/core/network/data/DohProviderTests.kt @@ -25,6 +25,7 @@ import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk import kotlinx.coroutines.delay +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runTest import me.proton.core.network.data.doh.DnsOverHttpsProviderRFC8484 @@ -49,6 +50,13 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +private val primaryService = DohService { _, _ -> delay(100); listOf("1.1.1.1")} +private val primaryServiceFail = DohService { _, _ -> delay(1000); null } +private val secondaryService = DohService { _, _ -> delay(10); listOf("2.2.2.2") } +private val secondaryServiceFail = DohService { _, _ -> delay(1000); null } +private const val SECONDARY_URL = "https://secondary.com" +private const val SECONDARY_URL_FAIL = "https://secondaryFail.com" + @Config(sdk = [Build.VERSION_CODES.M]) @RunWith(RobolectricTestRunner::class) internal class DohProviderTests { @@ -65,10 +73,11 @@ internal class DohProviderTests { @BeforeTest fun before() { MockKAnnotations.init(this) - every { networkManager.isConnectedToNetwork() } returns isNetworkAvailable + every { networkManager.isConnectedToNetwork() } answers { isNetworkAvailable } isNetworkAvailable = true webServer = MockWebServer() + DohProvider.lastAlternativesRefresh = Long.MIN_VALUE val okHttpClient = OkHttpClient.Builder().build() val client = MockApiClient() dohProvider = DnsOverHttpsProviderRFC8484( @@ -120,18 +129,13 @@ internal class DohProviderTests { listOf("proxy.com") } - val protonService = mockk() val prefs = MockNetworkPrefs() - val provider = DohProvider( - "https://test.com", - MockApiClient(), + val provider = createDohProvider( listOf(service1, service2), - protonService, - this, + emptyList(), + { url -> mockk() }, prefs, - { currentTime }, - null, - null) + ) val start = currentTime provider.refreshAlternatives() @@ -144,4 +148,112 @@ internal class DohProviderTests { // Make sure it finishes as soon as one service succeeds. assertEquals(1000, duration) } + + @Test + fun `primary providers results are preferred to secondary ones`() = runTest { + val prefs = MockNetworkPrefs() + val provider = createDohProvider( + listOf(primaryService), + listOf(SECONDARY_URL, SECONDARY_URL_FAIL), + { url -> + when (url) { + SECONDARY_URL -> secondaryService + SECONDARY_URL_FAIL -> secondaryServiceFail + else -> throw IllegalStateException("Unexpected url") + } + }, + prefs, + ) + + val start = currentTime + provider.refreshAlternatives() + val duration = currentTime - start + + assertEquals(listOf("1.1.1.1"), prefs.alternativeBaseUrls) + + // We'll wait for the primary service to finish, even if secondary one finished already. + assertEquals(100, duration) + assertEquals(SECONDARY_URL, prefs.successfulSecondaryDohServiceUrl) + } + + @Test + fun `when primary providers fail, secondary results are used` () = runTest { + val prefs = MockNetworkPrefs() + val provider = createDohProvider( + listOf(primaryServiceFail), + listOf(SECONDARY_URL), + { secondaryService }, + prefs, + ) + + val start = currentTime + provider.refreshAlternatives() + val duration = currentTime - start + + assertEquals(listOf("2.2.2.2"), prefs.alternativeBaseUrls) + assertEquals(SECONDARY_URL, prefs.successfulSecondaryDohServiceUrl) + // Wait for primary to fail (1000ms) and only then take secondary result. + assertEquals(1000, duration) + } + + @Test + fun `when successful secondary fails, pref is cleared`() = runTest { + val prefs = MockNetworkPrefs() + prefs.successfulSecondaryDohServiceUrl = SECONDARY_URL_FAIL + + val primaryServiceLong = DohService { _, _ -> delay(2_000); listOf("1.1.1.1") } + + val provider = createDohProvider( + listOf(primaryServiceLong), + listOf(SECONDARY_URL_FAIL), + { secondaryServiceFail }, + prefs, + ) + + val start = currentTime + provider.refreshAlternatives() + val duration = currentTime - start + + assertEquals(null, prefs.successfulSecondaryDohServiceUrl) + assertEquals(listOf("1.1.1.1"), prefs.alternativeBaseUrls) + assertEquals(2_000, duration) + } + + @Test + fun `when primary works it doesn't wait for slow secondary`() = runTest { + val prefs = MockNetworkPrefs() + val secondaryServiceSlow = DohService { _, _ -> delay(5_000); listOf("2.2.2.2") } + val provider = createDohProvider( + listOf(primaryService), + listOf(SECONDARY_URL), + { secondaryServiceSlow }, + prefs, + ) + + val start = currentTime + provider.refreshAlternatives() + val duration = currentTime - start + + assertEquals(listOf("1.1.1.1"), prefs.alternativeBaseUrls) + assertEquals(100, duration) + } + + private fun TestScope.createDohProvider( + primaryServices: List = listOf(primaryService), + secondaryServicesUrls: List = listOf(SECONDARY_URL), + createSecondaryService: (String) -> DohService = { secondaryService }, + prefs: MockNetworkPrefs = MockNetworkPrefs(), + ) = DohProvider( + "https://test.com", + MockApiClient(), + primaryServices, + secondaryServicesUrls, + createSecondaryService, + mockk(), + this, + prefs, + ::currentTime, + null, + null + ) } diff --git a/network/data/src/test/java/me/proton/core/network/data/util/MockClient.kt b/network/data/src/test/java/me/proton/core/network/data/util/MockClient.kt index ecb2674ab..dde00f343 100644 --- a/network/data/src/test/java/me/proton/core/network/data/util/MockClient.kt +++ b/network/data/src/test/java/me/proton/core/network/data/util/MockClient.kt @@ -98,6 +98,7 @@ class MockNetworkPrefs : NetworkPrefs { override var activeAltBaseUrl: String? = null override var lastPrimaryApiFail: Long = Long.MIN_VALUE override var alternativeBaseUrls: List? = null + override var successfulSecondaryDohServiceUrl: String? = null } class MockLogger : Logger { diff --git a/network/domain/src/main/kotlin/me/proton/core/network/domain/DohProvider.kt b/network/domain/src/main/kotlin/me/proton/core/network/domain/DohProvider.kt index 4b033c3d6..b51d25aa8 100644 --- a/network/domain/src/main/kotlin/me/proton/core/network/domain/DohProvider.kt +++ b/network/domain/src/main/kotlin/me/proton/core/network/domain/DohProvider.kt @@ -32,19 +32,21 @@ import java.util.concurrent.TimeUnit /** * Gets the list of alternative baseUrls for Proton API. */ -interface DohService { +fun interface DohService { suspend fun getAlternativeBaseUrls(sessionId: SessionId?, primaryBaseUrl: String): List? } /** - * Refreshes alternative urls for [baseUrl] using given list of DoH services ([dohServices]). Makes - * sure that only one refresh operation takes place at one time for given baseUrl. Single instance - * should exist per baseUrl. + * Refreshes alternative urls for [baseUrl] using given list of DoH services ([primaryDohServices] + randomly selected + * services from [secondaryDohServicesUrls]). Makes sure that only one refresh operation takes place at one time for + * given baseUrl. Single instance should exist per baseUrl. */ class DohProvider( private val baseUrl: String, private val apiClient: ApiClient, - private val dohServices: List, + private val primaryDohServices: List, + private val secondaryDohServicesUrls: List, + private val createSecondaryDohService: (String) -> DohService, private val protonDohService: DohService, private val networkMainScope: CoroutineScope, private val prefs: NetworkPrefs, @@ -77,24 +79,51 @@ class DohProvider( } private suspend fun tryDohServices(): Boolean = coroutineScope { - var allServicesFailed = true + // Select 2 random secondary services, but include successful one if available. + val secondaryServiceUrls = + prefs.successfulSecondaryDohServiceUrl?.let { + listOfNotNull(it, secondaryDohServicesUrls.randomOrNull()) + } ?: secondaryDohServicesUrls.asSequence().shuffled().take(2).toList() + val secondaryDohServices = secondaryServiceUrls.map { createSecondaryDohService(it) } + val dohServices = primaryDohServices + secondaryDohServices + + val successfulServices = mutableListOf>>() val jobs = mutableListOf() dohServices.mapTo(jobs) { service -> launch { - val success = withTimeoutOrNull(apiClient.dohServiceTimeoutMs) { - val result = service.getAlternativeBaseUrls(sessionId, baseUrl) - if (result != null) - prefs.alternativeBaseUrls = result - result != null - } ?: false - if (success) { - allServicesFailed = false - jobs.forEach { it.cancel() } + val isSecondaryService = service in secondaryDohServices + val result = withTimeoutOrNull(apiClient.dohServiceTimeoutMs) { + service.getAlternativeBaseUrls(sessionId, baseUrl) + } + if (result != null) { + successfulServices += service to result + if (!isSecondaryService) { + jobs.forEach { it.cancel() } + } + } else if (isSecondaryService) { + val index = secondaryDohServices.indexOf(service) + if (index != -1 && secondaryServiceUrls[index] == prefs.successfulSecondaryDohServiceUrl) { + prefs.successfulSecondaryDohServiceUrl = null + } } } } jobs.joinAll() - allServicesFailed + + val firstSuccessfulPrimaryService = successfulServices.firstOrNull { (service, _) -> service in primaryDohServices } + val firstSuccessfulSecondaryService = successfulServices.firstOrNull { (service, _) -> service in secondaryDohServices } + if (firstSuccessfulPrimaryService != null) { + prefs.alternativeBaseUrls = firstSuccessfulPrimaryService.second + } else if (firstSuccessfulSecondaryService != null) { + prefs.alternativeBaseUrls = firstSuccessfulSecondaryService.second + } + if (firstSuccessfulSecondaryService != null) { + prefs.successfulSecondaryDohServiceUrl = + secondaryServiceUrls[secondaryDohServices.indexOf(firstSuccessfulSecondaryService.first)] + } + + val allFailed = successfulServices.isEmpty() + allFailed } companion object { diff --git a/network/domain/src/main/kotlin/me/proton/core/network/domain/NetworkPrefs.kt b/network/domain/src/main/kotlin/me/proton/core/network/domain/NetworkPrefs.kt index f0d330dfe..8971da6c9 100644 --- a/network/domain/src/main/kotlin/me/proton/core/network/domain/NetworkPrefs.kt +++ b/network/domain/src/main/kotlin/me/proton/core/network/domain/NetworkPrefs.kt @@ -31,4 +31,7 @@ interface NetworkPrefs { /** List of base urls for Proton API proxies returned in the last DoH query. */ var alternativeBaseUrls: List? + + /** Url/IP of secondary DoH service that successfully responded last time. */ + var successfulSecondaryDohServiceUrl: String? }