From ab6c5f4c50977357fb2e1113691d0a6207b62035 Mon Sep 17 00:00:00 2001
From: lynxnb <niccolo.betto@gmail.com>
Date: Wed, 9 Nov 2022 02:07:18 +0100
Subject: [PATCH] Improve robustness of `KeyReader.import`

* Close the input and output file streams before moving the output file to the final destination
* Clean up the destination path before moving the new file
* Introduce a `ImportResult` return value to differentiate between the possible causes of import errors
* Display more meaningful error messages in the UI
---
 app/src/main/java/emu/skyline/KeyReader.kt    | 46 ++++++++++++++++---
 .../skyline/preference/KeyPickerPreference.kt | 12 ++++-
 app/src/main/res/values/strings.xml           |  5 +-
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/app/src/main/java/emu/skyline/KeyReader.kt b/app/src/main/java/emu/skyline/KeyReader.kt
index e881fdce..ef900e83 100644
--- a/app/src/main/java/emu/skyline/KeyReader.kt
+++ b/app/src/main/java/emu/skyline/KeyReader.kt
@@ -14,6 +14,14 @@ import java.io.File
 object KeyReader {
     private val Tag = KeyReader::class.java.simpleName
 
+    enum class ImportResult {
+        Success,
+        InvalidInputPath,
+        InvalidKeys,
+        DeletePreviousFailed,
+        MoveFailed,
+    }
+
     enum class KeyType(val keyName : String, val fileName : String) {
         Title("title_keys", "title.keys"), Prod("prod_keys", "prod.keys");
 
@@ -39,20 +47,23 @@ object KeyReader {
     /**
      * Reads keys file, trims and writes to internal app data storage, it makes sure file is properly formatted
      */
-    fun import(context : Context, uri : Uri, keyType : KeyType) : Boolean {
+    fun import(context : Context, uri : Uri, keyType : KeyType) : ImportResult {
         Log.i(Tag, "Parsing ${keyType.name} $uri")
 
         if (!DocumentFile.isDocumentUri(context, uri))
-            return false
+            return ImportResult.InvalidInputPath
 
         val outputDirectory = File("${context.filesDir.canonicalFile}/keys/")
         if (!outputDirectory.exists())
             outputDirectory.mkdirs()
-        val tmpOutputFile = File(outputDirectory, "${keyType.fileName}.tmp")
+
+        val outputFile = File(outputDirectory, keyType.fileName)
+        val tmpOutputFile = File("${outputFile}.tmp")
+        var valid = false
 
         context.contentResolver.openInputStream(uri).use { inputStream ->
             tmpOutputFile.bufferedWriter().use { writer ->
-                val valid = inputStream!!.bufferedReader().useLines {
+                valid = inputStream!!.bufferedReader().useLines {
                     for (line in it) {
                         if (line.startsWith(";") || line.isBlank()) continue
 
@@ -81,11 +92,32 @@ object KeyReader {
                     }
                     true
                 }
-
-                if (valid) tmpOutputFile.renameTo(File("${tmpOutputFile.parent}/${keyType.fileName}"))
-                return valid
             }
         }
+
+        val cleanup = {
+            try {
+                tmpOutputFile.delete()
+            } catch (_ : Exception) {
+            }
+        }
+
+        if (!valid) {
+            cleanup()
+            return ImportResult.InvalidKeys
+        }
+
+        if (outputFile.exists() && !outputFile.delete()) {
+            cleanup()
+            return ImportResult.DeletePreviousFailed
+        }
+
+        if (!tmpOutputFile.renameTo(outputFile)) {
+            cleanup()
+            return ImportResult.MoveFailed
+        }
+
+        return ImportResult.Success
     }
 
     private fun isHexString(str : String) : Boolean {
diff --git a/app/src/main/java/emu/skyline/preference/KeyPickerPreference.kt b/app/src/main/java/emu/skyline/preference/KeyPickerPreference.kt
index 778b366b..79529e6a 100644
--- a/app/src/main/java/emu/skyline/preference/KeyPickerPreference.kt
+++ b/app/src/main/java/emu/skyline/preference/KeyPickerPreference.kt
@@ -24,10 +24,18 @@ class KeyPickerPreference @JvmOverloads constructor(context : Context, attrs : A
 
             context.getSettings().refreshRequired = true
 
-            val success = KeyReader.import(context, uri, KeyReader.KeyType.parse(key))
-            Snackbar.make((context as SettingsActivity).binding.root, if (success) R.string.import_keys_success else R.string.import_keys_failed, Snackbar.LENGTH_LONG).show()
+            val result = KeyReader.import(context, uri, KeyReader.KeyType.parse(key))
+            Snackbar.make((context as SettingsActivity).binding.root, resolveImportResultString(result), Snackbar.LENGTH_LONG).show()
         }
     }
 
     override fun onClick() = documentPicker.launch(arrayOf("*/*"))
+
+    private fun resolveImportResultString(result : KeyReader.ImportResult) = when (result) {
+        KeyReader.ImportResult.Success -> R.string.import_keys_success
+        KeyReader.ImportResult.InvalidInputPath -> R.string.import_keys_invalid_input_path
+        KeyReader.ImportResult.InvalidKeys -> R.string.import_keys_invalid_keys
+        KeyReader.ImportResult.DeletePreviousFailed -> R.string.import_keys_delete_previous_failed
+        KeyReader.ImportResult.MoveFailed -> R.string.import_keys_move_failed
+    }
 }
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index f6c59961..b20fde9f 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -53,7 +53,10 @@
     <string name="prod_keys">Production Keys</string>
     <string name="title_keys">Title Keys</string>
     <string name="import_keys_success">Successfully imported keys</string>
-    <string name="import_keys_failed">Failed to import keys</string>
+    <string name="import_keys_invalid_input_path">The path to the provided keys is invalid</string>
+    <string name="import_keys_invalid_keys">The keys you tried to import are invalid</string>
+    <string name="import_keys_delete_previous_failed">Failed to delete the currently installed keys</string>
+    <string name="import_keys_move_failed">Failed to move the keys to the internal directory</string>
     <!-- Settings - Display -->
     <string name="display">Display</string>
     <string name="screen_orientation">Screen orientation</string>