From 2735bee8437addd39af482397fa7a273160f6940 Mon Sep 17 00:00:00 2001 From: hussainmohd-a <56958445+hussainmohd-a@users.noreply.github.com> Date: Thu, 18 Nov 2021 02:03:14 +0530 Subject: [PATCH] v53h: Delete leftover local blocklists (#414) Leftover local blocklist files are not cleared from the app's canonical-path which results in increase in app's data-dir. Worse, user have no way to delete these. The leftover files, if any, are deleted on every download run, with only the current local blocklist files retained. Leftovers are also handled for remote blocklist files. Also: When files from canonical path go missing also reset the current blocklist timestamp value to ensure the user is prompted to re-download the files. Also: Show cannot-download dialog when vpn is in lockdown mode and user attempts a blocklist download. --- app/build.gradle | 4 +- .../bravedns/download/AppDownloadManager.kt | 2 +- .../download/BlocklistDownloadHelper.kt | 7 +-- .../bravedns/download/FileHandleWorker.kt | 10 ++++ .../celzero/bravedns/net/go/GoVpnAdapter.kt | 7 ++- .../ui/DNSConfigureWebViewActivity.kt | 5 +- .../celzero/bravedns/ui/HomeScreenActivity.kt | 18 ++++--- .../celzero/bravedns/ui/SettingsFragment.kt | 26 +++++++--- .../com/celzero/bravedns/util/Utilities.kt | 52 ++++++++++++++++--- 9 files changed, 103 insertions(+), 28 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 3fd5a2dc2..10dfb30f0 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -31,8 +31,8 @@ android { applicationId "com.celzero.bravedns" minSdkVersion 23 targetSdkVersion 30 - versionCode 18 // For version name 053g - versionName "053g" + versionCode 19 // For version name 053h + versionName "053h" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } diff --git a/app/src/main/java/com/celzero/bravedns/download/AppDownloadManager.kt b/app/src/main/java/com/celzero/bravedns/download/AppDownloadManager.kt index 1c3d31bd2..6bb3ed98e 100644 --- a/app/src/main/java/com/celzero/bravedns/download/AppDownloadManager.kt +++ b/app/src/main/java/com/celzero/bravedns/download/AppDownloadManager.kt @@ -107,7 +107,7 @@ class AppDownloadManager(private val context: Context) { */ private fun purge(context: Context, timestamp: Long) { downloadIds = LongArray(ONDEVICE_BLOCKLISTS.count()) - BlocklistDownloadHelper.deleteOldFiles(context, timestamp) + BlocklistDownloadHelper.deleteFromExternalDir(context, timestamp) } private fun enqueueDownload(url: String, fileName: String, timestamp: String): Long { diff --git a/app/src/main/java/com/celzero/bravedns/download/BlocklistDownloadHelper.kt b/app/src/main/java/com/celzero/bravedns/download/BlocklistDownloadHelper.kt index d6969e292..cbb262812 100644 --- a/app/src/main/java/com/celzero/bravedns/download/BlocklistDownloadHelper.kt +++ b/app/src/main/java/com/celzero/bravedns/download/BlocklistDownloadHelper.kt @@ -20,6 +20,7 @@ import android.util.Log import com.celzero.bravedns.ui.HomeScreenActivity.GlobalVariable.DEBUG import com.celzero.bravedns.util.Constants import com.celzero.bravedns.util.LoggerConstants.Companion.LOG_TAG_DOWNLOAD +import com.celzero.bravedns.util.Utilities.Companion.cleanupOldLocalBlocklistFiles import com.celzero.bravedns.util.Utilities.Companion.deleteRecursive import java.io.File @@ -55,7 +56,7 @@ class BlocklistDownloadHelper { * Now in v053 we are moving the files from external dir to canonical path. * So deleting the old files in the external directory. */ - fun deleteOldFiles(context: Context, timestamp: Long) { + fun deleteFromExternalDir(context: Context, timestamp: Long) { val dir = File(context.getExternalFilesDir( null).toString() + Constants.ONDEVICE_BLOCKLIST_DOWNLOAD_PATH + timestamp) if (DEBUG) Log.d(LOG_TAG_DOWNLOAD, @@ -64,8 +65,8 @@ class BlocklistDownloadHelper { } fun deleteFromCanonicalPath(context: Context, timestamp: Long) { - val canonicalPath = File("${context.filesDir.canonicalPath}${File.separator}$timestamp") - deleteRecursive(canonicalPath) + val canonicalPath = File(context.filesDir.canonicalPath + File.separator) + cleanupOldLocalBlocklistFiles(canonicalPath, timestamp.toString()) } fun getExternalFilePath(context: Context, timestamp: String): String { diff --git a/app/src/main/java/com/celzero/bravedns/download/FileHandleWorker.kt b/app/src/main/java/com/celzero/bravedns/download/FileHandleWorker.kt index 3effb1491..901813746 100644 --- a/app/src/main/java/com/celzero/bravedns/download/FileHandleWorker.kt +++ b/app/src/main/java/com/celzero/bravedns/download/FileHandleWorker.kt @@ -20,6 +20,7 @@ import android.util.Log import androidx.work.Worker import androidx.work.WorkerParameters import androidx.work.workDataOf +import com.celzero.bravedns.download.BlocklistDownloadHelper.Companion.deleteFromExternalDir import com.celzero.bravedns.service.PersistentState import com.celzero.bravedns.ui.HomeScreenActivity.GlobalVariable.DEBUG import com.celzero.bravedns.util.Constants @@ -96,9 +97,17 @@ class FileHandleWorker(val context: Context, workerParameters: WorkerParameters) return false } + // In case of failure, all files in + // 1. "from" dir will be deleted by deleteFromExternalDir + // 2. "to" dir will be delete by deleteFromCanonicalPath + // during the next download downloadLocalBlocklist for (i in children.indices) { val from = dir.absolutePath + File.separator + children[i] val to = localBlocklistDownloadPath(context, children[i], timestamp) + if (to == null) { + Log.w(LOG_TAG_DOWNLOAD, "Copy failed from $from, to: $to") + return false + } val result = Utilities.copy(from, to) if (!result) { @@ -117,6 +126,7 @@ class FileHandleWorker(val context: Context, workerParameters: WorkerParameters) } updatePersistenceOnCopySuccess(timestamp) + deleteFromExternalDir(context, timestamp) return true } catch (e: Exception) { diff --git a/app/src/main/java/com/celzero/bravedns/net/go/GoVpnAdapter.kt b/app/src/main/java/com/celzero/bravedns/net/go/GoVpnAdapter.kt index 5d74f7694..43e7d7735 100644 --- a/app/src/main/java/com/celzero/bravedns/net/go/GoVpnAdapter.kt +++ b/app/src/main/java/com/celzero/bravedns/net/go/GoVpnAdapter.kt @@ -446,8 +446,13 @@ class GoVpnAdapter(private val context: Context, private val externalScope: Coro path + ONDEVICE_BLOCKLIST_FILE_TAG) } catch (e: Exception) { Log.e(LOG_TAG_VPN, "Local brave dns set exception :${e.message}", e) - // Set local blocklist enabled to false if there is a failure creating bravedns + // Set local blocklist enabled to false and reset the timestamp + // if there is a failure creating bravedns persistentState.blocklistEnabled = false + // this exception may occur when the local blocklist files are either missing + // or corrupted. Reset local blocklist timestamp to make sure + // user is prompted to download blocklists again on the next try + persistentState.localBlocklistTimestamp = 0 null } } diff --git a/app/src/main/java/com/celzero/bravedns/ui/DNSConfigureWebViewActivity.kt b/app/src/main/java/com/celzero/bravedns/ui/DNSConfigureWebViewActivity.kt index 173531b95..f3d988e96 100644 --- a/app/src/main/java/com/celzero/bravedns/ui/DNSConfigureWebViewActivity.kt +++ b/app/src/main/java/com/celzero/bravedns/ui/DNSConfigureWebViewActivity.kt @@ -50,6 +50,8 @@ import com.celzero.bravedns.util.LoggerConstants.Companion.LOG_TAG_VPN import com.celzero.bravedns.util.Themes import com.celzero.bravedns.util.Themes.Companion.getCurrentTheme import com.celzero.bravedns.util.Utilities +import com.celzero.bravedns.util.Utilities.Companion.cleanupRemoteBlocklistDir +import com.celzero.bravedns.util.Utilities.Companion.hasRemoteBlocklistFile import com.celzero.bravedns.util.Utilities.Companion.isAtleastO import com.celzero.bravedns.util.Utilities.Companion.remoteBlocklistDir import kotlinx.coroutines.Dispatchers @@ -425,9 +427,10 @@ class DNSConfigureWebViewActivity : AppCompatActivity(R.layout.activity_faq_webv val t = parseLong(timestamp) - if (persistentState.remoteBlocklistTimestamp >= t) return + if (hasRemoteBlocklistFile(applicationContext, BLOCKLIST_REMOTE_FOLDER_NAME, t)) return try { + cleanupRemoteBlocklistDir(applicationContext, BLOCKLIST_REMOTE_FOLDER_NAME) val filetag = makeFile(t) ?: return filetag.writeText(fileContent) diff --git a/app/src/main/java/com/celzero/bravedns/ui/HomeScreenActivity.kt b/app/src/main/java/com/celzero/bravedns/ui/HomeScreenActivity.kt index c832f257a..888e389f7 100644 --- a/app/src/main/java/com/celzero/bravedns/ui/HomeScreenActivity.kt +++ b/app/src/main/java/com/celzero/bravedns/ui/HomeScreenActivity.kt @@ -44,6 +44,8 @@ import com.celzero.bravedns.util.LoggerConstants.Companion.LOG_TAG_APP_UPDATE import com.celzero.bravedns.util.LoggerConstants.Companion.LOG_TAG_DOWNLOAD import com.celzero.bravedns.util.LoggerConstants.Companion.LOG_TAG_UI import com.celzero.bravedns.util.Themes.Companion.getCurrentTheme +import com.celzero.bravedns.util.Utilities.Companion.cleanupOldLocalBlocklistFiles +import com.celzero.bravedns.util.Utilities.Companion.deleteRecursive import com.celzero.bravedns.util.Utilities.Companion.getPackageMetadata import com.celzero.bravedns.util.Utilities.Companion.isPlayStoreFlavour import com.celzero.bravedns.util.Utilities.Companion.isWebsiteFlavour @@ -52,6 +54,7 @@ import kotlinx.coroutines.* import okhttp3.* import org.koin.android.ext.android.get import org.koin.android.ext.android.inject +import java.io.File import java.util.* import java.util.concurrent.TimeUnit @@ -125,10 +128,12 @@ class HomeScreenActivity : AppCompatActivity(R.layout.activity_home_screen) { } private fun removeThisMethod() { - persistentState.numberOfRequests = persistentState.oldNumberRequests.toLong() - persistentState.numberOfBlockedRequests = persistentState.oldBlockedRequests.toLong() io { - refreshDatabase.refreshAppInfoDatabase() + val canonicalDir = File(this.filesDir.canonicalPath + File.separator) + cleanupOldLocalBlocklistFiles(canonicalDir, persistentState.localBlocklistTimestamp.toString()) + // clean up files in external files dir + val externalDir = File(this.getExternalFilesDir(null).toString() + Constants.ONDEVICE_BLOCKLIST_DOWNLOAD_PATH) + deleteRecursive(externalDir) } } @@ -162,9 +167,10 @@ class HomeScreenActivity : AppCompatActivity(R.layout.activity_home_screen) { persistentState.appVersion = version persistentState.showWhatsNewChip = true - // FIXME: remove this post v053g - // this is to fix the persistence state which was saved as Int instead of Long. - // Modification of persistence state + // FIXME: remove this post v054 + // this is to fix the pile up of local blocklist folders in app's canonical path + // deletes all the folder names starting with "16" other than current local blocklist + // timestamp removeThisMethod() } diff --git a/app/src/main/java/com/celzero/bravedns/ui/SettingsFragment.kt b/app/src/main/java/com/celzero/bravedns/ui/SettingsFragment.kt index 6c3cb262b..86271f73a 100644 --- a/app/src/main/java/com/celzero/bravedns/ui/SettingsFragment.kt +++ b/app/src/main/java/com/celzero/bravedns/ui/SettingsFragment.kt @@ -414,11 +414,9 @@ class SettingsFragment : Fragment(R.layout.activity_settings_screen) { persistentState.numberOfLocalBlocklists.toString()) } else { b.settingsActivityOnDeviceBlockSwitch.isChecked = false - if (VpnController.isVpnLockdown()) { - showVpnLockdownDownloadDialog() - } else { + maybeDownloadLocalBlocklist(yes = { showDownloadDialog() - } + }) } } } @@ -436,7 +434,9 @@ class SettingsFragment : Fragment(R.layout.activity_settings_screen) { } b.settingsActivityOnDeviceBlockRefreshBtn.setOnClickListener { - updateBlocklistIfNeeded(isRefresh = true) + maybeDownloadLocalBlocklist(yes = { + updateBlocklistIfNeeded(isRefresh = true) + }) } val workManager = WorkManager.getInstance(requireContext().applicationContext) @@ -474,6 +474,20 @@ class SettingsFragment : Fragment(R.layout.activity_settings_screen) { }) } + // As of now, the android download manager is used to download blocklist files. + // When VPN is in lockdown mode, downloads are not succeeding. As an interim fix, + // prompt dialog to the user about lockdown mode. No need for the below case if + // local blocklists download is carried away by the in-built download manager. + private fun maybeDownloadLocalBlocklist(yes: () -> Unit): Boolean { + return if (VpnController.isVpnLockdown()) { + showVpnLockdownDownloadDialog() + false + } else { + yes() + true + } + } + private fun refreshOnDeviceBlocklistUi() { if (isPlayStoreFlavour()) { // hide the parent view b.settingsActivityOnDeviceBlockRl.visibility = View.GONE @@ -554,7 +568,7 @@ class SettingsFragment : Fragment(R.layout.activity_settings_screen) { val json = JSONObject(stringResponse) val version = json.optInt(Constants.JSON_VERSION, 0) if (DEBUG) Log.d(LOG_TAG_DOWNLOAD, - "client onResponse for refresh blocklist files- $version") + "client onResponse for refresh blocklist files: $version") if (version != Constants.UPDATE_CHECK_RESPONSE_VERSION) { return } diff --git a/app/src/main/java/com/celzero/bravedns/util/Utilities.kt b/app/src/main/java/com/celzero/bravedns/util/Utilities.kt index b4b2835e5..78bded8a8 100644 --- a/app/src/main/java/com/celzero/bravedns/util/Utilities.kt +++ b/app/src/main/java/com/celzero/bravedns/util/Utilities.kt @@ -504,40 +504,76 @@ class Utilities { } } - fun localBlocklistDownloadPath(ctx: Context, which: String, timestamp: Long): String { + fun localBlocklistDownloadPath(ctx: Context?, which: String, timestamp: Long): String? { + ctx ?: return null + return ctx.filesDir.canonicalPath + File.separator + timestamp + File.separator + which } - fun hasLocalBlocklists(ctx: Context, timestamp: Long): Boolean { + fun cleanupOldLocalBlocklistFiles(fileOrDir: File, activeDir: String) { + if (fileOrDir.name.equals(activeDir)) return + + // local blocklist dirs are named with timestamps, ex: 1635754946983 + // Dirs other than activeDir (current timestamp) + // should be deleted (path: ../files/). + // delete both files and dirs starting with "16". + if (fileOrDir.name.startsWith("16")) { + deleteRecursive(fileOrDir) + return + } + + if (fileOrDir.isDirectory) { + fileOrDir.listFiles()?.forEach { child -> + cleanupOldLocalBlocklistFiles(child, activeDir) + } + } // ignore files that don't start with "16" + } + + fun hasLocalBlocklists(ctx: Context?, timestamp: Long): Boolean { val a = Constants.ONDEVICE_BLOCKLISTS.all { localBlocklistFile(ctx, it.filename, timestamp)?.exists() == true } return a } - private fun localBlocklistFile(ctx: Context, which: String, timestamp: Long): File? { + private fun localBlocklistFile(ctx: Context?, which: String, timestamp: Long): File? { return try { - return File(localBlocklistDownloadPath(ctx, which, timestamp)) + val localBlocklist = localBlocklistDownloadPath(ctx, which, timestamp) ?: return null + + return File(localBlocklist) } catch (e: IOException) { Log.e(LOG_TAG_VPN, "Could not fetch remote blocklist: " + e.message, e) null } } + fun hasRemoteBlocklistFile(ctx: Context?, which: String, timestamp: Long): Boolean { + val remoteDir = remoteBlocklistDir(ctx, which, timestamp) ?: return false + val remoteFile = remoteBlocklistFile(remoteDir.absolutePath, + Constants.ONDEVICE_BLOCKLIST_FILE_TAG) ?: return false + return remoteFile.exists() + } + + fun cleanupRemoteBlocklistDir(ctx: Context?, which: String) { + ctx ?: return + + val dir = File(remoteBlocklistDownloadBasePath(ctx, which)) + deleteRecursive(dir) + } + fun remoteBlocklistDir(ctx: Context?, which: String, timestamp: Long): File? { if (ctx == null) return null return try { - File(remoteBlocklistDownloadBasePath(ctx, which, timestamp)) + File(remoteBlocklistDownloadBasePath(ctx, which) + File.separator + timestamp) } catch (e: IOException) { Log.e(LOG_TAG_VPN, "Could not fetch remote blocklist: " + e.message, e) null } } - private fun remoteBlocklistDownloadBasePath(ctx: Context, which: String, - timestamp: Long): String { - return ctx.filesDir.canonicalPath + File.separator + which + File.separator + timestamp + private fun remoteBlocklistDownloadBasePath(ctx: Context, which: String): String { + return ctx.filesDir.canonicalPath + File.separator + which } fun remoteBlocklistFile(dirPath: String, fileName: String): File? {