From 7854d85b1af011f869859c6746fe8e85cf4de120 Mon Sep 17 00:00:00 2001 From: xxfast Date: Wed, 5 Aug 2026 00:01:46 +0530 Subject: [PATCH 1/2] Fix KVersionedStore to handle corrupt or partially written files gracefully --- .../kstore/file/extensions/KVersionedStore.kt | 59 ++++++++++++++++--- .../file/extensions/KVersionedStoreTests.kt | 57 +++++++++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-) diff --git a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt index 0050384..c195397 100644 --- a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt +++ b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt @@ -3,6 +3,7 @@ package io.github.xxfast.kstore.file.extensions import io.github.xxfast.kstore.Codec import io.github.xxfast.kstore.DefaultJson import io.github.xxfast.kstore.KStore +import io.github.xxfast.kstore.file.moveOrCopy import io.github.xxfast.kstore.storeOf import kotlinx.io.buffered import kotlinx.io.files.FileNotFoundException @@ -63,22 +64,60 @@ public class VersionedCodec( private val tempVersionPath: Path = Path("$versionPath.temp"), ) : Codec { + /** + * Decodes the file to a value. + * If the file does not exist, null is returned. + * If the file does not hold the current shape of [T], the value is recovered through [migration]. + * @return optional value that is decoded + */ override suspend fun decode(): T? = try { SystemFileSystem.source(file).buffered().use { json.decode(serializer, it) } + } catch (e: FileNotFoundException) { + null } catch (e: SerializationException) { - val previousVersion: Int = - if (SystemFileSystem.exists(versionPath)) - SystemFileSystem.source(versionPath).buffered().use { json.decode(Int.serializer(), it) } - else 0 + // The file doesn't hold the current shape of [T]. Either it was written by an older version of + // this store - which [migration] can recover from - or it is corrupt/partially written, in which + // case there is nothing to recover and [migration] is handed what little is known. + migration(decodeVersion(), decodeData()) + } - val data: JsonElement = - SystemFileSystem.source(file).buffered().use { json.decode(it) } - migration(previousVersion, data) + /** + * Reads the version the file was last written with. + * @return 0 when there is no version file - stores written before this one was versioned - or + * null when the version file exists but cannot be read, in which case the version is unknown. + */ + private fun decodeVersion(): Int? = + if (!SystemFileSystem.exists(versionPath)) 0 + else try { + SystemFileSystem.source(versionPath).buffered().use { json.decode(Int.serializer(), it) } } catch (e: FileNotFoundException) { + 0 + } catch (e: SerializationException) { + null + } + + /** + * Reads the raw contents of the file for [migration] to recover a value from. + * @return null when the file cannot be parsed as json at all, i.e. it is corrupt or empty. + */ + private fun decodeData(): JsonElement? = + try { + SystemFileSystem.source(file).buffered().use { json.decode(it) } + } catch (e: FileNotFoundException) { + null + } catch (e: SerializationException) { null } + /** + * Encodes the given value to the file, along with the current [version]. + * If the value is null, both files are deleted. + * If the encoding fails, the temp files are deleted. + * On platforms where atomic move is not supported (e.g., Android 7 and below) this falls back to a + * non-atomic copy-and-delete; the transactional guarantee does not hold for that fallback path. + * @param value optional value to encode + */ override suspend fun encode(value: T?) { if (value == null) { SystemFileSystem.delete(versionPath, mustExist = false) @@ -95,7 +134,9 @@ public class VersionedCodec( throw e } - SystemFileSystem.atomicMove(source = tempPath, destination = file) - SystemFileSystem.atomicMove(source = tempVersionPath, destination = versionPath) + // Data first, version second. A crash in between leaves the new data with a stale version, which + // still decodes directly. The reverse order would claim a version the data hasn't been written to. + moveOrCopy(source = tempPath, destination = file) + moveOrCopy(source = tempVersionPath, destination = versionPath) } } diff --git a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt index 9387ef7..f29310f 100644 --- a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt +++ b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt @@ -3,8 +3,10 @@ package io.github.xxfast.kstore.file.extensions import io.github.xxfast.kstore.KStore import io.github.xxfast.kstore.file.storeOf import kotlinx.coroutines.test.runTest +import kotlinx.io.buffered import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem +import kotlinx.io.writeString import kotlinx.serialization.KSerializer import kotlinx.serialization.Serializable import kotlinx.serialization.encoding.Decoder @@ -48,6 +50,11 @@ val MYLO_V42 = CatV42(name = "mylo", friends = mapOf("oreo" to 5, "kat" to 10)) class KVersionedStoreTests { private val file: Path = Path("test_migration.json") + private val versionFile: Path = Path("$file.version") + + private fun write(path: Path, contents: String) { + SystemFileSystem.sink(path).buffered().use { it.writeString(contents) } + } private val storeV0: KStore = storeOf(file = file) @@ -101,7 +108,7 @@ class KVersionedStoreTests { @AfterTest fun cleanup() { SystemFileSystem.delete(file, mustExist = false) - SystemFileSystem.delete(Path("${file.name}.version"), mustExist = false) + SystemFileSystem.delete(versionFile, mustExist = false) SystemFileSystem.delete(Path("${file.name}.temp"), mustExist = false) SystemFileSystem.delete(Path("${file.name}.version.temp"), mustExist = false) } @@ -155,6 +162,54 @@ class KVersionedStoreTests { assertEquals(expect, actual) } + // Corrupt or partially written stores must decode to null rather than crash - see issues #80, #157, #162 + + @Test + fun testDecodeEmptyFileWithEmptyVersionFile() = runTest { + write(file, "") + write(versionFile, "") + assertEquals(null, storeV2.get()) + } + + @Test + fun testDecodeEmptyFileWithVersionFile() = runTest { + write(file, "") + write(versionFile, "1") + assertEquals(null, storeV2.get()) + } + + @Test + fun testDecodeTruncatedFile() = runTest { + write(file, """{"name":"mylo","liv""") + write(versionFile, "1") + assertEquals(null, storeV2.get()) + } + + @Test + fun testDecodeEmptyFileWithoutVersionFile() = runTest { + write(file, "") + assertEquals(null, storeV2.get()) + } + + @Test + fun testCorruptStoreRepairsOnNextWrite() = runTest { + write(file, "") + write(versionFile, "") + assertEquals(null, storeV2.get()) + + storeV2.set(MYLO_V2) + assertEquals(MYLO_V2, storeV2.get()) + } + + @Test + fun testMigrationWithUnreadableVersionFile() = runTest { + // The data is intact and from v1, but the version file didn't survive. The version is unknown, + // so the migration can't place the data and falls through to its else branch rather than crashing. + storeV1.set(MYLO_V1) + write(versionFile, "") + assertEquals(null, storeV2.get()) + } + @Test fun testTransactionalEncode() = runTest { assertFailsWith { storeV41.set(MYLO_V41) } From c71362e9b2b4048ee1f4b0ecf96163025bf07296 Mon Sep 17 00:00:00 2001 From: xxfast Date: Wed, 5 Aug 2026 00:21:16 +0530 Subject: [PATCH 2/2] Improve KVersionedStore to handle corrupt or partially written files during migration --- .../kstore/file/extensions/KVersionedStore.kt | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt index c195397..10c1450 100644 --- a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt +++ b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt @@ -9,6 +9,7 @@ import kotlinx.io.buffered import kotlinx.io.files.FileNotFoundException import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem +import kotlinx.serialization.DeserializationStrategy import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.KSerializer import kotlinx.serialization.Serializable @@ -79,33 +80,28 @@ public class VersionedCodec( // The file doesn't hold the current shape of [T]. Either it was written by an older version of // this store - which [migration] can recover from - or it is corrupt/partially written, in which // case there is nothing to recover and [migration] is handed what little is known. - migration(decodeVersion(), decodeData()) + // No version file at all means the store predates versioning, so it reads as 0. One that exists + // but can't be read leaves the version unknown, same as unreadable data. + migration( + decodeOrNull(versionPath, Int.serializer(), whenMissing = 0), + decodeOrNull(file, JsonElement.serializer(), whenMissing = null), + ) } /** - * Reads the version the file was last written with. - * @return 0 when there is no version file - stores written before this one was versioned - or - * null when the version file exists but cannot be read, in which case the version is unknown. + * Reads [path] with [deserializer], degrading rather than throwing so that a corrupt store can be + * migrated or reset instead of crashing on every read. + * @return [whenMissing] when there is no such file, or null when its contents cannot be decoded */ - private fun decodeVersion(): Int? = - if (!SystemFileSystem.exists(versionPath)) 0 - else try { - SystemFileSystem.source(versionPath).buffered().use { json.decode(Int.serializer(), it) } - } catch (e: FileNotFoundException) { - 0 - } catch (e: SerializationException) { - null - } - - /** - * Reads the raw contents of the file for [migration] to recover a value from. - * @return null when the file cannot be parsed as json at all, i.e. it is corrupt or empty. - */ - private fun decodeData(): JsonElement? = + private fun decodeOrNull( + path: Path, + deserializer: DeserializationStrategy, + whenMissing: R?, + ): R? = try { - SystemFileSystem.source(file).buffered().use { json.decode(it) } + SystemFileSystem.source(path).buffered().use { json.decode(deserializer, it) } } catch (e: FileNotFoundException) { - null + whenMissing } catch (e: SerializationException) { null }