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 deleted file mode 100644 index 96975b681e3..00000000000 --- a/androidshared/src/main/java/org/odk/collect/androidshared/utils/PathUtils.kt +++ /dev/null @@ -1,27 +0,0 @@ -package org.odk.collect.androidshared.utils - -import org.odk.collect.shared.files.FileExt.sanitizedCanonicalPath -import timber.log.Timber -import java.io.File - -object PathUtils { - @JvmStatic - fun getAbsoluteFilePath(dirPath: String, filePath: String): String { - val absoluteFilePath = - if (filePath.startsWith(dirPath)) filePath else dirPath + File.separator + filePath - - 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" + - "dirPath: $dirPath\n" + - "filePath: $filePath\n" + - "absoluteFilePath: $absoluteFilePath\n" + - "canonicalAbsoluteFilePath: $canonicalAbsoluteFilePath\n" + - "canonicalDirPath: $canonicalDirPath" - ) - } - return absoluteFilePath - } -} diff --git a/androidshared/src/test/java/org/odk/collect/androidshared/utils/PathUtilsTest.kt b/androidshared/src/test/java/org/odk/collect/androidshared/utils/PathUtilsTest.kt index 472b285027d..27bad2a3d8a 100644 --- a/androidshared/src/test/java/org/odk/collect/androidshared/utils/PathUtilsTest.kt +++ b/androidshared/src/test/java/org/odk/collect/androidshared/utils/PathUtilsTest.kt @@ -3,6 +3,7 @@ package org.odk.collect.androidshared.utils import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test +import org.odk.collect.shared.PathUtils import org.odk.collect.shared.TempFiles import java.io.File diff --git a/collect_app/src/main/java/org/odk/collect/android/database/DatabaseObjectMapper.kt b/collect_app/src/main/java/org/odk/collect/android/database/DatabaseObjectMapper.kt index f24e3e632a4..53cc4442eb4 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/DatabaseObjectMapper.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/DatabaseObjectMapper.kt @@ -5,9 +5,9 @@ import android.database.Cursor import android.provider.BaseColumns import org.odk.collect.android.database.forms.DatabaseFormColumns import org.odk.collect.android.database.instances.DatabaseInstanceColumns -import org.odk.collect.androidshared.utils.PathUtils.getAbsoluteFilePath import org.odk.collect.forms.Form import org.odk.collect.forms.instances.Instance +import org.odk.collect.shared.PathUtils.getAbsoluteFilePath import org.odk.collect.shared.PathUtils.getRelativeFilePath import java.lang.Boolean diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index 4d89d544110..87d8b39ece6 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -382,8 +382,8 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep val missingColumns = entity.properties .map { EntitiesTable.getPropertyColumn(it.first) } - .filterNot { columnNames.contains(it) } .distinctBy { it.lowercase() } + .filterNot { columnName -> columnNames.any { it.equals(columnName, ignoreCase = true) } } if (missingColumns.isNotEmpty()) { databaseConnection.resetTransaction { diff --git a/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt index 6b126b04acd..363f2c0f779 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/savepoints/DatabaseSavepointsRepository.kt @@ -8,7 +8,6 @@ import org.odk.collect.android.database.DatabaseConstants.SAVEPOINTS_DATABASE_VE import org.odk.collect.android.database.DatabaseConstants.SAVEPOINTS_TABLE_NAME import org.odk.collect.android.database.savepoints.DatabaseSavepointsColumns.FORM_DB_ID import org.odk.collect.android.database.savepoints.DatabaseSavepointsColumns.INSTANCE_DB_ID -import org.odk.collect.androidshared.utils.PathUtils.getAbsoluteFilePath import org.odk.collect.db.sqlite.CursorExt.foldAndClose import org.odk.collect.db.sqlite.DatabaseConnection import org.odk.collect.db.sqlite.SQLiteDatabaseExt.delete @@ -16,6 +15,7 @@ import org.odk.collect.db.sqlite.SQLiteDatabaseExt.query import org.odk.collect.forms.savepoints.Savepoint import org.odk.collect.forms.savepoints.SavepointsRepository import org.odk.collect.shared.PathUtils +import org.odk.collect.shared.PathUtils.getAbsoluteFilePath import java.io.File class DatabaseSavepointsRepository( diff --git a/collect_app/src/main/java/org/odk/collect/android/fastexternalitemset/ItemsetDbAdapter.java b/collect_app/src/main/java/org/odk/collect/android/fastexternalitemset/ItemsetDbAdapter.java index e19bfa8f1e1..c540c146362 100644 --- a/collect_app/src/main/java/org/odk/collect/android/fastexternalitemset/ItemsetDbAdapter.java +++ b/collect_app/src/main/java/org/odk/collect/android/fastexternalitemset/ItemsetDbAdapter.java @@ -1,6 +1,6 @@ package org.odk.collect.android.fastexternalitemset; -import static org.odk.collect.androidshared.utils.PathUtils.getAbsoluteFilePath; +import static org.odk.collect.shared.PathUtils.getAbsoluteFilePath; import android.content.ContentValues; import android.database.Cursor; diff --git a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt index 759bbc87fa0..6a09b148bc9 100644 --- a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt @@ -736,7 +736,7 @@ abstract class EntitiesRepositoryTest { } @Test - fun `#save ignores case-insensitive duplicate properties`() { + fun `#save ignores case-insensitive duplicate new properties`() { val repository = buildSubject() val entity = Entity.New( "1", @@ -749,4 +749,24 @@ abstract class EntitiesRepositoryTest { assertThat(savedEntities[0].properties.size, equalTo(1)) assertThat(savedEntities[0].properties[0].first, equalTo("prop")) } + + @Test + fun `#save ignores case-insensitive duplicate properties if one of them has already been saved`() { + val repository = buildSubject() + val entity = Entity.New( + "1", + "One", + properties = listOf(Pair("prop", "value")) + ) + + repository.save("things", entity) + var savedEntities = repository.getEntities("things") + assertThat(savedEntities[0].properties.size, equalTo(1)) + assertThat(savedEntities[0].properties[0].first, equalTo("prop")) + + repository.save("things", entity.copy(properties = listOf(Pair("Prop", "value")))) + savedEntities = repository.getEntities("things") + assertThat(savedEntities[0].properties.size, equalTo(1)) + assertThat(savedEntities[0].properties[0].first, equalTo("prop")) + } } diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java index d83730bcc24..8edd9d61180 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/download/ServerFormDownloaderTest.java @@ -11,9 +11,9 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.odk.collect.android.utilities.FileUtils.read; -import static org.odk.collect.androidshared.utils.PathUtils.getAbsoluteFilePath; import static org.odk.collect.formstest.FormUtils.buildForm; import static org.odk.collect.formstest.FormUtils.createXFormBody; +import static org.odk.collect.shared.PathUtils.getAbsoluteFilePath; import static java.util.Arrays.asList; import com.google.common.io.Files; diff --git a/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt b/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt index e467df1a104..dc07a81d9b1 100644 --- a/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt +++ b/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt @@ -121,13 +121,15 @@ class InMemEntitiesRepository : EntitiesRepository { private fun updateLists(list: String, entity: Entity) { lists.add(list) - listProperties.getOrPut(list) { + val properties = listProperties.getOrPut(list) { mutableSetOf() - }.addAll( + } + properties.addAll( entity .properties - .distinctBy { it.first.lowercase() } .map { it.first } + .distinctBy { it.lowercase() } + .filterNot { properties.any { property -> property.equals(it, ignoreCase = true) } } ) } diff --git a/shared/src/main/java/org/odk/collect/shared/PathUtils.kt b/shared/src/main/java/org/odk/collect/shared/PathUtils.kt index 1f4ba227cc8..102c33e97b6 100644 --- a/shared/src/main/java/org/odk/collect/shared/PathUtils.kt +++ b/shared/src/main/java/org/odk/collect/shared/PathUtils.kt @@ -1,5 +1,8 @@ package org.odk.collect.shared +import org.odk.collect.shared.files.FileExt.sanitizedCanonicalPath +import java.io.File + object PathUtils { @JvmStatic @@ -10,4 +13,16 @@ object PathUtils { // https://stackoverflow.com/questions/2679699/what-characters-allowed-in-file-names-on-android @JvmStatic fun getPathSafeFileName(fileName: String) = fileName.replace("[\"*/:<>?\\\\|]".toRegex(), "_") + + @JvmStatic + fun getAbsoluteFilePath(dirPath: String, filePath: String): String { + val absoluteFilePath = + if (filePath.startsWith(dirPath)) filePath else dirPath + File.separator + filePath + + if (File(absoluteFilePath).sanitizedCanonicalPath().startsWith(File(dirPath).sanitizedCanonicalPath())) { + return absoluteFilePath + } else { + throw SecurityException("Contact support@getodk.org. Attempt to access file outside of Collect directory: $absoluteFilePath") + } + } }