Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented #sanitizedCanonicalPath method #6404

Merged
merged 9 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -67,7 +66,7 @@ class ExistingProjectMigrator(
}

try {
DirectoryUtils.deleteDirectory(File(rootDir, ".cache"))
File(rootDir, ".cache").deleteRecursively()
} catch (e: Exception) {
// ignore
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -79,7 +79,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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
}
}
Expand Down
1 change: 1 addition & 0 deletions shared/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ dependencies {

testImplementation(Dependencies.junit)
testImplementation(Dependencies.hamcrest)
testImplementation(Dependencies.mockito_kotlin)
}

tasks.register("testDebug") {
Expand Down

This file was deleted.

60 changes: 60 additions & 0 deletions shared/src/main/java/org/odk/collect/shared/files/FileExt.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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 canonicalPath = canonicalPath

if (canonicalPath.contains(androidDataSegment, true)) {
val regex = Regex(androidDataSegment, RegexOption.IGNORE_CASE)
return canonicalPath.replace(regex, androidDataSegment)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to do the replace without a contains check -- if there's no match it just won't be replaced.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

}

return canonicalPath
}

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)
}
}
}
}
23 changes: 0 additions & 23 deletions shared/src/main/java/org/odk/collect/shared/files/FileUtils.kt

This file was deleted.

Loading