diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 3ff611686..34ba10c86 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -132,6 +132,8 @@ dependencies { implementation("com.journeyapps:zxing-android-embedded:4.3.0") { isTransitive = false } implementation(libs.zxing.core) + implementation(libs.okhttp) + implementation(libs.hilt.android) implementation(libs.androidx.hilt.navigation.compose) ksp(libs.hilt.android.compiler) diff --git a/app/src/androidTest/java/org/fdroid/download/DnsCacheTest.kt b/app/src/androidTest/java/org/fdroid/download/DnsCacheTest.kt new file mode 100644 index 000000000..1d157b7d4 --- /dev/null +++ b/app/src/androidTest/java/org/fdroid/download/DnsCacheTest.kt @@ -0,0 +1,103 @@ +package org.fdroid.download + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.fdroid.settings.SettingsManager +import org.junit.runner.RunWith +import java.net.InetAddress +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class DnsCacheTest { + + private val context = ApplicationProvider.getApplicationContext() + private val settings = SettingsManager(context) + + private val url1 = "locaihost" + private val url2 = "fdroid.org" + private val url3 = "fdroid.net" + + private val ip1 = InetAddress.getByName("127.0.0.1") + private val ip2 = InetAddress.getByName("127.0.0.2") + private val ip3 = InetAddress.getByName("127.0.0.3") + + private val list1 = listOf(ip1, ip2, ip3) + private val list2 = listOf(ip2) + private val list3 = listOf(ip3) + + @Test + fun basicCacheTest() { + // test setup + settings.useDnsCache = true + val testObject = DnsCache(settings) + + // populate cache + testObject.insert(url1, list1) + testObject.insert(url2, list2) + testObject.insert(url3, list3) + + // check for cached lookup results + val testList1 = testObject.lookup(url1) + assertNotNull(testList1) + assertEquals(3, testList1.size.toLong()) + assertEquals(ip1.hostAddress, testList1[0].hostAddress) + assertEquals(ip2.hostAddress, testList1[1].hostAddress) + assertEquals(ip3.hostAddress, testList1[2].hostAddress) + + // toggle preference (false) + settings.useDnsCache = false + + // attempt non-cached lookup + val testList2 = testObject.lookup(url1) + assertNull(testList2) + + // toggle preference (true) + settings.useDnsCache = true + + // confirm lookup results remain in cache + val testList3 = testObject.lookup(url2) + assertNotNull(testList3) + assertEquals(1, testList3.size.toLong()) + assertEquals(ip2.hostAddress, testList3[0].hostAddress) + + // test removal + testObject.remove(url2) + + // confirm result was removed from cache + val testList4 = testObject.lookup(url2) + assertNull(testList4) + } + + @Test + fun dnsRetryTest() { + // test setup + settings.useDnsCache = true + val testCache = DnsCache(settings) + val testObject = DnsWithCache(settings, testCache) + + // insert dummy value into cache + testCache.insert(url2, list2) + + // check initial status + val testList1 = testObject.lookup(url2) + assertEquals(1, testList1.size.toLong()) + assertEquals(ip2.hostAddress, testList1[0].hostAddress) + + // mismatch with dummy value should require retry and clear cache + val testFlag = testObject.shouldRetryRequest(url2) + assertTrue(testFlag) + val testList2 = testCache.lookup(url2) + assertNull(testList2) + + // subsequent lookup should cache actual dns result (not testing actual values) + val testList3 = testObject.lookup(url2) + assertNotNull(testList3) + val testList4 = testCache.lookup(url2) + assertNotNull(testList4) + } +} diff --git a/app/src/main/kotlin/org/fdroid/download/DnsCache.kt b/app/src/main/kotlin/org/fdroid/download/DnsCache.kt new file mode 100644 index 000000000..fa8578e2f --- /dev/null +++ b/app/src/main/kotlin/org/fdroid/download/DnsCache.kt @@ -0,0 +1,105 @@ +package org.fdroid.download + +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import mu.KotlinLogging +import org.fdroid.settings.SettingsManager +import java.net.InetAddress +import java.net.UnknownHostException +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.concurrent.atomics.AtomicBoolean +import kotlin.concurrent.atomics.ExperimentalAtomicApi + +@OptIn(ExperimentalAtomicApi::class) +@Singleton +class DnsCache @Inject constructor( + private val settingsManager: SettingsManager +) { + + private val log = KotlinLogging.logger {} + + private var cache: MutableMap> + private var writeScheduled: AtomicBoolean = AtomicBoolean(false) + + init { + cache = stringToIpMap(settingsManager.dnsCache).toMutableMap() + } + + fun insert(hostname: String, ipList: List) { + cache[hostname] = ipList + cacheWrite() + } + + fun remove(hostname: String) { + val ipList = cache.remove(hostname) + if (ipList != null) { + cacheWrite() + } + } + + fun lookup(hostname: String): List? { + return if (settingsManager.useDnsCache) { + cache[hostname] + } else { + null + } + } + + private fun cacheWrite() { + if (writeScheduled.compareAndSet(expectedValue = false, newValue = true)) { + MainScope().launch { + delay(1000L) + settingsManager.dnsCache = ipMapToString(cache) + writeScheduled.store(false) + } + } + } + + private fun ipMapToString(ipMap: Map>): String { + try { + var output = "" + ipMap.forEach { (key, addresses) -> + if (!output.isEmpty()) { + output += "\n" + } + output += key + for (item in addresses) { + if (!output.isEmpty()) { + output += " " + } + output += item.hostAddress + } + } + return output + } catch (e: Exception) { + log.error(e) { "Error converting IP map to string, returning empty string: " } + return "" + } + } + + private fun stringToIpMap(string: String): Map> { + try { + val output = mutableMapOf>() + for (line in string.split("\n")) { + val items = line.split(" ").toMutableList() + val key = items.removeAt(0) + val ipList = mutableListOf() + for (ip in items) { + try { + ipList.add(InetAddress.getByName(ip)) + } catch (e: UnknownHostException) { + // should not occur, if an ip address is supplied only the format is checked + log.error(e) { "Error parsing IP address, moving on to next item: " } + } + } + output[key] = ipList + } + return output + } catch (e: Exception) { + log.error(e) { "Error converting string to IP map, returning empty map: " } + return emptyMap() + } + } +} diff --git a/app/src/main/kotlin/org/fdroid/download/DnsWithCache.kt b/app/src/main/kotlin/org/fdroid/download/DnsWithCache.kt new file mode 100644 index 000000000..084309099 --- /dev/null +++ b/app/src/main/kotlin/org/fdroid/download/DnsWithCache.kt @@ -0,0 +1,59 @@ +package org.fdroid.download + +import okhttp3.Dns +import org.fdroid.settings.SettingsManager +import java.net.InetAddress +import java.net.UnknownHostException +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class DnsWithCache @Inject constructor( + private val settingsManager: SettingsManager, + private val cache: DnsCache +) : Dns { + + override fun lookup(hostname: String): List { + if (!settingsManager.useDnsCache) { + return Dns.SYSTEM.lookup(hostname) + } + var ipList = cache.lookup(hostname) + if (ipList == null) { + ipList = Dns.SYSTEM.lookup(hostname) + cache.insert(hostname, ipList) + } + return ipList + } + + /** + * in case a host is unreachable, check whether the cached dns result is different from + * the current result. if the cached result is different, remove that result from the + * cache. returns true if a cached result was removed, indicating that the connection + * should be retried, otherwise returns false. + */ + fun shouldRetryRequest(hostname: String): Boolean { + if (!settingsManager.useDnsCache) { + // the cache feature was not enabled, so a cached result didn't cause the failure + return false + } + // if no cached result was found, a cached result didn't cause the failure + val ipList = cache.lookup(hostname) ?: return false + try { + val dnsList = Dns.SYSTEM.lookup(hostname) + for (address in dnsList) { + if (!ipList.contains(address)) { + // the cached result doesn't match the current dns result, + // so the connection should be retried + cache.remove(hostname) + return true + } + } + // the cached result matches the current dns result, + // so a cached result didn't cause the failure + return false + } catch (_: UnknownHostException) { + // the url returned an unknown host exception, so there's no point in retrying + return false + } + } +} diff --git a/app/src/main/kotlin/org/fdroid/download/DownloadModule.kt b/app/src/main/kotlin/org/fdroid/download/DownloadModule.kt index f3d860583..0d6d85a1d 100644 --- a/app/src/main/kotlin/org/fdroid/download/DownloadModule.kt +++ b/app/src/main/kotlin/org/fdroid/download/DownloadModule.kt @@ -18,12 +18,14 @@ object DownloadModule { @Singleton fun provideHttpManager( settingsManager: SettingsManager, - mirrorParameters: MirrorParameters, + dns: DnsWithCache, + manager: FDroidMirrorParameterManager ): HttpManager { return HttpManager( userAgent = USER_AGENT, proxyConfig = settingsManager.proxyConfig, - mirrorParameterManager = mirrorParameters, + customDns = dns, + mirrorParameterManager = manager ) } diff --git a/app/src/main/kotlin/org/fdroid/download/MirrorParameters.kt b/app/src/main/kotlin/org/fdroid/download/FDroidMirrorParameterManager.kt similarity index 72% rename from app/src/main/kotlin/org/fdroid/download/MirrorParameters.kt rename to app/src/main/kotlin/org/fdroid/download/FDroidMirrorParameterManager.kt index abb554d10..46465b433 100644 --- a/app/src/main/kotlin/org/fdroid/download/MirrorParameters.kt +++ b/app/src/main/kotlin/org/fdroid/download/FDroidMirrorParameterManager.kt @@ -8,13 +8,24 @@ import org.fdroid.settings.SettingsConstants import org.fdroid.settings.SettingsManager import java.util.Locale import javax.inject.Inject +import javax.inject.Singleton +import kotlin.concurrent.atomics.ExperimentalAtomicApi -class MirrorParameters @Inject constructor( +@OptIn(ExperimentalAtomicApi::class) +@Singleton +class FDroidMirrorParameterManager @Inject constructor( @param:ApplicationContext private val context: Context, - private val settingsManager: SettingsManager + private val settingsManager: SettingsManager, + private val dnsWithCache: DnsWithCache ) : MirrorParameterManager { + + override fun shouldRetryRequest(mirrorUrl: String): Boolean { + return dnsWithCache.shouldRetryRequest(mirrorUrl) + } + // TODO overhaul default MirrorChooser override fun incrementMirrorErrorCount(mirrorUrl: String) {} + override fun getMirrorErrorCount(mirrorUrl: String): Int = 0 override fun preferForeignMirrors(): Boolean { diff --git a/app/src/main/kotlin/org/fdroid/settings/SettingsConstants.kt b/app/src/main/kotlin/org/fdroid/settings/SettingsConstants.kt index 6113562ef..f73f592fa 100644 --- a/app/src/main/kotlin/org/fdroid/settings/SettingsConstants.kt +++ b/app/src/main/kotlin/org/fdroid/settings/SettingsConstants.kt @@ -43,6 +43,12 @@ object SettingsConstants { const val PREF_KEY_PROXY = "proxy" const val PREF_DEFAULT_PROXY = "" + const val PREF_USE_DNS_CACHE = "useDnsCache" + const val PREF_USE_DNS_CACHE_DEFAULT = false + + const val PREF_DNS_CACHE = "dnsCache" + const val PREF_DNS_CACHE_DEFAULT = "" + const val PREF_KEY_PREVENT_SCREENSHOTS = "preventScreenshots" const val PREF_DEFAULT_PREVENT_SCREENSHOTS = false diff --git a/app/src/main/kotlin/org/fdroid/settings/SettingsManager.kt b/app/src/main/kotlin/org/fdroid/settings/SettingsManager.kt index f022cd526..b30f99c37 100644 --- a/app/src/main/kotlin/org/fdroid/settings/SettingsManager.kt +++ b/app/src/main/kotlin/org/fdroid/settings/SettingsManager.kt @@ -28,6 +28,8 @@ import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_PROXY import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_REPO_UPDATES import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_SHOW_INCOMPATIBLE import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_THEME +import org.fdroid.settings.SettingsConstants.PREF_DNS_CACHE +import org.fdroid.settings.SettingsConstants.PREF_DNS_CACHE_DEFAULT import org.fdroid.settings.SettingsConstants.PREF_KEY_APP_LIST_SORT_ORDER import org.fdroid.settings.SettingsConstants.PREF_KEY_AUTO_UPDATES import org.fdroid.settings.SettingsConstants.PREF_KEY_DYNAMIC_COLORS @@ -41,6 +43,8 @@ import org.fdroid.settings.SettingsConstants.PREF_KEY_PROXY import org.fdroid.settings.SettingsConstants.PREF_KEY_REPO_UPDATES import org.fdroid.settings.SettingsConstants.PREF_KEY_SHOW_INCOMPATIBLE import org.fdroid.settings.SettingsConstants.PREF_KEY_THEME +import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE +import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE_DEFAULT import org.fdroid.settings.SettingsConstants.getAppListSortOrder import org.fdroid.settings.SettingsConstants.toSettings import java.net.InetSocketAddress @@ -151,6 +155,22 @@ class SettingsManager @Inject constructor( it.get(PREF_KEY_PREVENT_SCREENSHOTS) ?: PREF_DEFAULT_PREVENT_SCREENSHOTS }.distinctUntilChanged() + var useDnsCache: Boolean + get() { + return prefs.getBoolean(PREF_USE_DNS_CACHE, PREF_USE_DNS_CACHE_DEFAULT) + } + set(value) { + return prefs.edit { putBoolean(PREF_USE_DNS_CACHE, value) } + } + + var dnsCache: String + get() { + return prefs.getString(PREF_DNS_CACHE, null) ?: PREF_DNS_CACHE_DEFAULT + } + set(value) { + return prefs.edit { putString(PREF_DNS_CACHE, value) } + } + val filterIncompatible: Boolean get() = !prefs.getBoolean(PREF_KEY_SHOW_INCOMPATIBLE, PREF_DEFAULT_SHOW_INCOMPATIBLE) val appListSortOrder: AppListSortOrder diff --git a/app/src/main/kotlin/org/fdroid/ui/repositories/details/RepoDetailsViewModel.kt b/app/src/main/kotlin/org/fdroid/ui/repositories/details/RepoDetailsViewModel.kt index af7a80b40..d71e1b936 100644 --- a/app/src/main/kotlin/org/fdroid/ui/repositories/details/RepoDetailsViewModel.kt +++ b/app/src/main/kotlin/org/fdroid/ui/repositories/details/RepoDetailsViewModel.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.withContext import mu.KotlinLogging import org.fdroid.database.FDroidDatabase import org.fdroid.database.Repository +import org.fdroid.download.DnsCache import org.fdroid.download.Mirror import org.fdroid.download.NetworkMonitor import org.fdroid.index.RepoManager @@ -46,6 +47,7 @@ class RepoDetailsViewModel @AssistedInject constructor( private val updateManager: UpdatesManager, repoUpdateManager: RepoUpdateManager, private val settingsManager: SettingsManager, + private val dnsCache: DnsCache, private val onboardingManager: OnboardingManager, @param:IoDispatcher private val ioScope: CoroutineScope, ) : AndroidViewModel(app), RepoDetailsActions { @@ -96,6 +98,14 @@ class RepoDetailsViewModel @AssistedInject constructor( override fun deleteRepository() { ioScope.launch { + // before deleting a repo, clear the related entries from the dns cache + val repo = repoManager.getRepository(repoId) + if (repo != null) { + val mirrors = repo.getMirrors() + for (mirror in mirrors) { + dnsCache.remove(mirror.url.host) + } + } repoManager.deleteRepository(repoId) updateManager.loadUpdates() } diff --git a/app/src/main/kotlin/org/fdroid/ui/settings/Settings.kt b/app/src/main/kotlin/org/fdroid/ui/settings/Settings.kt index 5adf52d18..921f2e501 100644 --- a/app/src/main/kotlin/org/fdroid/ui/settings/Settings.kt +++ b/app/src/main/kotlin/org/fdroid/ui/settings/Settings.kt @@ -16,6 +16,7 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.BrightnessMedium import androidx.compose.material.icons.filled.ColorLens +import androidx.compose.material.icons.filled.Dns import androidx.compose.material.icons.filled.Lan import androidx.compose.material.icons.filled.Notifications import androidx.compose.material.icons.filled.Save @@ -74,6 +75,8 @@ import org.fdroid.settings.SettingsConstants.PREF_KEY_PREVENT_SCREENSHOTS import org.fdroid.settings.SettingsConstants.PREF_KEY_PROXY import org.fdroid.settings.SettingsConstants.PREF_KEY_REPO_UPDATES import org.fdroid.settings.SettingsConstants.PREF_KEY_THEME +import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE +import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE_DEFAULT import org.fdroid.settings.toAutoUpdateValue import org.fdroid.settings.toMirrorChooserValue import org.fdroid.ui.FDroidContent @@ -353,6 +356,23 @@ fun Settings( key = "pref_category_privacy", title = { Text(stringResource(R.string.privacy)) }, ) + switchPreference( + key = PREF_USE_DNS_CACHE, + defaultValue = PREF_USE_DNS_CACHE_DEFAULT, + title = { + Text(stringResource(R.string.useDnsCache)) + }, + icon = { + Icon( + imageVector = Icons.Default.Dns, + contentDescription = null, + modifier = Modifier.semantics { hideFromAccessibility() }, + ) + }, + summary = { + Text(stringResource(R.string.useDnsCacheSummary)) + }, + ) switchPreference( key = PREF_KEY_PREVENT_SCREENSHOTS, defaultValue = PREF_DEFAULT_PREVENT_SCREENSHOTS, diff --git a/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsCacheTest.kt b/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsCacheTest.kt new file mode 100644 index 000000000..56e8d9891 --- /dev/null +++ b/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsCacheTest.kt @@ -0,0 +1,93 @@ +package org.fdroid.fdroid.net + +import org.fdroid.fdroid.Preferences +import org.junit.Assert +import java.net.InetAddress +import java.util.Arrays +import kotlin.test.Test + +class DnsCacheTest { + + private val url1: String = "locaihost" + private val url2: String = "fdroid.org" + private val url3: String = "fdroid.net" + + private val ip1: InetAddress = InetAddress.getByName("127.0.0.1") + private val ip2: InetAddress = InetAddress.getByName("127.0.0.2") + private val ip3: InetAddress = InetAddress.getByName("127.0.0.3") + + private val list1: MutableList = Arrays.asList(ip1, ip2, ip3) + private val list2: MutableList = Arrays.asList(ip2) + private val list3: MutableList = Arrays.asList(ip3) + + @Test + fun basicCacheTest() { + // test setup + val prefs = Preferences.get() + prefs.setDnsCacheEnabledValue(true) + val testObject = DnsCache.get() + + // populate cache + testObject.insert(url1, list1) + testObject.insert(url2, list2) + testObject.insert(url3, list3) + + // check for cached lookup results + val testList1 = testObject.lookup(url1) + Assert.assertEquals(3, testList1.size.toLong()) + Assert.assertEquals(ip1.hostAddress, testList1[0]!!.hostAddress) + Assert.assertEquals(ip2.hostAddress, testList1[1]!!.hostAddress) + Assert.assertEquals(ip3.hostAddress, testList1[2]!!.hostAddress) + + // toggle preference (false) + prefs.setDnsCacheEnabledValue(false) + + // attempt non-cached lookup + val testList2 = testObject.lookup(url1) + Assert.assertNull(testList2) + + // toggle preference (true) + prefs.setDnsCacheEnabledValue(true) + + // confirm lookup results remain in cache + val testList3 = testObject.lookup(url2) + Assert.assertEquals(1, testList3.size.toLong()) + Assert.assertEquals(ip2.hostAddress, testList3[0].hostAddress) + + // test removal + val testList4 = testObject.remove(url2) + Assert.assertEquals(1, testList4.size.toLong()) + Assert.assertEquals(ip2.hostAddress, testList4[0].hostAddress) + val testList5 = testObject.lookup(url2) + Assert.assertNull(testList5) + } + + @Test + fun dnsRetryTest() { + // test setup + val prefs = Preferences.get() + prefs.setDnsCacheEnabledValue(true) + val testObject = DnsWithCache.get() + val testCache = DnsCache.get() + + // insert dummy value into cache + testCache.insert(url2, list2) + + // check initial status + val testList1 = testObject.lookup(url2) + Assert.assertEquals(1, testList1.size.toLong()) + Assert.assertEquals(ip2.hostAddress, testList1[0].hostAddress) + + // mismatch with dummy value should require retry and clear cache + val testFlag = testObject.shouldRetryRequest(url2) + Assert.assertTrue(testFlag) + val testList2 = testCache.lookup(url2) + Assert.assertNull(testList2) + + // subsequent lookup should cache actual dns result (not testing actual values) + val testList3 = testObject.lookup(url2) + Assert.assertNotNull(testList3) + val testList4 = testCache.lookup(url2) + Assert.assertNotNull(testList4) + } +} diff --git a/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsWithCacheTest.java b/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsWithCacheTest.java deleted file mode 100644 index abbe40c98..000000000 --- a/legacy/src/androidTest/java/org/fdroid/fdroid/net/DnsWithCacheTest.java +++ /dev/null @@ -1,80 +0,0 @@ -package org.fdroid.fdroid.net; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; - -import org.fdroid.fdroid.Preferences; -import org.junit.Test; - -import java.io.IOException; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.Arrays; -import java.util.List; - -public class DnsWithCacheTest { - - private static final String URL_1 = "locaihost"; - private static final String URL_2 = "fdroid.org"; - private static final String URL_3 = "fdroid.net"; - - private static final InetAddress IP_1; - private static final InetAddress IP_2; - private static final InetAddress IP_3; - - static { - try { - IP_1 = InetAddress.getByName("127.0.0.1"); - IP_2 = InetAddress.getByName("127.0.0.2"); - IP_3 = InetAddress.getByName("127.0.0.3"); - } catch (UnknownHostException e) { - throw new RuntimeException(e); - } - } - - private static final List LIST_1 = Arrays.asList(IP_1, IP_2, IP_3); - private static final List LIST_2 = Arrays.asList(IP_2); - private static final List LIST_3 = Arrays.asList(IP_3); - - @Test - public void basicCacheTest() throws IOException, InterruptedException { - // test setup - Preferences prefs = Preferences.get(); - prefs.setDnsCacheEnabledValue(true); - DnsCache testObject = DnsCache.get(); - - // populate cache - testObject.insert(URL_1, LIST_1); - testObject.insert(URL_2, LIST_2); - testObject.insert(URL_3, LIST_3); - - // check for cached lookup results - List testList = testObject.lookup(URL_1); - assertEquals(3, testList.size()); - assertEquals(IP_1.getHostAddress(), testList.get(0).getHostAddress()); - assertEquals(IP_2.getHostAddress(), testList.get(1).getHostAddress()); - assertEquals(IP_3.getHostAddress(), testList.get(2).getHostAddress()); - - // toggle preference (false) - prefs.setDnsCacheEnabledValue(false); - - // attempt non-cached lookup - testList = testObject.lookup(URL_1); - assertNull(testList); - - // toggle preference (true) - prefs.setDnsCacheEnabledValue(true); - - // confirm lookup results remain in cache - testList = testObject.lookup(URL_2); - assertEquals(1, testList.size()); - assertEquals(IP_2.getHostAddress(), testList.get(0).getHostAddress()); - - // test removal - testList = testObject.remove(URL_2); - assertEquals(1, testList.size()); - assertEquals(IP_2.getHostAddress(), testList.get(0).getHostAddress()); - testList = testObject.lookup(URL_2); - assertNull(testList); - } -} diff --git a/legacy/src/main/java/org/fdroid/fdroid/FDroidApp.java b/legacy/src/main/java/org/fdroid/fdroid/FDroidApp.java index 78f5f8e3c..94215c239 100644 --- a/legacy/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/legacy/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -71,7 +71,6 @@ import org.fdroid.fdroid.nearby.PublicSourceDirProvider; import org.fdroid.fdroid.nearby.SDCardScannerService; import org.fdroid.fdroid.nearby.WifiStateChangeService; import org.fdroid.fdroid.net.ConnectivityMonitorService; -import org.fdroid.fdroid.net.DnsCache; import org.fdroid.fdroid.net.DownloaderFactory; import org.fdroid.fdroid.panic.HidingManager; import org.fdroid.fdroid.receiver.DeviceStorageReceiver; @@ -302,7 +301,6 @@ public class FDroidApp extends Application implements androidx.work.Configuratio .build()); } Preferences.setup(this); - DnsCache.setup(); Languages.setLanguage(this); Preferences preferences = Preferences.get(); diff --git a/legacy/src/main/java/org/fdroid/fdroid/net/DnsCache.java b/legacy/src/main/java/org/fdroid/fdroid/net/DnsCache.java index 6b541c493..3b9739b76 100644 --- a/legacy/src/main/java/org/fdroid/fdroid/net/DnsCache.java +++ b/legacy/src/main/java/org/fdroid/fdroid/net/DnsCache.java @@ -1,7 +1,5 @@ package org.fdroid.fdroid.net; -import android.util.Log; - import androidx.annotation.NonNull; import org.fdroid.fdroid.Preferences; @@ -15,8 +13,6 @@ import java.util.concurrent.TimeUnit; public final class DnsCache { - private static final String TAG = "DnsCache"; - private volatile HashMap> cache; private static final int DELAY_TIME = 1; private static final TimeUnit DELAY_UNIT = TimeUnit.SECONDS; @@ -40,20 +36,9 @@ public final class DnsCache { } } - public static void setup() { - if (instance != null) { - final String error = "DnsCache can only be initialized once"; - Log.e(TAG, error); - throw new RuntimeException(error); - } - instance = new DnsCache(); - } - public static DnsCache get() { if (instance == null) { - final String error = "DnsCache must be initialized first"; - Log.e(TAG, error); - throw new RuntimeException(error); + instance = new DnsCache(); } return instance; } diff --git a/legacy/src/main/java/org/fdroid/fdroid/net/DnsWithCache.java b/legacy/src/main/java/org/fdroid/fdroid/net/DnsWithCache.java index 2ee9eef86..02f3eab54 100644 --- a/legacy/src/main/java/org/fdroid/fdroid/net/DnsWithCache.java +++ b/legacy/src/main/java/org/fdroid/fdroid/net/DnsWithCache.java @@ -3,6 +3,7 @@ package org.fdroid.fdroid.net; import androidx.annotation.NonNull; import org.fdroid.fdroid.Preferences; +import org.jetbrains.annotations.NotNull; import java.net.InetAddress; import java.net.UnknownHostException; @@ -10,7 +11,22 @@ import java.util.List; import okhttp3.Dns; -public class DnsWithCache implements Dns { +public final class DnsWithCache implements Dns { + + private final DnsCache cache; + + private static DnsWithCache instance; + + private DnsWithCache() { + cache = DnsCache.get(); + } + + public static DnsWithCache get() { + if (instance == null) { + instance = new DnsWithCache(); + } + return instance; + } @NonNull @Override @@ -19,7 +35,6 @@ public class DnsWithCache implements Dns { if (!prefs.isDnsCacheEnabled()) { return Dns.SYSTEM.lookup(url); } - DnsCache cache = DnsCache.get(); List list = cache.lookup(url); if (list == null) { list = Dns.SYSTEM.lookup(url); @@ -27,4 +42,39 @@ public class DnsWithCache implements Dns { } return list; } + + /** + * in case a host is unreachable, check whether the cached dns result is different from + * the current result. if the cached result is different, remove that result from the + * cache. returns true if a cached result was removed, indicating that the connection + * should be retried, otherwise returns false. + */ + public boolean shouldRetryRequest(@NotNull String url) { + Preferences prefs = Preferences.get(); + if (!prefs.isDnsCacheEnabled()) { + // the cache feature was not enabled, so a cached result didn't cause the failure + return false; + } + List list = cache.lookup(url); + if (list == null) { + // no cached result was found, so a cached result didn't cause the failure + return false; + } else { + try { + List lookupList = Dns.SYSTEM.lookup(url); + for (InetAddress address : lookupList) { + if (!list.contains(address)) { + // the cached result doesn't match the current dns result, so the connection should be retried + cache.remove(url); + return true; + } + } + // the cached result matches the current dns result, so a cached result didn't cause the failure + return false; + } catch (UnknownHostException e) { + // the url returned an unknown host exception, so there's no point in retrying + return false; + } + } + } } diff --git a/legacy/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java b/legacy/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java index 19f99c36d..606685f33 100644 --- a/legacy/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java +++ b/legacy/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java @@ -37,7 +37,7 @@ public class DownloaderFactory extends org.fdroid.download.DownloaderFactory { Utils.getUserAgent(), FDroidApp.queryString, NetCipher.getProxy(), - new DnsWithCache(), + DnsWithCache.get(), new FDroidMirrorParameterManager()); @NonNull diff --git a/legacy/src/main/java/org/fdroid/fdroid/net/FDroidMirrorParameterManager.java b/legacy/src/main/java/org/fdroid/fdroid/net/FDroidMirrorParameterManager.java index 6c111f59b..eb60f2ee2 100644 --- a/legacy/src/main/java/org/fdroid/fdroid/net/FDroidMirrorParameterManager.java +++ b/legacy/src/main/java/org/fdroid/fdroid/net/FDroidMirrorParameterManager.java @@ -10,6 +10,7 @@ import org.fdroid.download.MirrorParameterManager; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.data.App; +import org.jetbrains.annotations.NotNull; import java.util.Collections; import java.util.HashMap; @@ -66,6 +67,15 @@ public class FDroidMirrorParameterManager implements MirrorParameterManager { } } + /** + * Returns true or false depending on whether a particular mirror should be retried before + * moving on to the next one (based on checking dns results) + */ + @Override + public boolean shouldRetryRequest(@NotNull String mirrorUrl) { + return DnsWithCache.get().shouldRetryRequest(mirrorUrl); + } + @Override public boolean preferForeignMirrors() { Preferences prefs = Preferences.get(); diff --git a/libs/download/api/android/download.api b/libs/download/api/android/download.api index 2472e03c5..025ca8d30 100644 --- a/libs/download/api/android/download.api +++ b/libs/download/api/android/download.api @@ -163,6 +163,7 @@ public abstract interface class org/fdroid/download/MirrorParameterManager { public abstract fun getMirrorErrorCount (Ljava/lang/String;)I public abstract fun incrementMirrorErrorCount (Ljava/lang/String;)V public abstract fun preferForeignMirrors ()Z + public abstract fun shouldRetryRequest (Ljava/lang/String;)Z } public final class org/fdroid/download/NoResumeException : java/lang/Exception { diff --git a/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt b/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt index 1a48faf27..beba71c0b 100644 --- a/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt +++ b/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt @@ -59,7 +59,7 @@ internal abstract class MirrorChooserImpl : MirrorChooser { mirror.getUrl(downloadRequest.indexFile.name) } try { - return request(mirror, url) + return executeRequest(mirror, url, request) } catch (e: ResponseException) { // don't try other mirrors if we got Forbidden response, but supplied credentials if (downloadRequest.hasCredentials && e.response.status == Forbidden) throw e @@ -77,6 +77,14 @@ internal abstract class MirrorChooserImpl : MirrorChooser { error("Reached code that was thought to be unreachable.") } + open suspend fun executeRequest( + mirror: Mirror, + url: Url, + request: suspend (mirror: Mirror, url: Url) -> T, + ): T { + return request(mirror, url) + } + open fun handleException(e: Exception, mirror: Mirror, mirrorIndex: Int, mirrorCount: Int) { val wasLastMirror = mirrorIndex == mirrorCount - 1 log.info { @@ -186,10 +194,28 @@ internal class MirrorChooserWithParameters( mirrorList.addAll(unknownList) mirrorList.addAll(foreignList) } - return mirrorList } + override suspend fun executeRequest( + mirror: Mirror, + url: Url, + request: suspend (mirror: Mirror, url: Url) -> T, + ): T { + return try { + request(mirror, url) + } catch (e: Exception) { + // in case of an exception, potentially attempt a single retry + if (mirrorParameterManager != null && + mirrorParameterManager.shouldRetryRequest(url.host) + ) { + request(mirror, url) + } else { + throw e + } + } + } + override fun handleException(e: Exception, mirror: Mirror, mirrorIndex: Int, mirrorCount: Int) { if (e is ResponseException || e is IOException) { mirrorParameterManager?.incrementMirrorErrorCount(mirror.baseUrl) diff --git a/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorParameterManager.kt b/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorParameterManager.kt index a1a7fe75a..e1d27db55 100644 --- a/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorParameterManager.kt +++ b/libs/download/src/commonMain/kotlin/org/fdroid/download/MirrorParameterManager.kt @@ -18,6 +18,12 @@ public interface MirrorParameterManager { public fun getMirrorErrorCount(mirrorUrl: String): Int + /** + * Returns true or false depending on whether a particular mirror should be retried before + * moving on to the next one (typically based on checking dns results) + */ + public fun shouldRetryRequest(mirrorUrl: String): Boolean + /** * Returns true or false depending on whether the location preference has been enabled. This * preference reflects whether mirrors matching your location should get priority.