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

@@ -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"
     }
 

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 {

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 {

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) {

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
         }
     }

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)

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()
     }
 

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
                 }

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/<timestamp>).
+            // 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? {