diff --git a/dd-sdk-android-ndk/src/main/kotlin/com/datadog/android/ndk/NdkCrashReportsPlugin.kt b/dd-sdk-android-ndk/src/main/kotlin/com/datadog/android/ndk/NdkCrashReportsPlugin.kt index e234b91665..2711e81d77 100644 --- a/dd-sdk-android-ndk/src/main/kotlin/com/datadog/android/ndk/NdkCrashReportsPlugin.kt +++ b/dd-sdk-android-ndk/src/main/kotlin/com/datadog/android/ndk/NdkCrashReportsPlugin.kt @@ -104,7 +104,7 @@ class NdkCrashReportsPlugin : DatadogPlugin { // endregion companion object { - internal const val NDK_CRASH_REPORTS_FOLDER = "ndk_crash_reports" + internal const val NDK_CRASH_REPORTS_FOLDER = "ndk_crash_reports_v2" private const val TAG: String = "NdkCrashReportsPlugin" private const val ERROR_LOADING_NATIVE_MESSAGE: String = "We could not load the native library" diff --git a/dd-sdk-android/build.gradle.kts b/dd-sdk-android/build.gradle.kts index 6b131e7942..da820cf998 100644 --- a/dd-sdk-android/build.gradle.kts +++ b/dd-sdk-android/build.gradle.kts @@ -167,7 +167,6 @@ unMock { keep("android.os.SystemProperties") keep("android.view.Choreographer") keep("android.view.DisplayEventReceiver") - keep("android.util.Base64") keepStartingWith("org.json") } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt index 12024dd66b..044317c176 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt @@ -261,7 +261,7 @@ internal object CoreFeature { UserInfoDeserializer(sdkLogger), sdkLogger, timeProvider, - localDataEncryption + BatchFileHandler.create(sdkLogger, localDataEncryption) ) ndkCrashHandler.prepareData() } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataFlusher.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataFlusher.kt index f8a348a6b7..28a04bf144 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataFlusher.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/data/upload/DataFlusher.kt @@ -10,6 +10,7 @@ import com.datadog.android.core.internal.net.DataUploader import com.datadog.android.core.internal.persistence.PayloadDecoration import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.core.internal.persistence.file.FileOrchestrator +import com.datadog.android.core.internal.utils.join internal class DataFlusher( internal val fileOrchestrator: FileOrchestrator, @@ -20,12 +21,12 @@ internal class DataFlusher( override fun flush(uploader: DataUploader) { val toUploadFiles = fileOrchestrator.getFlushableFiles() toUploadFiles.forEach { - val batch = handler.readData( - it, - decoration.prefixBytes, - decoration.suffixBytes, - decoration.separatorBytes - ) + val batch = handler.readData(it) + .join( + separator = decoration.separatorBytes, + prefix = decoration.prefixBytes, + suffix = decoration.suffixBytes + ) uploader.upload(batch) handler.delete(it) } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandler.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandler.kt index 08ef2bdd18..14a37975ad 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandler.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandler.kt @@ -6,44 +6,20 @@ package com.datadog.android.core.internal.persistence.file -import android.util.Base64 -import com.datadog.android.core.internal.utils.copyTo import com.datadog.android.core.internal.utils.devLogger -import com.datadog.android.log.Logger -import com.datadog.android.log.internal.utils.errorWithTelemetry import com.datadog.android.security.Encryption import java.io.File internal class EncryptedFileHandler( internal val encryption: Encryption, - internal val delegate: FileHandler, - private val internalLogger: Logger, - private val base64Encoder: (ByteArray) -> ByteArray = { Base64.encode(it, ENCODING_FLAGS) }, - private val base64Decoder: (ByteArray) -> ByteArray = { - // lambda call is safe-guarded at the call site - @Suppress("UnsafeThirdPartyFunctionCall") - Base64.decode(it, ENCODING_FLAGS) - } -) : FileHandler { - - // region FileHandler + internal val delegate: FileHandler +) : FileHandler by delegate { override fun writeData( file: File, data: ByteArray, - append: Boolean, - separator: ByteArray? + append: Boolean ): Boolean { - if (separator != null && !checkSeparator(separator)) { - internalLogger.errorWithTelemetry(INVALID_SEPARATOR_MESSAGE) - return false - } - - if (append && separator == null) { - internalLogger.errorWithTelemetry(MISSING_SEPARATOR_MESSAGE) - return false - } - val encryptedData = encryption.encrypt(data) if (data.isNotEmpty() && encryptedData.isEmpty()) { @@ -53,205 +29,22 @@ internal class EncryptedFileHandler( return delegate.writeData( file, - // Base64 produces bytes per US-ASCII encoding, while separator may be in UTF-8 encoding - // but this is fine, because UTF-8 is backward compatible with US-ASCII (char in - // US-ASCII encoding has the same byte value as char in UTF-8 encoding) - base64Encoder(encryptedData), - append, - separator + encryptedData, + append ) } override fun readData( - file: File, - prefix: ByteArray?, - suffix: ByteArray?, - separator: ByteArray? - ): ByteArray { - if (separator != null && !checkSeparator(separator)) { - internalLogger.errorWithTelemetry(INVALID_SEPARATOR_MESSAGE) - return EMPTY_BYTE_ARRAY - } - - val data = delegate.readData(file, prefix, suffix, separator) - - val rawData = removeSuffixAndPrefix(data, prefix, suffix) - - return if (separator != null) { - val decrypted = decryptBatchData(rawData, separator) - if (decrypted.isEmpty()) { - assemble(EMPTY_BYTE_ARRAY, prefix, suffix) - } else { - assemble(decrypted, prefix, suffix, separator) - } - } else { - val decrypted = decryptSingleItemData(rawData) - assemble(decrypted, prefix, suffix) - } - } - - override fun delete(target: File) = delegate.delete(target) - - override fun moveFiles(srcDir: File, destDir: File) = delegate.moveFiles(srcDir, destDir) - - // endregion - - // region private - - private fun decryptSingleItemData(data: ByteArray): ByteArray { - val decoded = safeDecodeBase64(data) - return if (decoded.isNotEmpty()) { - encryption.decrypt(decoded) - } else { - decoded - } - } - - private fun decryptBatchData(data: ByteArray, separator: ByteArray): List { - return data - .splitBy(separator) + file: File + ): List { + return delegate.readData(file) .map { - decryptSingleItemData(it) + encryption.decrypt(it) } - .filter { it.isNotEmpty() } } - private fun removeSuffixAndPrefix( - data: ByteArray, - prefix: ByteArray?, - suffix: ByteArray? - ): ByteArray { - return if (prefix != null || suffix != null) { - val prefixSize = prefix?.size ?: 0 - val suffixSize = suffix?.size ?: 0 - - if (data.size < prefixSize + suffixSize) { - internalLogger.e(BAD_DATA_READ_MESSAGE) - devLogger.e(BAD_DATA_READ_MESSAGE) - EMPTY_BYTE_ARRAY - } else { - // we check indexes validity just above, plus prefix size and suffix size - // cannot be negative - @Suppress("UnsafeThirdPartyFunctionCall") - data.copyOfRange(prefixSize, data.size - suffixSize) - } - } else { - data - } - } - - private fun safeDecodeBase64(encoded: ByteArray): ByteArray { - return try { - base64Decoder(encoded) - } catch (iae: IllegalArgumentException) { - internalLogger.e(BASE64_DECODING_ERROR_MESSAGE, iae) - devLogger.e(BASE64_DECODING_ERROR_MESSAGE, iae) - EMPTY_BYTE_ARRAY - } - } - - private fun assemble( - items: List, - prefix: ByteArray?, - suffix: ByteArray?, - separator: ByteArray? - ): ByteArray { - val prefixSize = prefix?.size ?: 0 - val suffixSize = suffix?.size ?: 0 - val separatorSize = separator?.size ?: 0 - - val result = ByteArray( - items.sumOf { it.size } + - prefixSize + suffixSize + separatorSize * (items.size - 1) - ) - - var offset = 0 - - if (prefix != null) { - prefix.copyTo(0, result, 0, prefix.size) - offset += prefix.size - } - - for (item in items.withIndex()) { - item.value.copyTo(0, result, offset, item.value.size) - offset += item.value.size - if (separator != null && item.index != items.size - 1) { - separator.copyTo(0, result, offset, separator.size) - offset += separator.size - } - } - - suffix?.copyTo(0, result, offset, suffix.size) - - return result - } - - private fun assemble(item: ByteArray, prefix: ByteArray?, suffix: ByteArray?): ByteArray { - if (prefix == null && suffix == null) { - return item - } - return assemble(listOf(item), prefix, suffix, null) - } - - private fun checkSeparator(separator: ByteArray): Boolean { - // Separator MAY include chars of BASE64 encoding, we cannot allow just ALL chars to - // be BASE64, because in that case separator bytes sequence can be found in the encoded - // item, leading to a wrong split - return separator.any { it.toInt().toChar() !in BASE_64_CHARS } - } - - private fun ByteArray.splitBy(separator: ByteArray): List { - val result = mutableListOf() - - var chunkStart = 0 - var current = 0 - - while (current < this.size) { - var separatorFound = true - for (separatorIndex in separator.indices) { - if (this[separatorIndex + current] != separator[separatorIndex]) { - separatorFound = false - break - } - } - if (separatorFound && chunkStart != current) { - // indices are safe - @Suppress("UnsafeThirdPartyFunctionCall") - result.add(this.copyOfRange(chunkStart, current)) - chunkStart += (current - chunkStart) + separator.size - current = chunkStart - } else { - current++ - } - } - - if (chunkStart < this.size) { - // indices are safe - @Suppress("UnsafeThirdPartyFunctionCall") - result.add(this.copyOfRange(chunkStart, this.size)) - } - - return result - } - - // endregion - companion object { - const val ENCODING_FLAGS = Base64.DEFAULT or Base64.NO_WRAP - - private val EMPTY_BYTE_ARRAY = ByteArray(0) - private val BASE_64_CHARS = - (('A'..'Z') + ('a'..'z') + ('0'..'9') + listOf('+', '/', '=')).toSet() - - internal const val INVALID_SEPARATOR_MESSAGE = "Illegal separator is provided," + - " it cannot be empty or in the Base64 characters set." - internal const val MISSING_SEPARATOR_MESSAGE = - "Separator should be provided in the append mode." internal const val BAD_ENCRYPTION_RESULT_MESSAGE = "Encryption of non-empty data produced" + " empty result, aborting write operation." - internal const val BASE64_DECODING_ERROR_MESSAGE = - "Failure to decode encrypted data from Base64 format. Will return empty item instead." - internal const val BAD_DATA_READ_MESSAGE = - "Corrupted data read: data size should be more than prefix size + suffix size." } } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EventMeta.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EventMeta.kt new file mode 100644 index 0000000000..73ba603b9f --- /dev/null +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/EventMeta.kt @@ -0,0 +1,51 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.core.internal.persistence.file + +import com.google.gson.JsonObject +import com.google.gson.JsonParseException +import com.google.gson.JsonParser +import kotlin.jvm.Throws + +internal data class EventMeta(val eventSize: Int) { + + val asBytes: ByteArray + get() { + return JsonObject() + .apply { + addProperty(EVENT_SIZE_KEY, eventSize) + } + .toString() + .toByteArray(Charsets.UTF_8) + } + + companion object { + + private const val EVENT_SIZE_KEY = "ev_size" + + @Throws(JsonParseException::class) + @Suppress("ThrowingInternalException", "TooGenericExceptionCaught") + fun fromBytes(metaBytes: ByteArray): EventMeta { + return try { + @Suppress("UnsafeThirdPartyFunctionCall") // there is Throws annotation + val json = JsonParser.parseString(String(metaBytes, Charsets.UTF_8)) + .asJsonObject + EventMeta( + eventSize = json.get(EVENT_SIZE_KEY).asInt + ) + } catch (e: IllegalStateException) { + throw JsonParseException(e) + } catch (e: ClassCastException) { + throw JsonParseException(e) + } catch (e: NumberFormatException) { + throw JsonParseException(e) + } catch (e: NullPointerException) { + throw JsonParseException(e) + } + } + } +} diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FileHandler.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FileHandler.kt index dfec1b4ab4..6e71d9456e 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FileHandler.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FileHandler.kt @@ -18,31 +18,22 @@ internal interface FileHandler { * @param file the file to write to * @param data the data to write * @param append whether to append data at the end of the file or overwrite - * @param separator an optional [ByteArray] used when appending to a non empty file * @return whether the write operation was successful */ fun writeData( file: File, data: ByteArray, - append: Boolean, - separator: ByteArray? + append: Boolean ): Boolean /** * Reads data from the given file. * @param file the file to read from - * @param prefix an (optional) prefix to embed before the file content - * @param suffix an (optional) suffix to embed after the file content - * @param separator an (optional) separator used to write the data in append mode, is needed - * only in the case of using encryption for data storage - * @return the [ByteArray] data or an empty array if the file can't be read (e.g.: exception) + * @return the list of events as [ByteArray] data stored in a file. */ fun readData( - file: File, - prefix: ByteArray?, - suffix: ByteArray?, - separator: ByteArray? - ): ByteArray + file: File + ): List /** * Deletes the file or directory (recursively if needed). diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FilePersistenceConfig.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FilePersistenceConfig.kt index bd7a3d152d..501d993570 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FilePersistenceConfig.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/FilePersistenceConfig.kt @@ -12,7 +12,7 @@ package com.datadog.android.core.internal.persistence.file internal data class FilePersistenceConfig( val recentDelayMs: Long = MAX_DELAY_BETWEEN_MESSAGES_MS, val maxBatchSize: Long = MAX_BATCH_SIZE, - val maxItemSize: Long = MAX_ITEMS_SIZE, + val maxItemSize: Long = MAX_ITEM_SIZE, val maxItemsPerBatch: Int = MAX_ITEMS_PER_BATCH, val oldFileThreshold: Long = OLD_FILE_THRESHOLD, val maxDiskSpace: Long = MAX_DISK_SPACE @@ -20,7 +20,7 @@ internal data class FilePersistenceConfig( companion object { internal const val MAX_BATCH_SIZE: Long = 4L * 1024 * 1024 // 4 MB internal const val MAX_ITEMS_PER_BATCH: Int = 500 - internal const val MAX_ITEMS_SIZE: Long = 512L * 1024 // 512 KB + internal const val MAX_ITEM_SIZE: Long = 512L * 1024 // 512 KB internal const val OLD_FILE_THRESHOLD: Long = 18L * 60L * 60L * 1000L // 18 hours internal const val MAX_DISK_SPACE: Long = 128 * MAX_BATCH_SIZE // 512 MB internal const val MAX_DELAY_BETWEEN_MESSAGES_MS = 5000L diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestrator.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestrator.kt index ef9e5a6941..b9a416c8e8 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestrator.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestrator.kt @@ -56,7 +56,7 @@ internal class FeatureFileOrchestrator( ) companion object { - internal const val VERSION = 1 + internal const val VERSION = 2 internal const val PENDING_DIR = "dd-%s-pending-v$VERSION" internal const val GRANTED_DIR = "dd-%s-v$VERSION" diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReader.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReader.kt index e01025e612..8099812f08 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReader.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReader.kt @@ -11,6 +11,7 @@ import com.datadog.android.core.internal.persistence.DataReader import com.datadog.android.core.internal.persistence.PayloadDecoration import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.core.internal.persistence.file.FileOrchestrator +import com.datadog.android.core.internal.utils.join import com.datadog.android.log.Logger import java.io.File import java.util.Locale @@ -31,12 +32,12 @@ internal class BatchFileDataReader( override fun lockAndReadNext(): Batch? { val file = getAndLockReadableFile() ?: return null - val data = handler.readData( - file, - decoration.prefixBytes, - decoration.suffixBytes, - decoration.separatorBytes - ) + val data = handler.readData(file) + .join( + separator = decoration.separatorBytes, + prefix = decoration.prefixBytes, + suffix = decoration.suffixBytes + ) return Batch(file.name, data) } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriter.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriter.kt index dc1be2498d..d295a37db1 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriter.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriter.kt @@ -70,7 +70,7 @@ internal open class BatchFileDataWriter( private fun writeData(byteArray: ByteArray): Boolean { val file = fileOrchestrator.getWritableFile(byteArray.size) ?: return false - return handler.writeData(file, byteArray, true, decoration.separatorBytes) + return handler.writeData(file, byteArray, true) } // endregion diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandler.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandler.kt index 727b2bb242..d9a145a02e 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandler.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandler.kt @@ -7,6 +7,7 @@ package com.datadog.android.core.internal.persistence.file.batch import com.datadog.android.core.internal.persistence.file.EncryptedFileHandler +import com.datadog.android.core.internal.persistence.file.EventMeta import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.core.internal.persistence.file.existsSafe import com.datadog.android.core.internal.persistence.file.isDirectorySafe @@ -15,18 +16,27 @@ import com.datadog.android.core.internal.persistence.file.listFilesSafe import com.datadog.android.core.internal.persistence.file.mkdirsSafe import com.datadog.android.core.internal.persistence.file.renameToSafe import com.datadog.android.core.internal.utils.copyTo +import com.datadog.android.core.internal.utils.devLogger import com.datadog.android.core.internal.utils.use import com.datadog.android.log.Logger import com.datadog.android.log.internal.utils.errorWithTelemetry import com.datadog.android.security.Encryption +import com.google.gson.JsonParseException import java.io.File import java.io.FileNotFoundException import java.io.FileOutputStream import java.io.IOException +import java.io.InputStream import java.util.Locale internal class BatchFileHandler( - private val internalLogger: Logger + private val internalLogger: Logger, + private val metaGenerator: (data: ByteArray) -> ByteArray = { + EventMeta(it.size).asBytes + }, + private val metaParser: (metaBytes: ByteArray) -> EventMeta = { + EventMeta.fromBytes(it) + } ) : FileHandler { // region FileHandler @@ -34,11 +44,10 @@ internal class BatchFileHandler( override fun writeData( file: File, data: ByteArray, - append: Boolean, - separator: ByteArray? + append: Boolean ): Boolean { return try { - lockFileAndWriteData(file, append, separator, data) + lockFileAndWriteData(file, append, data) true } catch (e: IOException) { internalLogger.errorWithTelemetry(ERROR_WRITE.format(Locale.US, file.path), e) @@ -50,19 +59,16 @@ internal class BatchFileHandler( } override fun readData( - file: File, - prefix: ByteArray?, - suffix: ByteArray?, - separator: ByteArray? - ): ByteArray { + file: File + ): List { return try { - readFileData(file, prefix ?: EMPTY_BYTE_ARRAY, suffix ?: EMPTY_BYTE_ARRAY) + readFileData(file) } catch (e: IOException) { internalLogger.errorWithTelemetry(ERROR_READ.format(Locale.US, file.path), e) - EMPTY_BYTE_ARRAY + emptyList() } catch (e: SecurityException) { internalLogger.errorWithTelemetry(ERROR_READ.format(Locale.US, file.path), e) - EMPTY_BYTE_ARRAY + emptyList() } } @@ -110,14 +116,34 @@ internal class BatchFileHandler( private fun lockFileAndWriteData( file: File, append: Boolean, - separator: ByteArray?, data: ByteArray ) { FileOutputStream(file, append).use { outputStream -> outputStream.channel.lock().use { - if (file.length() > 0 && separator != null) { - outputStream.write(separator) + val meta = metaGenerator(data) + + if (meta.size > MAX_META_SIZE_BYTES) { + @Suppress("ThrowingInternalException") + throw MetaTooBigException( + "Meta size is bigger than limit of $MAX_META_SIZE_BYTES" + + " bytes, cannot write data." + ) } + + // 1 byte for version + // 1 byte for meta.size value + // rest is meta + val header = ByteArray(2 + meta.size).apply { + set(0, HEADER_VERSION) + // in Kotlin and Java byte type is signed, meaning it goes from -127 to 128. + // It is completely fine to have size more than 128, it will be just stored + // as negative value. Later at read() byte step we will get strictly positive + // value, because read() returns int. + set(1, meta.size.toByte()) + meta.copyTo(0, this, 2, meta.size) + } + + outputStream.write(header) outputStream.write(data) } } @@ -126,38 +152,38 @@ internal class BatchFileHandler( @Throws(IOException::class) @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block private fun readFileData( - file: File, - prefix: ByteArray, - suffix: ByteArray - ): ByteArray { + file: File + ): List { val inputLength = file.lengthSafe().toInt() - val outputLength = inputLength + prefix.size + suffix.size - val result = ByteArray(outputLength) - // Copy prefix - prefix.copyTo(0, result, 0, prefix.size) + val result = mutableListOf() // Read file iteratively - var offset = prefix.size var remaining = inputLength - file.inputStream().use { + file.inputStream().buffered().use { while (remaining > 0) { - val read = it.read(result, offset, remaining) - if (read < 0) break - offset += read + val (meta, headerSize) = readEventHeader(it) ?: break + + val eventBytes = ByteArray(meta.eventSize) + val readEventSize = it.read(eventBytes, 0, meta.eventSize) + + if (!checkReadSizeExpected(meta.eventSize, readEventSize, "read event")) { + break + } + + result.add(eventBytes) + val read = headerSize + readEventSize remaining -= read } } - // Copy suffix - suffix.copyTo(0, result, offset, suffix.size) - offset += suffix.size - - return if (result.size == offset) { - result - } else { - result.copyOf(offset) + if (remaining != 0) { + val message = WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) + devLogger.e(message) + internalLogger.errorWithTelemetry(message) } + + return result } private fun moveFile(file: File, destDir: File): Boolean { @@ -165,12 +191,58 @@ internal class BatchFileHandler( return file.renameToSafe(destFile) } + @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block + private fun readEventHeader(stream: InputStream): Pair? { + val version = stream.read() + if (version < 0) { + internalLogger.e(ERROR_EOF_AT_VERSION_BYTE) + return null + } + + val metaSize = stream.read() + if (metaSize < 0) { + internalLogger.e(ERROR_EOF_AT_META_SIZE_BYTE) + return null + } + + val metaBytes = ByteArray(metaSize) + val readMetaSize = stream.read(metaBytes, 0, metaBytes.size) + + if (!checkReadSizeExpected(metaSize, readMetaSize, "read meta")) { + return null + } + + val meta = try { + metaParser(metaBytes) + } catch (e: JsonParseException) { + internalLogger.e(ERROR_FAILED_META_PARSE, e) + return null + } + + return meta to 2 + readMetaSize + } + + private fun checkReadSizeExpected(expected: Int, actual: Int, operation: String): Boolean { + return if (expected != actual) { + internalLogger.e( + "Number of bytes read for operation='$operation' doesn't" + + " match with expected: expected=$expected, actual=$actual" + ) + false + } else { + true + } + } + + internal class MetaTooBigException(message: String) : IOException(message) + // endregion @Suppress("StringLiteralDuplication") companion object { - private val EMPTY_BYTE_ARRAY = ByteArray(0) + internal const val HEADER_VERSION: Byte = 1 + internal const val MAX_META_SIZE_BYTES = 255 internal const val ERROR_WRITE = "Unable to write data to file: %s" internal const val ERROR_READ = "Unable to read data from file: %s" @@ -182,6 +254,15 @@ internal class BatchFileHandler( internal const val ERROR_MOVE_NO_DST = "Unable to move files; " + "could not create directory: %s" + internal const val ERROR_EOF_AT_META_SIZE_BYTE = + "Cannot read meta size byte, because EOF reached." + internal const val ERROR_EOF_AT_VERSION_BYTE = + "Cannot read version byte, because EOF reached." + internal const val ERROR_FAILED_META_PARSE = + "Failed to parse meta bytes, stopping file read." + internal const val WARNING_NOT_ALL_DATA_READ = + "File %s is probably corrupted, not all content was read." + /** * Creates either plain [BatchFileHandler] or [BatchFileHandler] wrapped in * [EncryptedFileHandler] if encryption is provided. @@ -190,7 +271,7 @@ internal class BatchFileHandler( return if (encryption == null) { BatchFileHandler(internalLogger) } else { - EncryptedFileHandler(encryption, BatchFileHandler(internalLogger), internalLogger) + EncryptedFileHandler(encryption, BatchFileHandler(internalLogger)) } } } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriter.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriter.kt index e673ea54d5..a4708621bd 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriter.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriter.kt @@ -45,7 +45,7 @@ internal open class SingleItemDataWriter( private fun writeData(byteArray: ByteArray): Boolean { val file = fileOrchestrator.getWritableFile(byteArray.size) ?: return false - return handler.writeData(file, byteArray, false, null) + return handler.writeData(file, byteArray, false) } // endregion diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/utils/ByteArrayExt.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/utils/ByteArrayExt.kt index 93c5befbe9..294e3560b1 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/utils/ByteArrayExt.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/core/internal/utils/ByteArrayExt.kt @@ -31,6 +31,39 @@ internal fun ByteArray.split(delimiter: Byte): List { return result } +/** + * Joins a collection of [ByteArray] elements into a single [ByteArray], taking into account + * separator between elements and prefix and suffix decoration of the final array. + */ +internal fun Collection.join( + separator: ByteArray, + prefix: ByteArray = ByteArray(0), + suffix: ByteArray = ByteArray(0) +): ByteArray { + val result = ByteArray( + this.sumOf { it.size } + + prefix.size + suffix.size + separator.size * (this.size - 1) + ) + + var offset = 0 + + prefix.copyTo(0, result, 0, prefix.size) + offset += prefix.size + + for (item in this.withIndex()) { + item.value.copyTo(0, result, offset, item.value.size) + offset += item.value.size + if (item.index != this.size - 1) { + separator.copyTo(0, result, offset, separator.size) + offset += separator.size + } + } + + suffix.copyTo(0, result, offset, suffix.size) + + return result +} + /** * Returns the index within this [ByteArray] of the first occurrence of the specified [b], * starting from the specified [startIndex]. @@ -47,7 +80,7 @@ internal fun ByteArray.indexOf(b: Byte, startIndex: Int = 0): Int { } /** - * Performs a safe version of [System.arrayCopy] by performing the necessary checks and try-catch. + * Performs a safe version of [System.arraycopy] by performing the necessary checks and try-catch. * * @return true if the copy was successful. */ diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt index 4759275ea3..a371c68cf2 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt @@ -69,7 +69,7 @@ internal class RumDataWriter( // folder, so if NDK reporting plugin is not initialized, this NDK reports dir won't exist // as well (and no need to write). if (lastViewEventFile.parentFile?.existsSafe() == true) { - handler.writeData(lastViewEventFile, data, false, null) + handler.writeData(lastViewEventFile, data, false) } else { sdkLogger.i( LAST_VIEW_EVENT_DIR_MISSING_MESSAGE.format(Locale.US, lastViewEventFile.parent) diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandler.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandler.kt index 888b9482cc..af1629f39b 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandler.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandler.kt @@ -7,17 +7,15 @@ package com.datadog.android.rum.internal.ndk import android.content.Context -import android.util.Base64 import com.datadog.android.core.internal.CoreFeature import com.datadog.android.core.internal.persistence.DataWriter import com.datadog.android.core.internal.persistence.Deserializer -import com.datadog.android.core.internal.persistence.file.EncryptedFileHandler +import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.core.internal.persistence.file.existsSafe import com.datadog.android.core.internal.persistence.file.listFilesSafe -import com.datadog.android.core.internal.persistence.file.readBytesSafe import com.datadog.android.core.internal.persistence.file.readTextSafe import com.datadog.android.core.internal.time.TimeProvider -import com.datadog.android.core.internal.utils.devLogger +import com.datadog.android.core.internal.utils.join import com.datadog.android.core.model.NetworkInfo import com.datadog.android.core.model.UserInfo import com.datadog.android.log.LogAttributes @@ -28,7 +26,6 @@ import com.datadog.android.log.model.LogEvent import com.datadog.android.rum.internal.domain.event.RumEventSourceProvider import com.datadog.android.rum.model.ErrorEvent import com.datadog.android.rum.model.ViewEvent -import com.datadog.android.security.Encryption import java.io.File import java.util.Locale import java.util.concurrent.ExecutorService @@ -45,7 +42,7 @@ internal class DatadogNdkCrashHandler( private val userInfoDeserializer: Deserializer, private val internalLogger: Logger, private val timeProvider: TimeProvider, - private val localDataEncryption: Encryption?, + private val fileHandler: FileHandler, private val rumEventSourceProvider: RumEventSourceProvider = RumEventSourceProvider(CoreFeature.sourceName) ) : NdkCrashHandler { @@ -99,13 +96,13 @@ internal class DatadogNdkCrashHandler( CRASH_DATA_FILE_NAME -> lastSerializedNdkCrashLog = it.readTextSafe() RUM_VIEW_EVENT_FILE_NAME -> lastSerializedRumViewEvent = - readFileContent(it, localDataEncryption) + readFileContent(it, fileHandler) USER_INFO_FILE_NAME -> lastSerializedUserInformation = - readFileContent(it, localDataEncryption) + readFileContent(it, fileHandler) NETWORK_INFO_FILE_NAME -> lastSerializedNetworkInformation = - readFileContent(it, localDataEncryption) + readFileContent(it, fileHandler) } } } catch (e: SecurityException) { @@ -115,30 +112,12 @@ internal class DatadogNdkCrashHandler( } } - private fun readFileContent(file: File, encryption: Encryption?): String? { - return if (encryption == null) { - file.readTextSafe() + private fun readFileContent(file: File, fileHandler: FileHandler): String? { + val content = fileHandler.readData(file) + return if (content.isEmpty()) { + null } else { - val bytes = file.readBytesSafe() ?: return null - // if bytes is not a valid sequence for the given charset encoding, String - // constructor doesn't throw, but replaces bad bytes with default - // replacement sequence. If decrypt throws, we let it exception to propagate, - // because it is an issue in the user code. - // TODO RUMM-1944 Rework that, decoding logic should be encapsulated somewhere and - // shared with EncryptedFileHandler. Should maybe inject file handler in plugin - // instead? - val decoded = try { - Base64.decode(bytes, EncryptedFileHandler.ENCODING_FLAGS) - } catch (iae: IllegalArgumentException) { - devLogger.e("Cannot decode previously saved file", iae) - null - } - - if (decoded != null) { - String(encryption.decrypt(decoded), Charsets.UTF_8) - } else { - null - } + String(content.join(ByteArray(0))) } } @@ -366,11 +345,11 @@ internal class DatadogNdkCrashHandler( internal const val ERROR_TASK_REJECTED = "Unable to schedule operation on the executor" - internal const val NDK_CRASH_REPORTS_FOLDER_NAME = "ndk_crash_reports" - private const val NDK_CRASH_REPORTS_PENDING_FOLDER_NAME = "ndk_crash_reports_intermediary" + private const val STORAGE_VERSION = 2 - internal const val DESERIALIZE_CRASH_EVENT_ERROR_MESSAGE = - "Error while trying to deserialize the ndk crash log event" + internal const val NDK_CRASH_REPORTS_FOLDER_NAME = "ndk_crash_reports_v$STORAGE_VERSION" + private const val NDK_CRASH_REPORTS_PENDING_FOLDER_NAME = + "ndk_crash_reports_intermediary_v$STORAGE_VERSION" internal fun getNdkGrantedDir(context: Context): File { return File(context.cacheDir, NDK_CRASH_REPORTS_FOLDER_NAME) diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt index e21788fa4c..4c2033dd3a 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt @@ -310,7 +310,7 @@ internal abstract class SdkFeatureTest() diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataFlusherTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataFlusherTest.kt index 3064a5f3af..9545e64602 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataFlusherTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/data/upload/DataFlusherTest.kt @@ -74,29 +74,32 @@ internal class DataFlusherTest { ) { // Given val fakeFiles = forge.aList { mock() } - val fakeBatchesAsByteArray = forge + val fakeBatches = forge .aList(fakeFiles.size) { - forge.aString() + forge + .aList { + forge.aString() + } + .map { it.toByteArray() } } - .map { it.toByteArray(Charsets.UTF_8) } whenever(mockFileOrchestrator.getFlushableFiles()).thenReturn(fakeFiles) fakeFiles.forEachIndexed { index, file -> whenever( - mockFileHandler.readData( - file, - payloadDecoration.prefixBytes, - payloadDecoration.suffixBytes, - payloadDecoration.separatorBytes - ) - ).thenReturn(fakeBatchesAsByteArray[index]) + mockFileHandler.readData(file) + ).thenReturn(fakeBatches[index]) } // When testedFlusher.flush(mockDataUploader) // Then - fakeBatchesAsByteArray.forEach { - verify(mockDataUploader).upload(it) + fakeBatches.forEach { + val expectedPayload = + payloadDecoration.prefixBytes + it.reduce { acc, bytes -> + acc + payloadDecoration.separatorBytes + bytes + } + payloadDecoration.suffixBytes + + verify(mockDataUploader).upload(expectedPayload) } } @@ -106,21 +109,19 @@ internal class DataFlusherTest { ) { // Given val fakeFiles = forge.aList { mock() } - val fakeBatchesAsByteArray = forge + val fakeBatches = forge .aList(fakeFiles.size) { - forge.aString() + forge + .aList { + forge.aString() + } + .map { it.toByteArray() } } - .map { it.toByteArray(Charsets.UTF_8) } whenever(mockFileOrchestrator.getFlushableFiles()).thenReturn(fakeFiles) fakeFiles.forEachIndexed { index, file -> whenever( - mockFileHandler.readData( - file, - payloadDecoration.prefixBytes, - payloadDecoration.suffixBytes, - payloadDecoration.separatorBytes - ) - ).thenReturn(fakeBatchesAsByteArray[index]) + mockFileHandler.readData(file) + ).thenReturn(fakeBatches[index]) } // When diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandlerTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandlerTest.kt index 6c633dbbc4..05b107395b 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandlerTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EncryptedFileHandlerTest.kt @@ -8,7 +8,6 @@ package com.datadog.android.core.internal.persistence.file import android.util.Log import com.datadog.android.log.Logger -import com.datadog.android.log.internal.utils.ERROR_WITH_TELEMETRY_LEVEL import com.datadog.android.security.Encryption import com.datadog.android.utils.config.LoggerTestConfiguration import com.datadog.android.utils.forge.Configurator @@ -16,12 +15,9 @@ import com.datadog.tools.unit.annotations.TestConfigurationsProvider import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.nhaarman.mockitokotlin2.any -import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.doAnswer import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.eq -import com.nhaarman.mockitokotlin2.isNull -import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever @@ -31,11 +27,9 @@ import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import java.io.File -import java.util.Base64 import kotlin.experimental.inv import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.RepeatedTest import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions @@ -69,7 +63,7 @@ internal class EncryptedFileHandlerTest { @BeforeEach fun setUp() { - whenever(mockFileHandlerDelegate.writeData(any(), any(), any(), anyOrNull())) doReturn true + whenever(mockFileHandlerDelegate.writeData(any(), any(), any())) doReturn true whenever(mockEncryption.encrypt(any())) doAnswer { val bytes = it.getArgument(0) @@ -81,111 +75,50 @@ internal class EncryptedFileHandlerTest { } testedFileHandler = - EncryptedFileHandler(mockEncryption, mockFileHandlerDelegate, mockInternalLogger) + EncryptedFileHandler(mockEncryption, mockFileHandlerDelegate) } // region FileHandler#writeData tests @Test - fun `𝕄 encrypt data and return true 𝕎 writeData() { separator not in Base64 set }`( + fun `𝕄 encrypt data and return true 𝕎 writeData()`( @StringForgery data: String, - @BoolForgery append: Boolean, - forge: Forge + @BoolForgery append: Boolean ) { - // Given - val separator = forge.aNonBase64Separator() - val expected = Base64.getEncoder().encode(encrypt(data.toByteArray())) - // When val result = testedFileHandler.writeData( mockFile, data.toByteArray(), - append = append, - separator = separator + append = append ) + val encryptedData = encrypt(data.toByteArray()) // Then assertThat(result).isTrue() verify(mockFileHandlerDelegate) .writeData( mockFile, - expected, - append, - separator + encryptedData, + append ) verifyZeroInteractions(mockInternalLogger) verifyZeroInteractions(logger.mockDevLogHandler) } - @Test - fun `𝕄 log internal error and return false 𝕎 writeData() { separator is in Base64 set }`( - @StringForgery data: String, - @BoolForgery append: Boolean, - forge: Forge - ) { - // Given - val separator = forge.anElementFrom(BASE_64_CHARS) - - // When - val result = testedFileHandler.writeData( - mockFile, - data.toByteArray(), - append = append, - separator = ByteArray(1) { separator.code.toByte() } - ) - - // Then - assertThat(result).isFalse() - - verify(mockInternalLogger).log( - ERROR_WITH_TELEMETRY_LEVEL, - EncryptedFileHandler.INVALID_SEPARATOR_MESSAGE - ) - verifyZeroInteractions(mockEncryption) - verifyZeroInteractions(mockFileHandlerDelegate) - } - - @Test - fun `𝕄 log internal error and return false 𝕎 writeData() { append + missing separator }`( - @StringForgery data: String - ) { - // When - val result = testedFileHandler.writeData( - mockFile, - data.toByteArray(), - append = true, - separator = null - ) - - // Then - assertThat(result).isFalse() - - verify(mockInternalLogger).log( - ERROR_WITH_TELEMETRY_LEVEL, - EncryptedFileHandler.MISSING_SEPARATOR_MESSAGE - ) - verifyZeroInteractions(mockEncryption) - verifyZeroInteractions(mockFileHandlerDelegate) - } - @Test fun `𝕄 log internal error and return false 𝕎 writeData() { bad encryption result }`( @StringForgery data: String, - @BoolForgery append: Boolean, - forge: Forge + @BoolForgery append: Boolean ) { // Given - val separator = forge.aNonBase64Separator() - whenever(mockEncryption.encrypt(data.toByteArray())) doReturn ByteArray(0) // When val result = testedFileHandler.writeData( mockFile, data.toByteArray(), - append = append, - separator = separator + append = append ) // Then @@ -204,540 +137,70 @@ internal class EncryptedFileHandlerTest { // region FileHandler#readData tests @Test - fun `𝕄 decrypt data 𝕎 readData() { single item in a file + no prefix and suffix }`( - @StringForgery data: String - ) { - // Given - val encrypted = generateEncryptedData(listOf(data.toByteArray())) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - null, - null, - null - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, null, null, null) - - // Then - assertThat(result).isEqualTo(data.toByteArray()) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { single item in a file with prefix, no suffix }`( - @StringForgery data: String, - @StringForgery prefix: String - ) { - // Given - val prefixBytes = prefix.toByteArray() - - val encrypted = generateEncryptedData(listOf(data.toByteArray()), prefix = prefixBytes) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - null, - null - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, null, null) - - // Then - assertThat(result).isEqualTo(prefixBytes + data.toByteArray()) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { single item in a file with suffix, no prefix }`( - @StringForgery data: String, - @StringForgery suffix: String - ) { - // Given - val suffixBytes = suffix.toByteArray() - - val encrypted = generateEncryptedData(listOf(data.toByteArray()), suffix = suffixBytes) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - null, - suffixBytes, - null - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, null, suffixBytes, null) - - // Then - assertThat(result).isEqualTo(data.toByteArray() + suffixBytes) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { single item in a file with suffix and prefix }`( - @StringForgery data: String, - @StringForgery prefix: String, - @StringForgery suffix: String - ) { - // Given - val prefixBytes = prefix.toByteArray() - val suffixBytes = suffix.toByteArray() - - val encrypted = generateEncryptedData( - listOf(data.toByteArray()), - prefix = prefixBytes, - suffix = suffixBytes - ) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - null - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, null) - - // Then - assertThat(result).isEqualTo(prefixBytes + data.toByteArray() + suffixBytes) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { multiple items in a file + no prefix and suffix }`( - forge: Forge - ) { - // Given - val dataItems = forge.aList { - aString() - }.map { it.toByteArray() } - - val separator = forge.aNonBase64Separator() - - val encrypted = generateEncryptedData(dataItems, separator = separator) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - null, - null, - separator - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, null, null, separator) - - // Then - assertThat(result).isEqualTo( - dataItems.join(separator) - ) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { multiple items in a file with prefix, no suffix }`( - @StringForgery prefix: String, - forge: Forge - ) { - // Given - val dataItems = forge.aList { - aString() - }.map { it.toByteArray() } - - val prefixBytes = prefix.toByteArray() - val separator = forge.aNonBase64Separator() - - val encrypted = - generateEncryptedData(dataItems, prefix = prefixBytes, separator = separator) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - null, - separator - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, null, separator) - - // Then - assertThat(result).isEqualTo(prefixBytes + dataItems.join(separator)) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { multiple items in a file with suffix, no prefix }`( - @StringForgery suffix: String, + fun `𝕄 decrypt data 𝕎 readData()`( forge: Forge ) { // Given - val dataItems = forge.aList { - aString() - }.map { it.toByteArray() } - - val suffixBytes = suffix.toByteArray() - val separator = forge.aNonBase64Separator() - - val encrypted = - generateEncryptedData(dataItems, suffix = suffixBytes, separator = separator) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - null, - suffixBytes, - separator - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, null, suffixBytes, separator) - - // Then - assertThat(result).isEqualTo(dataItems.join(separator) + suffixBytes) - } - - @Test - fun `𝕄 decrypt data 𝕎 readData() { multiple items in a file with suffix and prefix }`( - @StringForgery prefix: String, - @StringForgery suffix: String, - forge: Forge - ) { - // Given - val dataItems = forge.aList { - aString() - }.map { it.toByteArray() } - - val prefixBytes = prefix.toByteArray() - val suffixBytes = suffix.toByteArray() - val separator = forge.aNonBase64Separator() - - val encrypted = - generateEncryptedData( - dataItems, - suffix = suffixBytes, - prefix = prefixBytes, - separator = separator - ) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - separator - ) - ) doReturn encrypted - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, separator) - - // Then - assertThat(result).isEqualTo(prefixBytes + dataItems.join(separator) + suffixBytes) - } - - @Test - fun `𝕄 log internal + dev error 𝕎 readData() { cannot decode Base64, file with single item }`( - @StringForgery prefix: String, - @StringForgery suffix: String, - forge: Forge - ) { - // Given - val decodingException = IllegalArgumentException() - testedFileHandler = EncryptedFileHandler( - mockEncryption, - mockFileHandlerDelegate, - mockInternalLogger, - base64Decoder = { throw decodingException } - ) - - val prefixBytes = forge.aNullable { prefix.toByteArray() } - val suffixBytes = forge.aNullable { suffix.toByteArray() } - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - null - ) - ) doReturn decorate(forge.aString().toByteArray(), prefixBytes, suffixBytes) - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, null) - - // Then - assertThat(result).isEqualTo( - (prefixBytes ?: EMPTY_BYTE_ARRAY) + (suffixBytes ?: EMPTY_BYTE_ARRAY) - ) - - verify(mockInternalLogger).e( - EncryptedFileHandler.BASE64_DECODING_ERROR_MESSAGE, - decodingException - ) - verify(logger.mockDevLogHandler).handleLog( - Log.ERROR, - EncryptedFileHandler.BASE64_DECODING_ERROR_MESSAGE, - decodingException - ) - - verifyZeroInteractions(mockEncryption) - } - - @Test - fun `𝕄 log internal + dev error 𝕎 readData() { cannot decode Base64, file with many items }`( - @StringForgery prefix: String, - @StringForgery suffix: String, - forge: Forge - ) { - // Given - val dataItems = forge.aList { aString() }.map { it.toByteArray() }.distinct() - - val prefixBytes = forge.aNullable { prefix.toByteArray() } - val suffixBytes = forge.aNullable { suffix.toByteArray() } - val separator = forge.aNonBase64Separator() - - val encryptedItems = dataItems.map { Base64.getEncoder().encode(encrypt(it)) } - val badItemIndex = forge.anInt(min = 0, max = encryptedItems.size) - val badItem = encryptedItems[badItemIndex] - - val encryptedData = decorate(encryptedItems.join(separator), prefixBytes, suffixBytes) - - val decodingException = IllegalArgumentException() - - var failCounter = 0 - testedFileHandler = EncryptedFileHandler( - mockEncryption, - mockFileHandlerDelegate, - mockInternalLogger, - base64Decoder = { - @Suppress("ReplaceArrayEqualityOpWithArraysEquals") - if (it.contentEquals(badItem)) { - failCounter++ - throw decodingException - } else { - Base64.getDecoder().decode(it) - } - } - ) - - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - separator - ) - ) doReturn encryptedData - - // When - val result = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, separator) - - // Then - val expected = dataItems.toMutableList().apply { - this[badItemIndex] = EMPTY_BYTE_ARRAY - }.join(separator) - - assertThat(result).isEqualTo(decorate(expected, prefixBytes, suffixBytes)) - - verify(mockInternalLogger, times(failCounter)).e( - EncryptedFileHandler.BASE64_DECODING_ERROR_MESSAGE, - decodingException - ) - verify(logger.mockDevLogHandler, times(failCounter)).handleLog( - Log.ERROR, - EncryptedFileHandler.BASE64_DECODING_ERROR_MESSAGE, - decodingException - ) - } - - @Test - fun `𝕄 log internal + dev error 𝕎 readData() { data is less than prefix or suffix size }`( - forge: Forge - ) { - // Given - var prefixBytes = forge.aNullable { forge.aString(forge.aSmallInt()).toByteArray() } - var suffixBytes = forge.aNullable { forge.aString(forge.aSmallInt()).toByteArray() } - - if (prefixBytes == null) { - suffixBytes = forge.aString(forge.aSmallInt()).toByteArray() - } else if (suffixBytes == null) { - prefixBytes = forge.aString(forge.aSmallInt()).toByteArray() + val events = forge.aList { + forge.aString().toByteArray() } whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - null - ) - ) doReturn forge.aString().toByteArray().take( - forge.anInt( - 0, - (prefixBytes?.size ?: 0) + (suffixBytes?.size ?: 0) - ) - ).toByteArray() + mockFileHandlerDelegate.readData(mockFile) + ) doReturn events.map { encrypt(it) } // When - val result = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, null) - - // Then - assertThat(result).isEqualTo(decorate(EMPTY_BYTE_ARRAY, prefixBytes, suffixBytes)) - - verify(mockInternalLogger).e(EncryptedFileHandler.BAD_DATA_READ_MESSAGE) - verify(logger.mockDevLogHandler) - .handleLog(Log.ERROR, EncryptedFileHandler.BAD_DATA_READ_MESSAGE) - - verifyZeroInteractions(mockEncryption) - } - - @Test - fun `𝕄 log internal error and return empty arr 𝕎 readData() { separator is in Base64 set }`( - @StringForgery prefix: String, - @StringForgery suffix: String, - forge: Forge - ) { - // When - val result = testedFileHandler.readData( - mockFile, - forge.aNullable { prefix.toByteArray() }, - forge.aNullable { suffix.toByteArray() }, - separator = ByteArray(1) { forge.anElementFrom(BASE_64_CHARS).code.toByte() } - ) + val result = testedFileHandler.readData(mockFile) // Then - assertThat(result).isEqualTo(EMPTY_BYTE_ARRAY) - - verify(mockInternalLogger).log( - ERROR_WITH_TELEMETRY_LEVEL, - EncryptedFileHandler.INVALID_SEPARATOR_MESSAGE - ) - verifyZeroInteractions(mockEncryption) - verifyZeroInteractions(mockFileHandlerDelegate) + assertThat(result).containsExactlyElementsOf(events) } // endregion - @RepeatedTest(4) - fun `𝕄 return valid data 𝕎 writeData() + readData() { single item file }`( - @StringForgery prefix: String, - @StringForgery suffix: String, - @StringForgery data: String, - forge: Forge - ) { - // Given - val dataBytes = data.toByteArray() - val prefixBytes = forge.aNullable { prefix.toByteArray() } - val suffixBytes = forge.aNullable { suffix.toByteArray() } - - val storage = mutableListOf() - - whenever( - mockFileHandlerDelegate.writeData( - eq(mockFile), - any(), - eq(false), - isNull() - ) - ) doAnswer { - it.getArgument(1).forEach { byte -> - storage.add(byte) - } - true - } + // region writeData + readData - whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - null - ) - ) doAnswer { decorate(storage.toByteArray(), prefixBytes, suffixBytes) } - - // When - val writeResult = testedFileHandler.writeData(mockFile, dataBytes, false, null) - val readResult = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, null) - - // Then - assertThat(writeResult).isTrue() - assertThat(readResult).isEqualTo(decorate(data.toByteArray(), prefixBytes, suffixBytes)) - - verifyZeroInteractions(mockInternalLogger) - verifyZeroInteractions(logger.mockDevLogHandler) - } - - @RepeatedTest(4) - fun `𝕄 return valid data 𝕎 writeData() + readData() { multiple items file }`( - @StringForgery prefix: String, - @StringForgery suffix: String, + @Test + fun `𝕄 return valid data 𝕎 writeData() + readData()`( forge: Forge ) { // Given - val dataItems = forge.aList { aString() }.map { it.toByteArray() } - val prefixBytes = forge.aNullable { prefix.toByteArray() } - val suffixBytes = forge.aNullable { suffix.toByteArray() } - val separator = forge.aNonBase64Separator() + val events = forge.aList { forge.aString().toByteArray() } - val storage = mutableListOf() + val storage = mutableListOf() whenever( mockFileHandlerDelegate.writeData( eq(mockFile), any(), - eq(true), - eq(separator) + eq(true) ) ) doAnswer { - it.getArgument(1).forEach { byte -> - storage.add(byte) - } - it.getArgument(3).forEach { byte -> - storage.add(byte) - } + storage.add(it.getArgument(1)) true } whenever( - mockFileHandlerDelegate.readData( - mockFile, - prefixBytes, - suffixBytes, - separator - ) - ) doAnswer { decorate(storage.toByteArray(), prefixBytes, suffixBytes) } + mockFileHandlerDelegate.readData(mockFile) + ) doAnswer { storage } // When var writeResult = true - for (item in dataItems) { - writeResult = - writeResult and testedFileHandler.writeData(mockFile, item, true, separator) + events.forEach { + writeResult = writeResult && testedFileHandler.writeData(mockFile, it, true) } - val readResult = testedFileHandler.readData(mockFile, prefixBytes, suffixBytes, separator) + val readResult = testedFileHandler.readData(mockFile) // Then assertThat(writeResult).isTrue() - assertThat(readResult).isEqualTo( - decorate( - dataItems.join(separator), - prefixBytes, - suffixBytes - ) - ) + assertThat(readResult).containsExactlyElementsOf(events) verifyZeroInteractions(mockInternalLogger) verifyZeroInteractions(logger.mockDevLogHandler) } + // endregion + // region private // this is valid encryption-decryption pair, after the round we will get the original data @@ -749,58 +212,9 @@ internal class EncryptedFileHandlerTest { return data.map { it.inv() }.toByteArray() } - private fun generateEncryptedData( - data: List, - prefix: ByteArray? = null, - suffix: ByteArray? = null, - separator: ByteArray? = null - ): ByteArray { - val encryptedItems = data - .map { - Base64.getEncoder().encode(encrypt(it)) - } - .join(separator ?: EMPTY_BYTE_ARRAY) - - return decorate(encryptedItems, prefix, suffix) - } - - private fun decorate(data: ByteArray, prefix: ByteArray?, suffix: ByteArray?): ByteArray { - return (prefix ?: EMPTY_BYTE_ARRAY) + data + (suffix ?: EMPTY_BYTE_ARRAY) - } - - private fun List.join(separator: ByteArray): ByteArray { - return filter { it.isNotEmpty() } - .flatMap { - listOf(it, separator) - } - .dropLast(1) - .flatMap { it.toList() } - .toByteArray() - } - - private fun isBase64Char(separator: String): Boolean { - if (separator.length != 1) { - return false - } - - val separatorChar = separator.elementAt(0) - return separatorChar in BASE_64_CHARS - } - - private fun Forge.aNonBase64Separator(): ByteArray { - var separator = aString() - while (isBase64Char(separator)) { - separator = aString() - } - return separator.toByteArray() - } - // endregion companion object { - private val EMPTY_BYTE_ARRAY = ByteArray(0) - private val BASE_64_CHARS = - (('A'..'Z') + ('a'..'z') + ('0'..'9') + arrayOf('+', '/', '=')).toSet() val logger = LoggerTestConfiguration() diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EventMetaTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EventMetaTest.kt new file mode 100644 index 0000000000..9d54989d94 --- /dev/null +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/EventMetaTest.kt @@ -0,0 +1,60 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.core.internal.persistence.file + +import com.google.gson.JsonParseException +import fr.xgouchet.elmyr.Forge +import fr.xgouchet.elmyr.annotation.IntForgery +import fr.xgouchet.elmyr.annotation.StringForgery +import fr.xgouchet.elmyr.junit5.ForgeExtension +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.extension.Extensions + +@Extensions( + ExtendWith(ForgeExtension::class) +) +internal class EventMetaTest { + + @Test + fun `𝕄 return original value 𝕎 asBytes + fromBytes()`( + @IntForgery(min = 0) eventSize: Int + ) { + // Given + val originalMeta = EventMeta(eventSize) + + // When + val restoredMeta = EventMeta.fromBytes(originalMeta.asBytes) + + // Then + assertThat(restoredMeta).isEqualTo(originalMeta) + } + + @Test + fun `𝕄 throw JsonParseException 𝕎 fromBytes() { bad meta bytes }`( + @StringForgery metaBytes: String + ) { + assertThrows { EventMeta.fromBytes(metaBytes.toByteArray()) } + } + + @Test + fun `𝕄 throw JsonParseException 𝕎 fromBytes() { unexpected json }`( + forge: Forge + ) { + // Given + val metaBytes = forge.anElementFrom( + "[]", + "{}", + "{\"ev_size\": \"string\"}" + ).toByteArray() + + // When + Then + assertThrows { EventMeta.fromBytes(metaBytes) } + } +} diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestratorTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestratorTest.kt index d58899c610..5387b15c2e 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestratorTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/FeatureFileOrchestratorTest.kt @@ -81,7 +81,7 @@ internal class FeatureFileOrchestratorTest { assertThat(orchestrator.pendingOrchestrator) .isInstanceOf(BatchFileOrchestrator::class.java) assertThat(orchestrator.pendingOrchestrator.getRootDir()) - .isEqualTo(File(appContext.fakeCacheDir, "dd-$fakeFeatureName-pending-v1")) + .isEqualTo(File(appContext.fakeCacheDir, "dd-$fakeFeatureName-pending-v2")) } @Test @@ -101,7 +101,7 @@ internal class FeatureFileOrchestratorTest { assertThat(orchestrator.grantedOrchestrator) .isInstanceOf(BatchFileOrchestrator::class.java) assertThat(orchestrator.grantedOrchestrator.getRootDir()) - .isEqualTo(File(appContext.fakeCacheDir, "dd-$fakeFeatureName-v1")) + .isEqualTo(File(appContext.fakeCacheDir, "dd-$fakeFeatureName-v2")) } @Test diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReaderTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReaderTest.kt index 3251839724..67ceed9b31 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReaderTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataReaderTest.kt @@ -21,6 +21,7 @@ import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever +import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.annotation.Forgery import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration @@ -75,19 +76,14 @@ internal class BatchFileDataReaderTest { @Test fun `𝕄 read batch 𝕎 lockAndReadNext()`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData // When @@ -96,7 +92,13 @@ internal class BatchFileDataReaderTest { // Then checkNotNull(result) assertThat(result.id).isEqualTo(file.name) - assertThat(result.data).isEqualTo(readData) + assertThat(result.data).isEqualTo( + readData.join( + fakeDecoration.separatorBytes, + fakeDecoration.prefixBytes, + fakeDecoration.suffixBytes + ) + ) } @Test @@ -117,19 +119,14 @@ internal class BatchFileDataReaderTest { @Test fun `𝕄 read batch twice 𝕎 lockAndReadNext() + release() + lockAndReadNext()`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData // When @@ -141,25 +138,26 @@ internal class BatchFileDataReaderTest { // Then checkNotNull(result2) assertThat(result2.id).isEqualTo(file.name) - assertThat(result2.data).isEqualTo(readData) + assertThat(result2.data).isEqualTo( + readData.join( + fakeDecoration.separatorBytes, + fakeDecoration.prefixBytes, + fakeDecoration.suffixBytes + ) + ) verify(mockFileHandler, never()).delete(any()) } @Test fun `𝕄 read batch twice 𝕎 lockAndReadNext() + release() + lockAndReadNext() {multithreaded}`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData val countDownLatch = CountDownLatch(2) @@ -182,25 +180,26 @@ internal class BatchFileDataReaderTest { val result2 = threadResult checkNotNull(result2) assertThat(result2.id).isEqualTo(file.name) - assertThat(result2.data).isEqualTo(readData) + assertThat(result2.data).isEqualTo( + readData.join( + fakeDecoration.separatorBytes, + fakeDecoration.prefixBytes, + fakeDecoration.suffixBytes + ) + ) verify(mockFileHandler, never()).delete(any()) } @Test fun `𝕄 read batch once 𝕎 lockAndReadNext() + release() {diff} + lockAndReadNext()`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData // When @@ -211,28 +210,34 @@ internal class BatchFileDataReaderTest { // Then checkNotNull(result1) assertThat(result1.id).isEqualTo(file.name) - assertThat(result1.data).isEqualTo(readData) + assertThat(result1.data).isEqualTo( + readData.join( + fakeDecoration.separatorBytes, + fakeDecoration.prefixBytes, + fakeDecoration.suffixBytes + ) + ) assertThat(result2).isNull() verify(mockFileHandler, never()).delete(any()) } @Test - fun `𝕄 read and release mutliple batches 𝕎 lockAndReadNext() + release() { multithreaded }`( - @StringForgery data: String, + fun `𝕄 read and release multiple batches 𝕎 lockAndReadNext() + release() { multithreaded }`( @Forgery file1: File, @Forgery file2: File, @Forgery file3: File, - @Forgery file4: File + @Forgery file4: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } val files = listOf(file1, file2, file3, file4) val expectedIds = files.map { it.name } whenever(mockOrchestrator.getReadableFile(any())) doAnswer { invocation -> val set = invocation.getArgument>(0) files.first { it.name !in set } } - whenever(mockFileHandler.readData(any(), any(), any(), any())) doReturn readData + whenever(mockFileHandler.readData(any())) doReturn readData val countDownLatch = CountDownLatch(4) // When @@ -280,19 +285,14 @@ internal class BatchFileDataReaderTest { @Test fun `𝕄 delete underlying file 𝕎 lockAndReadNext() + dropBatch()`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData whenever(mockFileHandler.delete(file)) doReturn true @@ -307,19 +307,14 @@ internal class BatchFileDataReaderTest { @Test fun `𝕄 warn 𝕎 lockAndReadNext() + dropBatch() {delete fails}`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData whenever(mockFileHandler.delete(file)) doReturn false @@ -359,20 +354,15 @@ internal class BatchFileDataReaderTest { @Test fun `𝕄 delete underlying file 𝕎 lockAndReadNext() + dropAll()`( - @StringForgery data: String, - @Forgery file: File + @Forgery file: File, + forge: Forge ) { // Given - val readData = data.toByteArray(Charsets.UTF_8) + val readData = forge.aList { aString().toByteArray(Charsets.UTF_8) } whenever(mockOrchestrator.getReadableFile(emptySet())) doReturn file whenever(mockOrchestrator.getAllFiles()) doReturn emptyList() whenever( - mockFileHandler.readData( - file, - fakeDecoration.prefixBytes, - fakeDecoration.suffixBytes, - fakeDecoration.separatorBytes - ) + mockFileHandler.readData(file) ) doReturn readData whenever(mockFileHandler.delete(file)) doReturn true @@ -407,4 +397,18 @@ internal class BatchFileDataReaderTest { } // endregion + + // region private + + private fun List.join( + separator: ByteArray, + prefix: ByteArray, + suffix: ByteArray + ): ByteArray { + return prefix + this.reduce { acc, bytes -> + acc + separator + bytes + } + suffix + } + + // endregion } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriterTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriterTest.kt index 4a90be69b8..10194724cb 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriterTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileDataWriterTest.kt @@ -129,8 +129,7 @@ internal class BatchFileDataWriterTest { .writeData( file, serialized, - append = true, - separator = fakeDecoration.separatorBytes + append = true ) } @@ -151,8 +150,7 @@ internal class BatchFileDataWriterTest { .writeData( same(file), capture(), - append = eq(true), - separator = eq(fakeDecoration.separatorBytes) + append = eq(true) ) assertThat(allValues) .containsExactlyElementsOf( @@ -169,7 +167,7 @@ internal class BatchFileDataWriterTest { @Forgery file: File ) { // Given - whenever(mockFileHandler.writeData(any(), any(), any(), any())) doReturn true + whenever(mockFileHandler.writeData(any(), any(), any())) doReturn true whenever(mockOrchestrator.getWritableFile(any())) doReturn file // When @@ -186,7 +184,7 @@ internal class BatchFileDataWriterTest { @Forgery file: File ) { // Given - whenever(mockFileHandler.writeData(any(), any(), any(), any())) doReturn false + whenever(mockFileHandler.writeData(any(), any(), any())) doReturn false whenever(mockOrchestrator.getWritableFile(any())) doReturn file // When diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandlerTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandlerTest.kt index 42f96142be..09649a44c9 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandlerTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileHandlerTest.kt @@ -8,14 +8,20 @@ package com.datadog.android.core.internal.persistence.file.batch import android.util.Log import com.datadog.android.core.internal.persistence.file.EncryptedFileHandler +import com.datadog.android.core.internal.persistence.file.EventMeta import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.log.Logger -import com.datadog.android.log.internal.logger.LogHandler import com.datadog.android.log.internal.utils.ERROR_WITH_TELEMETRY_LEVEL import com.datadog.android.security.Encryption +import com.datadog.android.utils.config.LoggerTestConfiguration import com.datadog.android.utils.forge.Configurator +import com.datadog.tools.unit.annotations.TestConfigurationsProvider +import com.datadog.tools.unit.extensions.TestConfigurationExtension +import com.datadog.tools.unit.extensions.config.TestConfiguration +import com.google.gson.JsonParseException import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.eq +import com.nhaarman.mockitokotlin2.isA import com.nhaarman.mockitokotlin2.isNull import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify @@ -34,14 +40,14 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions import org.junit.jupiter.api.io.TempDir -import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings import org.mockito.quality.Strictness @Extensions( ExtendWith(MockitoExtension::class), - ExtendWith(ForgeExtension::class) + ExtendWith(ForgeExtension::class), + ExtendWith(TestConfigurationExtension::class) ) @ForgeConfiguration(Configurator::class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -49,9 +55,6 @@ internal class BatchFileHandlerTest { lateinit var testedFileHandler: FileHandler - @Mock - lateinit var mockLogHandler: LogHandler - @StringForgery(regex = "([a-z]+)-([a-z]+)") lateinit var fakeSrcDirName: String @@ -68,103 +71,57 @@ internal class BatchFileHandlerTest { fun `set up`() { fakeSrcDir = File(fakeRootDirectory, fakeSrcDirName) fakeDstDir = File(fakeRootDirectory, fakeDstDirName) - testedFileHandler = BatchFileHandler(Logger(mockLogHandler)) + testedFileHandler = BatchFileHandler(Logger(logger.mockSdkLogHandler)) } // region writeData @Test - fun `𝕄 write data in empty file 𝕎 writeData() {append=false, separator=null}`( + fun `𝕄 write data in empty file 𝕎 writeData() {append=false}`( @StringForgery fileName: String, @StringForgery content: String ) { // Given val file = File(fakeRootDirectory, fileName) file.createNewFile() + val contentBytes = content.toByteArray() // When val result = testedFileHandler.writeData( file, - content.toByteArray(), - append = false, - separator = null + contentBytes, + append = false ) // Then assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) + assertThat(file).exists().hasBinaryContent(headerBytes(contentBytes) + contentBytes) } @Test - fun `𝕄 write data in empty file 𝕎 writeData() {append=true, separator=null}`( + fun `𝕄 write data in empty file 𝕎 writeData() {append=true}`( @StringForgery fileName: String, @StringForgery content: String ) { // Given val file = File(fakeRootDirectory, fileName) file.createNewFile() + val contentBytes = content.toByteArray() // When val result = testedFileHandler.writeData( file, - content.toByteArray(), - append = false, - separator = null - ) - - // Then - assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) - } - - @Test - fun `𝕄 write data in empty file 𝕎 writeData() {append=false, separator=non null}`( - @StringForgery fileName: String, - @StringForgery content: String, - @StringForgery separator: String - ) { - // Given - val file = File(fakeRootDirectory, fileName) - file.createNewFile() - - // When - val result = testedFileHandler.writeData( - file, - content.toByteArray(), - append = false, - separator = separator.toByteArray() - ) - - // Then - assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) - } - - @Test - fun `𝕄 write data in empty file 𝕎 writeData() {append=true, separator=non null }`( - @StringForgery fileName: String, - @StringForgery content: String, - @StringForgery separator: String - ) { - // Given - val file = File(fakeRootDirectory, fileName) - file.createNewFile() - - // When - val result = testedFileHandler.writeData( - file, - content.toByteArray(), - append = false, - separator = separator.toByteArray() + contentBytes, + append = false ) // Then assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) + assertThat(file).exists().hasBinaryContent(headerBytes(contentBytes) + contentBytes) } @Test - fun `𝕄 overwrite data in non empty file 𝕎 writeData() {append=false, separator=null}`( + fun `𝕄 overwrite data in non empty file 𝕎 writeData() {append=false}`( @StringForgery fileName: String, @StringForgery previousContent: String, @StringForgery content: String @@ -172,114 +129,98 @@ internal class BatchFileHandlerTest { // Given val file = File(fakeRootDirectory, fileName) file.writeText(previousContent) + val contentBytes = content.toByteArray() // When val result = testedFileHandler.writeData( file, - content.toByteArray(), - append = false, - separator = null + contentBytes, + append = false ) // Then assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) + assertThat(file).exists().hasBinaryContent(headerBytes(contentBytes) + contentBytes) } @Test - fun `𝕄 overwrite data in non empty file 𝕎 writeData() {append=false, separator=non null}`( - @StringForgery fileName: String, - @StringForgery previousContent: String, - @StringForgery content: String, - @StringForgery separator: String - ) { - // Given - val file = File(fakeRootDirectory, fileName) - file.writeText(previousContent) - - // When - val result = testedFileHandler.writeData( - file, - content.toByteArray(), - append = false, - separator = separator.toByteArray() - ) - - // Then - assertThat(result).isTrue() - assertThat(file).exists().hasContent(content) - } - - @Test - fun `𝕄 append data in non empty file 𝕎 writeData() {append=true, separator=null}`( + fun `𝕄 append data in non empty file 𝕎 writeData() {append=true}`( @StringForgery fileName: String, @StringForgery previousContent: String, @StringForgery content: String ) { // Given val file = File(fakeRootDirectory, fileName) - file.writeText(previousContent) + val previousData = previousContent.toByteArray() + file.writeBytes(headerBytes(previousData) + previousData) + val contentBytes = content.toByteArray() // When val result = testedFileHandler.writeData( file, - content.toByteArray(), - append = true, - separator = null + contentBytes, + append = true ) // Then assertThat(result).isTrue() - assertThat(file).exists().hasContent(previousContent + content) + assertThat(file).exists() + .hasBinaryContent( + headerBytes(previousData) + previousData + + headerBytes(contentBytes) + contentBytes + ) } @Test - fun `𝕄 append data in non empty file 𝕎 writeData() {append=true, separator=non null}`( + fun `𝕄 return false and warn 𝕎 writeData() {parent dir does not exist}`( @StringForgery fileName: String, - @StringForgery previousContent: String, @StringForgery content: String, - @StringForgery separator: String + @BoolForgery append: Boolean ) { // Given - val file = File(fakeRootDirectory, fileName) - file.writeText(previousContent) + assumeFalse(fakeSrcDir.exists()) + val file = File(fakeSrcDir, fileName) // When val result = testedFileHandler.writeData( file, content.toByteArray(), - append = true, - separator = separator.toByteArray() + append = append ) // Then - assertThat(result).isTrue() - assertThat(file).exists().hasContent(previousContent + separator + content) + assertThat(result).isFalse() + assertThat(file).doesNotExist() + verify(logger.mockSdkLogHandler).handleLog( + eq(ERROR_WITH_TELEMETRY_LEVEL), + eq(BatchFileHandler.ERROR_WRITE.format(Locale.US, file.path)), + any(), + eq(emptyMap()), + eq(emptySet()), + isNull() + ) } @Test - fun `𝕄 return false and warn 𝕎 writeData() {parent dir does not exist}`( + fun `𝕄 return false and warn 𝕎 writeData() {file is not file}`( @StringForgery fileName: String, @StringForgery content: String, - @BoolForgery append: Boolean, - @StringForgery separator: String + @BoolForgery append: Boolean ) { // Given - assumeFalse(fakeSrcDir.exists()) - val file = File(fakeSrcDir, fileName) + val file = File(fakeRootDirectory, fileName) + file.mkdirs() // When val result = testedFileHandler.writeData( file, content.toByteArray(), - append = append, - separator = separator.toByteArray() + append = append ) // Then assertThat(result).isFalse() - assertThat(file).doesNotExist() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( eq(ERROR_WITH_TELEMETRY_LEVEL), eq(BatchFileHandler.ERROR_WRITE.format(Locale.US, file.path)), any(), @@ -290,30 +231,35 @@ internal class BatchFileHandlerTest { } @Test - fun `𝕄 return false and warn 𝕎 writeData() {file is not file}`( + fun `𝕄 return false and warn 𝕎 writeData() { meta is too big }`( @StringForgery fileName: String, @StringForgery content: String, @BoolForgery append: Boolean, - @StringForgery separator: String + forge: Forge ) { // Given + testedFileHandler = BatchFileHandler( + Logger(logger.mockSdkLogHandler), + metaGenerator = { + ByteArray(BatchFileHandler.MAX_META_SIZE_BYTES + forge.aTinyInt()) + } + ) val file = File(fakeRootDirectory, fileName) - file.mkdirs() + file.createNewFile() // When val result = testedFileHandler.writeData( file, content.toByteArray(), - append = append, - separator = separator.toByteArray() + append = append ) // Then assertThat(result).isFalse() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( eq(ERROR_WITH_TELEMETRY_LEVEL), eq(BatchFileHandler.ERROR_WRITE.format(Locale.US, file.path)), - any(), + isA(), eq(emptyMap()), eq(emptySet()), isNull() @@ -325,29 +271,20 @@ internal class BatchFileHandlerTest { // region readData @Test - fun `𝕄 return empty ByteArray and warn 𝕎 readData() {file does not exist}`( - @StringForgery fileName: String, - @StringForgery prefix: String, - @StringForgery suffix: String, - @StringForgery separator: String, - forge: Forge + fun `𝕄 return empty list and warn 𝕎 readData() {file does not exist}`( + @StringForgery fileName: String ) { // Given val file = File(fakeRootDirectory, fileName) assumeFalse(file.exists()) // When - val result = testedFileHandler.readData( - file, - prefix.toByteArray(), - suffix.toByteArray(), - forge.aNullable { separator.toByteArray() } - ) + val result = testedFileHandler.readData(file) // Then assertThat(result).isEmpty() assertThat(file).doesNotExist() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( eq(ERROR_WITH_TELEMETRY_LEVEL), eq(BatchFileHandler.ERROR_READ.format(Locale.US, file.path)), any(), @@ -358,28 +295,19 @@ internal class BatchFileHandlerTest { } @Test - fun `𝕄 return empty ByteArray and warn 𝕎 readData() {file is not file}`( - @StringForgery fileName: String, - @StringForgery prefix: String, - @StringForgery suffix: String, - @StringForgery separator: String, - forge: Forge + fun `𝕄 return empty list and warn 𝕎 readData() {file is not file}`( + @StringForgery fileName: String ) { // Given val file = File(fakeRootDirectory, fileName) assumeFalse(file.exists()) // When - val result = testedFileHandler.readData( - file, - prefix.toByteArray(), - suffix.toByteArray(), - forge.aNullable { separator.toByteArray() } - ) + val result = testedFileHandler.readData(file) // Then assertThat(result).isEmpty() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( eq(ERROR_WITH_TELEMETRY_LEVEL), eq(BatchFileHandler.ERROR_READ.format(Locale.US, file.path)), any(), @@ -390,79 +318,214 @@ internal class BatchFileHandlerTest { } @Test - fun `𝕄 return file content 𝕎 readData() {postfix and suffix are null}`( + fun `𝕄 return empty list and warn user 𝕎 readData() { corrupted data }`( @StringForgery fileName: String, - @StringForgery content: String, - @StringForgery separator: String, - forge: Forge + @StringForgery content: String ) { // Given val file = File(fakeRootDirectory, fileName) - file.writeText(content) + file.writeBytes(content.toByteArray()) // When - val result = testedFileHandler.readData( - file, - null, - null, - forge.aNullable { separator.toByteArray() } - ) + val result = testedFileHandler.readData(file) // Then - assertThat(result).isEqualTo(content.toByteArray(Charsets.UTF_8)) + assertThat(result).isEmpty() + verify(logger.mockDevLogHandler).handleLog( + Log.ERROR, + BatchFileHandler.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) + ) + verify(logger.mockSdkLogHandler).handleLog( + ERROR_WITH_TELEMETRY_LEVEL, + BatchFileHandler.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) + ) } @Test - fun `𝕄 return decorated content 𝕎 readData() {postfix and suffix are not null}`( + fun `𝕄 return valid events read so far and warn 𝕎 readData() { stream cutoff at meta block }`( @StringForgery fileName: String, - @StringForgery content: String, - @StringForgery prefix: String, - @StringForgery suffix: String, - @StringForgery separator: String, forge: Forge ) { // Given val file = File(fakeRootDirectory, fileName) - file.writeText(content) + val events = forge.aList { + aString().toByteArray() + } + + file.writeBytes( + events.mapIndexed { index, bytes -> + if (index == events.lastIndex) { + headerBytes(bytes) + .let { it.take(forge.anInt(min = 2, max = it.size - 1)) } + .toByteArray() + } else { + headerBytes(bytes) + bytes + } + }.reduce { acc, bytes -> acc + bytes } + ) // When - val result = testedFileHandler.readData( - file, - prefix.toByteArray(), - suffix.toByteArray(), - forge.aNullable { separator.toByteArray() } + val result = testedFileHandler.readData(file) + + // Then + assertThat(result).containsExactlyElementsOf(events.take(events.size - 1)) + } + + @Test + fun `𝕄 return valid events read so far and warn 𝕎 readData() { malformed meta }`( + @StringForgery fileName: String, + forge: Forge + ) { + // Given + val file = File(fakeRootDirectory, fileName) + val events = forge.aList { + aString().toByteArray() + } + + file.writeBytes( + events.map { + headerBytes(it) + it + }.reduce { acc, bytes -> acc + bytes } + ) + + val malformedMetaIndex = forge.anInt(min = 0, max = events.size) + testedFileHandler = BatchFileHandler( + Logger(logger.mockSdkLogHandler), + metaParser = object : (ByteArray) -> EventMeta { + var invocations = 0 + + override fun invoke(metaBytes: ByteArray): EventMeta { + return if (invocations == malformedMetaIndex) { + throw JsonParseException(forge.aString()) + } else { + invocations++ + EventMeta.fromBytes(metaBytes) + } + } + } ) + // When + val result = testedFileHandler.readData(file) + // Then - assertThat(result).isEqualTo( - (prefix + content + suffix).toByteArray(Charsets.UTF_8) + assertThat(result).containsExactlyElementsOf(events.take(malformedMetaIndex)) + + verify(logger.mockSdkLogHandler).handleLog( + eq(Log.ERROR), + eq(BatchFileHandler.ERROR_FAILED_META_PARSE), + isA(), + eq(emptyMap()), + eq(emptySet()), + isNull() ) } @Test - fun `𝕄 return decoration only 𝕎 readData() {empty file, postfix and suffix are not null}`( + fun `𝕄 return valid events read so far and warn 𝕎 readData() {stream cutoff at event block}`( @StringForgery fileName: String, - @StringForgery prefix: String, - @StringForgery suffix: String, - @StringForgery separator: String, forge: Forge ) { // Given val file = File(fakeRootDirectory, fileName) - file.createNewFile() + val events = forge.aList { + aString().toByteArray() + } - // When - val result = testedFileHandler.readData( - file, - prefix.toByteArray(), - suffix.toByteArray(), - forge.aNullable { separator.toByteArray() } + file.writeBytes( + events.mapIndexed { index, bytes -> + headerBytes(bytes) + if (index == events.lastIndex) { + bytes.let { it.take(forge.anInt(min = 0, max = it.size - 1)) }.toByteArray() + } else { + bytes + } + }.reduce { acc, bytes -> acc + bytes } ) + // When + val result = testedFileHandler.readData(file) + // Then - assertThat(result).isEqualTo( - (prefix + suffix).toByteArray(Charsets.UTF_8) - ) + assertThat(result).containsExactlyElementsOf(events.take(events.size - 1)) + } + + @Test + fun `𝕄 return file content 𝕎 readData() { single event }`( + @StringForgery fileName: String, + @StringForgery event: String + ) { + // Given + val file = File(fakeRootDirectory, fileName) + val eventBytes = event.toByteArray() + file.writeBytes(headerBytes(eventBytes) + eventBytes) + + // When + val result = testedFileHandler.readData(file) + + // Then + assertThat(result).containsExactlyElementsOf(listOf(eventBytes)) + } + + @Test + fun `𝕄 return file content 𝕎 readData() { multiple events }`( + @StringForgery fileName: String, + forge: Forge + ) { + // Given + val file = File(fakeRootDirectory, fileName) + val events = forge.aList { + aString().toByteArray() + } + file.writeBytes(events.map { headerBytes(it) + it }.reduce { acc, bytes -> acc + bytes }) + + // When + val result = testedFileHandler.readData(file) + + // Then + assertThat(result).containsExactlyElementsOf(events) + } + + // endregion + + // region writeData + readData + + @Test + fun `𝕄 return file content 𝕎 writeData + readData() { append = false }`( + @StringForgery fileName: String, + @StringForgery content: String + ) { + // Given + val file = File(fakeRootDirectory, fileName) + + // When + val writeResult = testedFileHandler.writeData(file, content.toByteArray(), false) + val readResult = testedFileHandler.readData(file) + + // Then + assertThat(writeResult).isTrue() + assertThat(readResult).containsExactlyElementsOf(listOf(content.toByteArray())) + } + + @Test + fun `𝕄 return file content 𝕎 writeData + readData() { append = true }`( + @StringForgery fileName: String, + forge: Forge + ) { + // Given + val file = File(fakeRootDirectory, fileName) + + val data = forge.aList { + aString().toByteArray() + } + + // When + var writeResult = true + data.forEach { writeResult = writeResult && testedFileHandler.writeData(file, it, true) } + val readResult = testedFileHandler.readData(file) + + // Then + assertThat(writeResult).isTrue() + assertThat(readResult).containsExactlyElementsOf(data) } // endregion @@ -547,7 +610,7 @@ internal class BatchFileHandlerTest { // Then assertThat(result).isTrue() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( Log.INFO, BatchFileHandler.INFO_MOVE_NO_SRC.format(Locale.US, fakeSrcDir.path) ) @@ -564,7 +627,7 @@ internal class BatchFileHandlerTest { // Then assertThat(result).isFalse() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( ERROR_WITH_TELEMETRY_LEVEL, BatchFileHandler.ERROR_MOVE_NOT_DIR.format(Locale.US, fakeSrcDir.path) ) @@ -581,7 +644,7 @@ internal class BatchFileHandlerTest { // Then assertThat(result).isFalse() - verify(mockLogHandler).handleLog( + verify(logger.mockSdkLogHandler).handleLog( ERROR_WITH_TELEMETRY_LEVEL, BatchFileHandler.ERROR_MOVE_NOT_DIR.format(Locale.US, fakeDstDir.path) ) @@ -664,7 +727,7 @@ internal class BatchFileHandlerTest { @Test fun `𝕄 create BatchFileHandler 𝕎 create() { without encryption }`() { // When - val fileHandler = BatchFileHandler.create(Logger(mockLogHandler), null) + val fileHandler = BatchFileHandler.create(Logger(logger.mockSdkLogHandler), null) // Then assertThat(fileHandler) .isInstanceOf(BatchFileHandler::class.java) @@ -674,7 +737,7 @@ internal class BatchFileHandlerTest { fun `𝕄 create BatchFileHandler 𝕎 create() { with encryption }`() { // When val mockEncryption = mock() - val fileHandler = BatchFileHandler.create(Logger(mockLogHandler), mockEncryption) + val fileHandler = BatchFileHandler.create(Logger(logger.mockSdkLogHandler), mockEncryption) // Then assertThat(fileHandler) @@ -687,4 +750,27 @@ internal class BatchFileHandlerTest { } // endregion + + // region private + + private fun headerBytes(data: ByteArray): ByteArray { + val meta = EventMeta(eventSize = data.size).asBytes + + return ByteArray(2).apply { + set(0, BatchFileHandler.HEADER_VERSION) + set(1, meta.size.toByte()) + } + meta + } + + // endregion + + companion object { + val logger = LoggerTestConfiguration() + + @TestConfigurationsProvider + @JvmStatic + fun getTestConfigurations(): List { + return listOf(logger) + } + } } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriterTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriterTest.kt index 27931f8cdb..e1bed74d51 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriterTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/persistence/file/single/SingleItemDataWriterTest.kt @@ -103,8 +103,7 @@ internal class SingleItemDataWriterTest { .writeData( file, serialized, - append = false, - separator = null + append = false ) } @@ -125,8 +124,7 @@ internal class SingleItemDataWriterTest { .writeData( file, lastSerialized, - append = false, - separator = null + append = false ) } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/utils/ByteArrayExtTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/utils/ByteArrayExtTest.kt index a73482a35d..71d7b49816 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/utils/ByteArrayExtTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/core/internal/utils/ByteArrayExtTest.kt @@ -7,19 +7,18 @@ package com.datadog.android.core.internal.utils import fr.xgouchet.elmyr.Forge +import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions import org.mockito.junit.jupiter.MockitoExtension -import org.mockito.junit.jupiter.MockitoSettings @Extensions( ExtendWith(MockitoExtension::class), ExtendWith(ForgeExtension::class) ) -@MockitoSettings() internal class ByteArrayExtTest { // region split @@ -141,4 +140,135 @@ internal class ByteArrayExtTest { } // endregion + + // region join + + @Test + fun `𝕄 join items 𝕎 join() { no prefix }`( + @StringForgery separator: String, + @StringForgery suffix: String, + forge: Forge + ) { + // Given + val dataBytes = forge.aList { + forge.aString().toByteArray() + } + + val separatorBytes = separator.toByteArray() + val suffixBytes = suffix.toByteArray() + + val expected = dataBytes.reduce { acc, item -> + acc + separatorBytes + item + } + suffixBytes + + // When + val joined = dataBytes.join(separatorBytes, suffix = suffixBytes) + + // Then + assertThat(joined).isEqualTo(expected) + } + + @Test + fun `𝕄 join items 𝕎 join() { no suffix }`( + @StringForgery separator: String, + @StringForgery prefix: String, + forge: Forge + ) { + // Given + val dataBytes = forge.aList { + forge.aString().toByteArray() + } + + val separatorBytes = separator.toByteArray() + val prefixBytes = prefix.toByteArray() + + val expected = prefixBytes + dataBytes.reduce { acc, item -> + acc + separatorBytes + item + } + + // When + val joined = dataBytes.join(separatorBytes, prefix = prefixBytes) + + // Then + assertThat(joined).isEqualTo(expected) + } + + @Test + fun `𝕄 join items 𝕎 join() { no suffix and prefix }`( + @StringForgery separator: String, + forge: Forge + ) { + // Given + val dataBytes = forge.aList { + forge.aString().toByteArray() + } + + val separatorBytes = separator.toByteArray() + + val expected = dataBytes.reduce { acc, item -> + acc + separatorBytes + item + } + + // When + val joined = dataBytes.join(separatorBytes) + + // Then + assertThat(joined).isEqualTo(expected) + } + + @Test + fun `𝕄 join items 𝕎 join() { empty separator }`( + @StringForgery prefix: String, + @StringForgery suffix: String, + forge: Forge + ) { + // Given + val dataBytes = forge.aList { + forge.aString().toByteArray() + } + + val prefixBytes = prefix.toByteArray() + val suffixBytes = suffix.toByteArray() + + val expected = prefixBytes + dataBytes.reduce { acc, bytes -> + acc + bytes + } + suffixBytes + + // When + val joined = + dataBytes.join(separator = ByteArray(0), prefix = prefixBytes, suffix = suffixBytes) + + // Then + assertThat(joined).isEqualTo(expected) + } + + @Test + fun `𝕄 join items 𝕎 join()`( + @StringForgery separator: String, + @StringForgery prefix: String, + @StringForgery suffix: String, + forge: Forge + ) { + // Given + val dataBytes = forge.aList { + forge.aString().toByteArray() + } + + val separatorBytes = separator.toByteArray() + val prefixBytes = prefix.toByteArray() + val suffixBytes = suffix.toByteArray() + + val expected = prefixBytes + dataBytes.reduce { acc, item -> + acc + separatorBytes + item + } + suffixBytes + + // When + val joined = + dataBytes.join(separator = separatorBytes, prefix = prefixBytes, suffix = suffixBytes) + + // Then + assertThat(joined).isEqualTo(expected) + } + + // endregion } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt index 3012314519..3538c54656 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt @@ -120,7 +120,7 @@ internal class RumDataWriterTest { // Then verify(mockFileHandler) - .writeData(fakeLastViewEventFile, fakeSerializedData, false, null) + .writeData(fakeLastViewEventFile, fakeSerializedData, false) verifyZeroInteractions(logger.mockSdkLogHandler) } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandlerTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandlerTest.kt index a6363ee484..a2f7899db3 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandlerTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/ndk/DatadogNdkCrashHandlerTest.kt @@ -9,6 +9,7 @@ package com.datadog.android.rum.internal.ndk import android.content.Context import com.datadog.android.core.internal.persistence.DataWriter import com.datadog.android.core.internal.persistence.Deserializer +import com.datadog.android.core.internal.persistence.file.FileHandler import com.datadog.android.core.internal.time.TimeProvider import com.datadog.android.core.model.NetworkInfo import com.datadog.android.core.model.UserInfo @@ -23,7 +24,6 @@ import com.datadog.android.rum.assertj.ViewEventAssert import com.datadog.android.rum.internal.domain.event.RumEventSourceProvider import com.datadog.android.rum.model.ErrorEvent import com.datadog.android.rum.model.ViewEvent -import com.datadog.android.security.Encryption import com.datadog.android.utils.forge.Configurator import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.argumentCaptor @@ -40,7 +40,6 @@ import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import java.io.File -import java.util.Base64 import java.util.Locale import java.util.concurrent.ExecutorService import org.assertj.core.api.Assertions.assertThat @@ -99,6 +98,9 @@ internal class DatadogNdkCrashHandlerTest { @Mock lateinit var mockLogHandler: LogHandler + @Mock + lateinit var mockFileHandler: FileHandler + lateinit var fakeNdkCacheDir: File @Forgery @@ -110,9 +112,6 @@ internal class DatadogNdkCrashHandlerTest { @Mock lateinit var mockTimeProvider: TimeProvider - @Mock - lateinit var mockLocalDataEncryption: Encryption - var fakeSourceErrorEvent: ErrorEvent.ErrorEventSource? = null @Mock @@ -127,8 +126,11 @@ internal class DatadogNdkCrashHandlerTest { .thenReturn(fakeSourceErrorEvent) whenever(mockContext.cacheDir) doReturn fakeCacheDir fakeNdkCacheDir = File(fakeCacheDir, DatadogNdkCrashHandler.NDK_CRASH_REPORTS_FOLDER_NAME) - - whenever(mockLocalDataEncryption.decrypt(any())) doAnswer { it.getArgument(0) } + whenever(mockFileHandler.readData(any())) doAnswer { + listOf( + it.getArgument(0).readBytes() + ) + } testedHandler = DatadogNdkCrashHandler( mockContext, @@ -140,7 +142,7 @@ internal class DatadogNdkCrashHandlerTest { mockUserInfoDeserializer, Logger(mockLogHandler), mockTimeProvider, - null, + mockFileHandler, mockRumEventSourceProvider ) } @@ -183,39 +185,6 @@ internal class DatadogNdkCrashHandlerTest { .isEqualTo(viewEvent) } - @Test - fun `𝕄 read last RUM View event 𝕎 prepareData() { with encryption }`( - @StringForgery viewEvent: String - ) { - testedHandler = DatadogNdkCrashHandler( - mockContext, - mockExecutorService, - mockLogGenerator, - mockNdkCrashLogDeserializer, - mockRumEventDeserializer, - mockNetworkInfoDeserializer, - mockUserInfoDeserializer, - Logger(mockLogHandler), - mockTimeProvider, - mockLocalDataEncryption - ) - - // Given - fakeNdkCacheDir.mkdirs() - File(fakeNdkCacheDir, DatadogNdkCrashHandler.RUM_VIEW_EVENT_FILE_NAME) - .writeBytes(Base64.getEncoder().encode(viewEvent.toByteArray())) - - // When - testedHandler.prepareData() - - // Then - assertThat(testedHandler.lastSerializedRumViewEvent).isNull() - verify(mockExecutorService).submit(captureRunnable.capture()) - captureRunnable.firstValue.run() - assertThat(testedHandler.lastSerializedRumViewEvent) - .isEqualTo(viewEvent) - } - @Test fun `𝕄 read network info 𝕎 prepareData()`( @StringForgery networkInfo: String @@ -235,39 +204,6 @@ internal class DatadogNdkCrashHandlerTest { .isEqualTo(networkInfo) } - @Test - fun `𝕄 read network info 𝕎 prepareData() { with encryption }`( - @StringForgery networkInfo: String - ) { - testedHandler = DatadogNdkCrashHandler( - mockContext, - mockExecutorService, - mockLogGenerator, - mockNdkCrashLogDeserializer, - mockRumEventDeserializer, - mockNetworkInfoDeserializer, - mockUserInfoDeserializer, - Logger(mockLogHandler), - mockTimeProvider, - mockLocalDataEncryption - ) - - // Given - fakeNdkCacheDir.mkdirs() - File(fakeNdkCacheDir, DatadogNdkCrashHandler.NETWORK_INFO_FILE_NAME) - .writeBytes(Base64.getEncoder().encode(networkInfo.toByteArray())) - - // When - testedHandler.prepareData() - - // Then - assertThat(testedHandler.lastSerializedNetworkInformation).isNull() - verify(mockExecutorService).submit(captureRunnable.capture()) - captureRunnable.firstValue.run() - assertThat(testedHandler.lastSerializedNetworkInformation) - .isEqualTo(networkInfo) - } - @Test fun `𝕄 read user info 𝕎 prepareData()`( @StringForgery userInfo: String @@ -287,39 +223,6 @@ internal class DatadogNdkCrashHandlerTest { .isEqualTo(userInfo) } - @Test - fun `𝕄 read user info 𝕎 prepareData() { with encryption }`( - @StringForgery userInfo: String - ) { - testedHandler = DatadogNdkCrashHandler( - mockContext, - mockExecutorService, - mockLogGenerator, - mockNdkCrashLogDeserializer, - mockRumEventDeserializer, - mockNetworkInfoDeserializer, - mockUserInfoDeserializer, - Logger(mockLogHandler), - mockTimeProvider, - mockLocalDataEncryption - ) - - // Given - fakeNdkCacheDir.mkdirs() - File(fakeNdkCacheDir, DatadogNdkCrashHandler.USER_INFO_FILE_NAME) - .writeBytes(Base64.getEncoder().encode(userInfo.toByteArray())) - - // When - testedHandler.prepareData() - - // Then - assertThat(testedHandler.lastSerializedUserInformation).isNull() - verify(mockExecutorService).submit(captureRunnable.capture()) - captureRunnable.firstValue.run() - assertThat(testedHandler.lastSerializedUserInformation) - .isEqualTo(userInfo) - } - @Test fun `𝕄 do nothing 𝕎 prepareData {directory does not exist}`() { // When diff --git a/detekt.yml b/detekt.yml index 7bdd56e927..e373692c48 100644 --- a/detekt.yml +++ b/detekt.yml @@ -591,7 +591,6 @@ datadog: - "android.content.res.Resources.getResourceEntryName(kotlin.Int):android.content.res.Resources.NotFoundException" - "android.net.ConnectivityManager.registerDefaultNetworkCallback(android.net.ConnectivityManager.NetworkCallback):java.lang.IllegalArgumentException,java.lang.SecurityException" - "android.net.ConnectivityManager.unregisterNetworkCallback(android.net.ConnectivityManager.NetworkCallback):java.lang.SecurityException" - - "android.util.Base64.decode(kotlin.ByteArray, kotlin.Int):java.lang.IllegalArgumentException" - "android.view.Choreographer.getInstance():java.lang.IllegalStateException" - "android.view.Choreographer.postFrameCallback():java.lang.IllegalArgumentException" - "android.view.MotionEvent.obtain(android.view.MotionEvent):java.lang.IllegalArgumentException" @@ -618,6 +617,8 @@ datadog: - "java.io.FileInputStream.use(kotlin.Function1):java.io.IOException" - "java.io.FileOutputStream.use(kotlin.Function1):java.io.IOException" - "java.io.FileOutputStream.write(kotlin.ByteArray):java.io.IOException" + - "java.io.InputStream.read():java.io.IOException" + - "java.io.InputStream.read(kotlin.ByteArray, kotlin.Int, kotlin.Int):java.io.IOException" - "java.nio.channels.FileChannel.lock():java.io.IOException,java.lang.IllegalStateException" - "java.nio.channels.FileLock.release():java.io.IOException" # endregion @@ -735,7 +736,6 @@ datadog: - "android.os.Process.getStartElapsedRealtime()" - "android.os.Process.myPid()" - "android.os.SystemClock.elapsedRealtime()" - - "android.util.Base64.encode(kotlin.ByteArray, kotlin.Int)" - "android.util.Log.i(kotlin.String?, kotlin.String)" - "android.util.Log.e(kotlin.String?, kotlin.String)" - "android.util.Log.e(kotlin.String?, kotlin.String?, kotlin.Throwable?)" @@ -942,6 +942,8 @@ datadog: - "kotlin.ByteArray.any(kotlin.Function1)" - "kotlin.ByteArray.isEmpty()" - "kotlin.ByteArray.isNotEmpty()" + - "kotlin.collections.Collection.sumOf(kotlin.Function1)" + - "kotlin.collections.Collection.withIndex()" - "kotlin.collections.Iterable.any(kotlin.Function1)" - "kotlin.collections.List.any(kotlin.Function1)" - "kotlin.collections.List.asSequence()" @@ -1084,6 +1086,7 @@ datadog: - "kotlin.text.Regex.matchEntire(kotlin.CharSequence)" # region Gson - "com.google.gson.JsonArray.add(kotlin.String)" + - "com.google.gson.JsonParseException.constructor(kotlin.Throwable)" # endregion # region Kronos - "com.lyft.kronos.AndroidClockFactory.createKronosClock(android.content.Context, kotlin.collections.List, kotlin.Long, kotlin.Long, com.lyft.kronos.SyncListener?, kotlin.Long, kotlin.Long)" diff --git a/instrumented/integration/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt b/instrumented/integration/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt index c10550c80f..7d251b3387 100644 --- a/instrumented/integration/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt +++ b/instrumented/integration/src/androidTest/kotlin/com/datadog/android/sdk/integration/security/EncryptionTest.kt @@ -26,8 +26,8 @@ import fr.xgouchet.elmyr.junit4.ForgeRule import io.opentracing.Tracer import io.opentracing.util.GlobalTracer import java.io.File -import java.util.Base64 import java.util.concurrent.atomic.AtomicBoolean +import kotlin.experimental.inv import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Before @@ -76,7 +76,7 @@ internal class EncryptionTest { file.name.startsWith("dd") && file.name.contains("pending") } val isNdkPendingDirectory = { file: File -> - file.name == "ndk_crash_reports_intermediary" + file.name == "ndk_crash_reports_intermediary_v2" } val dataDirectories = @@ -107,15 +107,7 @@ internal class EncryptionTest { .doesNotContain("source") assertThat(content) .overridingErrorMessage("Expecting ${file.path} to contain encryption marker") - .contains( - Base64.getEncoder() - .withoutPadding() - .encode(ENCRYPTION_MARKER.toByteArray()) - .decodeToString() - // last 1 or 2 characters may be different in the final file, because - // we encode marker + payload as a whole, so we drop them for comparison - .dropLast(2) - ) + .contains(ENCRYPTION_MARKER) } } } @@ -134,7 +126,7 @@ internal class EncryptionTest { private fun createSdkConfiguration(): Configuration { val encryption = object : Encryption { override fun encrypt(data: ByteArray): ByteArray { - return ENCRYPTION_MARKER.toByteArray() + data + return ENCRYPTION_MARKER.toByteArray() + data.map { it.inv() } } override fun decrypt(data: ByteArray): ByteArray {