Merge pull request #6404 from grzesiek2010/COLLECT-6364

grzesiek2010 · web-flow · commit cff7a1e9e2c5 · 2024-10-01T22:34:51.000+02:00
Implemented #sanitizedCanonicalPath method
diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/utils/PathUtils.kt b/androidshared/src/main/java/org/odk/collect/androidshared/utils/PathUtils.kt
@@ -1,5 +1,6 @@
 package org.odk.collect.androidshared.utils
 
+import org.odk.collect.shared.files.FileExt.sanitizedCanonicalPath
 import timber.log.Timber
 import java.io.File
 
@@ -9,8 +10,8 @@ object PathUtils {
         val absoluteFilePath =
             if (filePath.startsWith(dirPath)) filePath else dirPath + File.separator + filePath
 
-        val canonicalAbsoluteFilePath = File(absoluteFilePath).canonicalPath
-        val canonicalDirPath = File(dirPath).canonicalPath
+        val canonicalAbsoluteFilePath = File(absoluteFilePath).sanitizedCanonicalPath()
+        val canonicalDirPath = File(dirPath).sanitizedCanonicalPath()
         if (!canonicalAbsoluteFilePath.startsWith(canonicalDirPath)) {
             Timber.e(
                 "Attempt to access file outside of Collect directory:\n" +
diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/support/rules/ResetStateRule.kt b/collect_app/src/androidTest/java/org/odk/collect/android/support/rules/ResetStateRule.kt
@@ -14,7 +14,6 @@ import org.odk.collect.androidshared.ui.ToastUtils
 import org.odk.collect.androidshared.ui.multiclicksafe.MultiClickGuard
 import org.odk.collect.db.sqlite.DatabaseConnection
 import org.odk.collect.material.BottomSheetBehavior
-import org.odk.collect.shared.files.DirectoryUtils
 import java.io.IOException
 
 private class ResetStateStatement(
@@ -43,11 +42,11 @@ private class ResetStateStatement(
     private fun clearDisk() {
         try {
             val internalFilesDir = ApplicationProvider.getApplicationContext<Application>().filesDir
-            DirectoryUtils.deleteDirectory(internalFilesDir)
+            internalFilesDir.deleteRecursively()
 
             val externalFilesDir =
                 ApplicationProvider.getApplicationContext<Application>().getExternalFilesDir(null)!!
-            DirectoryUtils.deleteDirectory(externalFilesDir)
+            externalFilesDir.deleteRecursively()
         } catch (e: IOException) {
             throw RuntimeException(e)
         }
diff --git a/collect_app/src/main/java/org/odk/collect/android/application/initialization/ExistingProjectMigrator.kt b/collect_app/src/main/java/org/odk/collect/android/application/initialization/ExistingProjectMigrator.kt
@@ -10,7 +10,6 @@ import org.odk.collect.settings.SettingsProvider
 import org.odk.collect.settings.importing.ProjectDetailsCreator
 import org.odk.collect.settings.keys.MetaKeys
 import org.odk.collect.settings.keys.ProjectKeys
-import org.odk.collect.shared.files.DirectoryUtils
 import org.odk.collect.upgrade.Upgrade
 import java.io.File
 import java.io.FileNotFoundException
@@ -67,7 +66,7 @@ class ExistingProjectMigrator(
         }
 
         try {
-            DirectoryUtils.deleteDirectory(File(rootDir, ".cache"))
+            File(rootDir, ".cache").deleteRecursively()
         } catch (e: Exception) {
             // ignore
         }
diff --git a/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java b/collect_app/src/main/java/org/odk/collect/android/database/forms/DatabaseFormsRepository.java
@@ -30,7 +30,7 @@
 import org.odk.collect.forms.Form;
 import org.odk.collect.forms.FormsRepository;
 import org.odk.collect.forms.savepoints.SavepointsRepository;
-import org.odk.collect.shared.files.DirectoryUtils;
+import org.odk.collect.shared.files.FileExt;
 import org.odk.collect.shared.strings.Md5;
 
 import java.io.File;
@@ -302,7 +302,7 @@ private void deleteFilesForForm(Form form) {
             File mediaDir = new File(form.getFormMediaPath());
 
             if (mediaDir.isDirectory()) {
-                DirectoryUtils.deleteDirectory(mediaDir);
+                FileExt.deleteDirectory(mediaDir);
             } else {
                 mediaDir.delete();
             }
diff --git a/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java b/collect_app/src/main/java/org/odk/collect/android/database/instances/DatabaseInstancesRepository.java
@@ -11,7 +11,7 @@
 import org.odk.collect.android.database.DatabaseConstants;
 import org.odk.collect.forms.instances.Instance;
 import org.odk.collect.forms.instances.InstancesRepository;
-import org.odk.collect.shared.files.DirectoryUtils;
+import org.odk.collect.shared.files.FileExt;
 
 import java.io.File;
 import java.util.ArrayList;
@@ -270,7 +270,7 @@ private void update(Long instanceId, ContentValues values) {
     }
 
     private void deleteInstanceFiles(Instance instance) {
-        DirectoryUtils.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());
+        FileExt.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());
     }
 
     private static List<Instance> getInstancesFromCursor(Cursor cursor, String instancesPath) {
diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java b/collect_app/src/main/java/org/odk/collect/android/formmanagement/download/ServerFormDownloader.java
@@ -16,7 +16,7 @@
 import org.odk.collect.forms.FormSource;
 import org.odk.collect.forms.FormSourceException;
 import org.odk.collect.forms.FormsRepository;
-import org.odk.collect.shared.files.DirectoryUtils;
+import org.odk.collect.shared.files.FileExt;
 import org.odk.collect.shared.strings.Md5;
 
 import java.io.File;
@@ -78,7 +78,7 @@ public void downloadForm(ServerFormDetails form, @Nullable ProgressReporter prog
         } catch (FormSourceException e) {
             throw new FormDownloadException.FormSourceError(e);
         } finally {
-            DirectoryUtils.deleteDirectory(tempDir);
+            FileExt.deleteDirectory(tempDir);
             for (Form formToDelete : preExistingFormsWithSameIdAndVersion) {
                 formsRepository.delete(formToDelete.getDbId());
             }
diff --git a/collect_app/src/main/java/org/odk/collect/android/savepoints/SavepointTask.kt b/collect_app/src/main/java/org/odk/collect/android/savepoints/SavepointTask.kt
@@ -5,7 +5,7 @@ import org.odk.collect.async.Scheduler
 import org.odk.collect.async.SchedulerAsyncTaskMimic
 import org.odk.collect.forms.savepoints.Savepoint
 import org.odk.collect.forms.savepoints.SavepointsRepository
-import org.odk.collect.shared.files.FileUtils
+import org.odk.collect.shared.files.FileExt.saveToFile
 import timber.log.Timber
 import java.io.File
 
@@ -32,7 +32,7 @@ class SavepointTask(
             val savepoint = Savepoint(formDbId, instanceDbId, savepointFile.absolutePath, formController.getInstanceFile()!!.absolutePath)
 
             if (priority == lastPriorityUsed) {
-                FileUtils.saveToFile(formController.getFilledInFormXml().payloadStream, savepointFile.absolutePath)
+                savepointFile.saveToFile(formController.getFilledInFormXml().payloadStream)
                 savepointsRepository.save(savepoint)
             }
 
diff --git a/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java b/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java
@@ -60,6 +60,7 @@
 import org.odk.collect.forms.Form;
 import org.odk.collect.forms.instances.Instance;
 import org.odk.collect.forms.instances.InstancesRepository;
+import org.odk.collect.shared.files.FileExt;
 
 import java.io.File;
 import java.io.IOException;
@@ -333,19 +334,17 @@ private Instance exportData(boolean markCompleted, FormSaver.ProgressListener pr
 
         ByteArrayPayload payload = formController.getFilledInFormXml();
         // write out xml
-        String instancePath = formController.getInstanceFile().getAbsolutePath();
-
         for (String fileName : tempFiles) {
             mediaUtils.deleteMediaFile(fileName);
         }
 
         progressListener.onProgressUpdate(getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.survey_saving_saving_message));
 
-        writeFile(payload, instancePath);
+        writeFile(payload, formController.getInstanceFile());
 
         // Write last-saved instance
         String lastSavedPath = formController.getLastSavedPath();
-        writeFile(payload, lastSavedPath);
+        writeFile(payload, new File(lastSavedPath));
 
         // update the uri. We have exported the reloadable instance, so update status...
         // Since we saved a reloadable instance, it is flagged as re-openable so that if any error
@@ -373,7 +372,7 @@ private Instance exportData(boolean markCompleted, FormSaver.ProgressListener pr
             progressListener.onProgressUpdate(
                     getLocalizedString(Collect.getInstance(), org.odk.collect.strings.R.string.survey_saving_finalizing_message));
 
-            writeFile(payload, submissionXml.getAbsolutePath());
+            writeFile(payload, submissionXml);
 
             // see if the form is encrypted and we can encrypt it...
             EncryptedFormInformation formInfo = EncryptionUtils.getEncryptedFormInformation(uri, formController.getSubmissionMetadata());
@@ -486,7 +485,7 @@ public static void manageFilesAfterSavingEncryptedForm(File instanceXml, File su
     /**
      * Writes payload contents to the disk.
      */
-    public static void writeFile(ByteArrayPayload payload, String path) throws IOException {
-        org.odk.collect.shared.files.FileUtils.saveToFile(payload.getPayloadStream(), path);
+    public static void writeFile(ByteArrayPayload payload, File file) throws IOException {
+        FileExt.saveToFile(file, payload.getPayloadStream());
     }
 }
diff --git a/forms-test/src/main/java/org/odk/collect/formstest/InMemFormsRepository.java b/forms-test/src/main/java/org/odk/collect/formstest/InMemFormsRepository.java
@@ -5,7 +5,7 @@
 import org.odk.collect.forms.Form;
 import org.odk.collect.forms.FormsRepository;
 import org.odk.collect.forms.savepoints.SavepointsRepository;
-import org.odk.collect.shared.files.DirectoryUtils;
+import org.odk.collect.shared.files.FileExt;
 import org.odk.collect.shared.strings.Md5;
 import org.odk.collect.shared.TempFiles;
 
@@ -216,7 +216,7 @@ private void deleteFilesForForm(Form form) {
             File mediaDir = new File(form.getFormMediaPath());
 
             if (mediaDir.isDirectory()) {
-                DirectoryUtils.deleteDirectory(mediaDir);
+                FileExt.deleteDirectory(mediaDir);
             } else {
                 mediaDir.delete();
             }
diff --git a/forms-test/src/main/java/org/odk/collect/formstest/InMemInstancesRepository.java b/forms-test/src/main/java/org/odk/collect/formstest/InMemInstancesRepository.java
@@ -2,7 +2,7 @@
 
 import org.odk.collect.forms.instances.Instance;
 import org.odk.collect.forms.instances.InstancesRepository;
-import org.odk.collect.shared.files.DirectoryUtils;
+import org.odk.collect.shared.files.FileExt;
 
 import java.io.File;
 import java.util.ArrayList;
@@ -192,7 +192,7 @@ public void removeInstanceById(Long databaseId) {
 
     private void deleteInstanceFiles(Instance instance) {
         if (instance.getInstanceFilePath() != null) {
-            DirectoryUtils.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());
+            FileExt.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());
         }
     }
 }
diff --git a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt
@@ -2,7 +2,7 @@ package org.odk.collect.maps.layers
 
 import org.odk.collect.maps.MapConfigurator
 import org.odk.collect.shared.PathUtils
-import org.odk.collect.shared.files.DirectoryUtils.listFilesRecursively
+import org.odk.collect.shared.files.FileExt.listFilesRecursively
 import java.io.File
 
 class DirectoryReferenceLayerRepository(
@@ -41,7 +41,7 @@ class DirectoryReferenceLayerRepository(
     }
 
     private fun getAllFilesWithDirectory() = listOf(sharedLayersDirPath, projectLayersDirPath).flatMap { dir ->
-        listFilesRecursively(File(dir)).map { file ->
+        File(dir).listFilesRecursively().map { file ->
             Pair(file, dir)
         }
     }
diff --git a/shared/build.gradle.kts b/shared/build.gradle.kts
@@ -18,6 +18,7 @@ dependencies {
 
     testImplementation(Dependencies.junit)
     testImplementation(Dependencies.hamcrest)
+    testImplementation(Dependencies.mockito_kotlin)
 }
 
 tasks.register("testDebug") {
diff --git a/shared/src/main/java/org/odk/collect/shared/files/DirectoryUtils.kt b/shared/src/main/java/org/odk/collect/shared/files/DirectoryUtils.kt
diff --git a/shared/src/main/java/org/odk/collect/shared/files/FileExt.kt b/shared/src/main/java/org/odk/collect/shared/files/FileExt.kt
@@ -0,0 +1,54 @@
+package org.odk.collect.shared.files
+
+import java.io.File
+import java.io.IOException
+import java.io.InputStream
+import kotlin.jvm.Throws
+
+object FileExt {
+    /**
+     * Original File.getCanonicalPath() may return paths with inconsistent letter casing for the
+     * /Android/data/ part of the path, such as /storage/emulated/0/android/data/... or /storage/emulated/0/Android/Data/...
+     * instead of the expected /storage/emulated/0/Android/data/...
+     * Since the Android file system is case-sensitive, this behavior appears to be a bug.
+     *
+     * For more details, see the discussion on Stack Overflow:
+     * https://stackoverflow.com/questions/78965720/file-getcanonicalpath-returns-inconsistent-letter-casing-in-path
+     */
+    fun File.sanitizedCanonicalPath(): String {
+        val androidDataSegment = "/Android/data/"
+        val regex = Regex(androidDataSegment, RegexOption.IGNORE_CASE)
+
+        return canonicalPath.replace(regex, androidDataSegment)
+    }
+
+    fun File.listFilesRecursively(): List<File> {
+        val listFiles = listFiles() ?: emptyArray()
+        return listFiles.flatMap {
+            if (it.isDirectory) {
+                it.listFilesRecursively()
+            } else {
+                listOf(it)
+            }
+        }
+    }
+
+    @JvmStatic
+    fun File.deleteDirectory() {
+        deleteRecursively()
+    }
+
+    @Throws(IOException::class)
+    @JvmStatic
+    fun File.saveToFile(inputStream: InputStream) {
+        if (exists() && !delete()) {
+            throw IOException("Cannot overwrite $absolutePath. Perhaps the file is locked?")
+        }
+
+        inputStream.use { input ->
+            outputStream().use { output ->
+                input.copyTo(output)
+            }
+        }
+    }
+}
diff --git a/shared/src/main/java/org/odk/collect/shared/files/FileUtils.kt b/shared/src/main/java/org/odk/collect/shared/files/FileUtils.kt
diff --git a/shared/src/test/java/org/odk/collect/shared/files/FileExtTest.kt b/shared/src/test/java/org/odk/collect/shared/files/FileExtTest.kt

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@`
`11`	`11`	`import org.odk.collect.android.database.DatabaseConstants;`
`12`	`12`	`import org.odk.collect.forms.instances.Instance;`
`13`	`13`	`import org.odk.collect.forms.instances.InstancesRepository;`
`14`		`-import org.odk.collect.shared.files.DirectoryUtils;`
	`14`	`+import org.odk.collect.shared.files.FileExt;`
`15`	`15`
`16`	`16`	`import java.io.File;`
`17`	`17`	`import java.util.ArrayList;`
`@@ -270,7 +270,7 @@ private void update(Long instanceId, ContentValues values) {`
`270`	`270`	`}`
`271`	`271`
`272`	`272`	`private void deleteInstanceFiles(Instance instance) {`
`273`		`- DirectoryUtils.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());`
	`273`	`+ FileExt.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());`
`274`	`274`	`}`
`275`	`275`
`276`	`276`	`private static List<Instance> getInstancesFromCursor(Cursor cursor, String instancesPath) {`
Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`
`3`	`3`	`import org.odk.collect.forms.instances.Instance;`
`4`	`4`	`import org.odk.collect.forms.instances.InstancesRepository;`
`5`		`-import org.odk.collect.shared.files.DirectoryUtils;`
	`5`	`+import org.odk.collect.shared.files.FileExt;`
`6`	`6`
`7`	`7`	`import java.io.File;`
`8`	`8`	`import java.util.ArrayList;`
`@@ -192,7 +192,7 @@ public void removeInstanceById(Long databaseId) {`
`192`	`192`
`193`	`193`	`private void deleteInstanceFiles(Instance instance) {`
`194`	`194`	`if (instance.getInstanceFilePath() != null) {`
`195`		`- DirectoryUtils.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());`
	`195`	`+ FileExt.deleteDirectory(new File(instance.getInstanceFilePath()).getParentFile());`
`196`	`196`	`}`
`197`	`197`	`}`
`198`	`198`	`}`