mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(backup): chunk Android backup reads to survive 2MB CursorWindow
KeyValStore.get() read the value via cursor.getString(), which throws SQLiteBlobTooBigException once a row exceeds Android's ~2MB CursorWindow. The on-device backup blob crosses 2MB after roughly a year of normal use, so the backup was written but never readable — surfacing as "Error invoking loadFromDb: Java exception" and silently breaking the #7901 eviction-recovery path. Combined with the data store living in evictable WebView IndexedDB (#7892), this caused total data loss on Android. Electron/iOS use file-based backups and are unaffected. - KeyValStore.get(): read in 256K-char substr() chunks + try/catch->default; also recovers already-written oversized rows. - KeyValStoreInstrumentedTest: emulator regression (Robolectric can't reproduce the CursorWindow limit). - local-backup.service: _loadAndroidDbValueSafe() logs blob size (not content) to the exportable log and degrades a read failure to "no backup" instead of an opaque crash. Closes #8401
This commit is contained in:
parent
4699499dc3
commit
9263384408
3 changed files with 157 additions and 19 deletions
|
|
@ -0,0 +1,70 @@
|
|||
package com.superproductivity.superproductivity.app
|
||||
|
||||
import androidx.test.InstrumentationRegistry
|
||||
import androidx.test.runner.AndroidJUnit4
|
||||
import com.superproductivity.superproductivity.App
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
|
||||
/**
|
||||
* Instrumented test for [KeyValStore].
|
||||
*
|
||||
* MUST run on an emulator/device, not Robolectric: the ~2 MB CursorWindow limit
|
||||
* that this is guarding against only manifests against real Android SQLite.
|
||||
*
|
||||
* Regression for the on-device backup data-loss bug: a backup blob larger than
|
||||
* Android's ~2 MB CursorWindow could be written via [KeyValStore.set] but not
|
||||
* read back via [KeyValStore.get] — `cursor.getString()` threw
|
||||
* SQLiteBlobTooBigException ("Row too big to fit into CursorWindow"). That surfaced
|
||||
* to the WebView as "Error invoking loadFromDb: Java exception …", so the
|
||||
* eviction-recovery path (#7901) silently failed and the user lost everything.
|
||||
*
|
||||
* Run: ./gradlew :app:connectedPlayDebugAndroidTest (emulator/device required).
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class KeyValStoreInstrumentedTest {
|
||||
|
||||
private val store by lazy {
|
||||
(InstrumentationRegistry.getTargetContext().applicationContext as App).keyValStore
|
||||
}
|
||||
|
||||
private val testKey = "instr_test_keyvalstore"
|
||||
|
||||
@After
|
||||
fun cleanUp() {
|
||||
store.set(testKey, null)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun smallValueRoundTrips() {
|
||||
val value = "hello / with 'apostrophes' and \"quotes\" and \n newlines"
|
||||
store.set(testKey, value)
|
||||
assertEquals(value, store.get(testKey, "DEFAULT"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun missingKeyReturnsDefault() {
|
||||
assertEquals("DEFAULT", store.get("instr_test_definitely_absent_key", "DEFAULT"))
|
||||
}
|
||||
|
||||
/**
|
||||
* Core regression: a value well past the ~2 MB CursorWindow limit.
|
||||
* Pre-fix this throws inside get(); post-fix the chunked read returns it whole.
|
||||
*/
|
||||
@Test
|
||||
fun largeValueOverCursorWindowRoundTrips() {
|
||||
val big = buildString {
|
||||
// ~3 MB of varied content (not one repeated char) to mimic a real JSON backup.
|
||||
val unit = "{\"id\":\"abc123def456\",\"title\":\"do the thing\",\"t\":600000},"
|
||||
while (length < 3 * 1024 * 1024) append(unit)
|
||||
}
|
||||
store.set(testKey, big)
|
||||
|
||||
val read = store.get(testKey, "DEFAULT")
|
||||
|
||||
assertEquals(big.length, read.length)
|
||||
assertEquals(big, read)
|
||||
}
|
||||
}
|
||||
|
|
@ -69,20 +69,48 @@ class KeyValStore(private val context: Context) :
|
|||
val newKey = DatabaseUtils.sqlEscapeString(key)
|
||||
Log.v(TAG, "getting db value: $newKey")
|
||||
val dbHelper = (context.applicationContext as App).keyValStore
|
||||
var value = defaultValue
|
||||
dbHelper.readableDatabase?.let { database ->
|
||||
database.query(
|
||||
DATABASE_TABLE, arrayOf(VALUE), "$KEY=?", arrayOf(newKey), null, null, null
|
||||
)?.let { cursor ->
|
||||
if (cursor.moveToNext()) {
|
||||
value = cursor.getString(cursor.getColumnIndexOrThrow(VALUE))
|
||||
val database = dbHelper.readableDatabase ?: return defaultValue
|
||||
return try {
|
||||
// Read the value in <2 MB chunks via substr(). A single cursor.getString()
|
||||
// on a row larger than Android's ~2 MB CursorWindow throws
|
||||
// SQLiteBlobTooBigException ("Row too big to fit into CursorWindow"), which
|
||||
// silently broke on-device backup restore once the backup blob grew past
|
||||
// ~2 MB (a year of normal use). substr() is 1-indexed; GET_CHUNK_CHARS is a
|
||||
// CHARACTER count kept well under the byte window even for 4-byte UTF-8.
|
||||
val sb = StringBuilder()
|
||||
var offset = 1
|
||||
var rowExists = false
|
||||
while (true) {
|
||||
database.rawQuery(
|
||||
"SELECT substr($VALUE, ?, ?) FROM $DATABASE_TABLE WHERE $KEY=? LIMIT 1",
|
||||
arrayOf(offset.toString(), GET_CHUNK_CHARS.toString(), newKey)
|
||||
).use { cursor ->
|
||||
if (cursor.moveToFirst()) {
|
||||
rowExists = true
|
||||
val chunk = cursor.getString(0)
|
||||
if (chunk == null) {
|
||||
offset = DONE // VALUE column is NULL
|
||||
} else {
|
||||
sb.append(chunk)
|
||||
// A short chunk means we read past the end → finished.
|
||||
offset = if (chunk.length < GET_CHUNK_CHARS) DONE else offset + GET_CHUNK_CHARS
|
||||
}
|
||||
} else {
|
||||
offset = DONE // no row for this key
|
||||
}
|
||||
}
|
||||
Log.v(TAG, "get db value size:" + value.length)
|
||||
cursor.close()
|
||||
if (offset == DONE) break
|
||||
}
|
||||
Log.v(TAG, "get db value size:" + sb.length)
|
||||
if (rowExists) sb.toString() else defaultValue
|
||||
} catch (e: Exception) {
|
||||
// Never let a read failure crash the JS bridge — degrade to default so
|
||||
// callers can surface "no backup" instead of an opaque invocation error.
|
||||
Log.e(TAG, "get failed for key $newKey", e)
|
||||
defaultValue
|
||||
} finally {
|
||||
database.close()
|
||||
}
|
||||
return value
|
||||
}
|
||||
|
||||
fun clearAll(context: Context) {
|
||||
|
|
@ -103,6 +131,13 @@ class KeyValStore(private val context: Context) :
|
|||
private const val VALUE: String = "VALUE"
|
||||
private const val KEY_CREATED_AT: String = "KEY_CREATED_AT"
|
||||
private const val TAG: String = "SupKeyValStore"
|
||||
|
||||
// Read large rows in chunks of this many CHARACTERS. Even at 4 bytes/char
|
||||
// (256K * 4 = 1 MB) this stays well under the ~2 MB CursorWindow limit.
|
||||
private const val GET_CHUNK_CHARS: Int = 256 * 1024
|
||||
|
||||
// Sentinel offset signalling the chunked read loop is finished.
|
||||
private const val DONE: Int = -1
|
||||
// IF NOT EXISTS so the additive onUpgrade() (which calls onCreate) is safe
|
||||
// to run against an already-populated DB without throwing (#7901).
|
||||
private const val CREATE_TABLE =
|
||||
|
|
|
|||
|
|
@ -46,6 +46,11 @@ const IOS_BACKUP_PREV_FILENAME = 'super-productivity-backup.prev.json';
|
|||
const NEAR_EMPTY_NEW_TASKS = 3;
|
||||
const SUBSTANTIAL_EXISTING_TASKS = 10;
|
||||
|
||||
// Warn when the Android backup blob nears Android's ~2 MB SQLite CursorWindow
|
||||
// row limit. A row past that limit could be written but not read back, silently
|
||||
// breaking eviction recovery (now fixed via a chunked native read in KeyValStore).
|
||||
const ANDROID_BACKUP_SIZE_WARN_CHARS = 1.8 * 1024 * 1024;
|
||||
|
||||
@Injectable({
|
||||
providedIn: 'root',
|
||||
})
|
||||
|
|
@ -86,13 +91,12 @@ export class LocalBackupService {
|
|||
checkBackupAvailable(): Promise<boolean | LocalBackupMeta> {
|
||||
if (this._isAndroidWebView) {
|
||||
// Available if either ring slot holds a backup (#7901).
|
||||
return androidInterface.loadFromDbWrapped(ANDROID_DB_KEY).then(async (primary) => {
|
||||
if (primary) {
|
||||
return (async () => {
|
||||
if (await this._loadAndroidDbValueSafe(ANDROID_DB_KEY)) {
|
||||
return true;
|
||||
}
|
||||
const prev = await androidInterface.loadFromDbWrapped(ANDROID_DB_KEY_PREV);
|
||||
return !!prev;
|
||||
});
|
||||
return !!(await this._loadAndroidDbValueSafe(ANDROID_DB_KEY_PREV));
|
||||
})();
|
||||
}
|
||||
if (this._platformService.isIOS()) {
|
||||
return this._checkBackupAvailableIOS();
|
||||
|
|
@ -112,12 +116,35 @@ export class LocalBackupService {
|
|||
// usable exists (degrades to the existing import-error snack rather than
|
||||
// throwing on the startup path).
|
||||
const [primary, prev] = await Promise.all([
|
||||
androidInterface.loadFromDbWrapped(ANDROID_DB_KEY),
|
||||
androidInterface.loadFromDbWrapped(ANDROID_DB_KEY_PREV),
|
||||
this._loadAndroidDbValueSafe(ANDROID_DB_KEY),
|
||||
this._loadAndroidDbValueSafe(ANDROID_DB_KEY_PREV),
|
||||
]);
|
||||
return selectBestBackupStr(primary, prev) ?? '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Android-only native-KV read with diagnostics + graceful failure.
|
||||
*
|
||||
* - Logs the blob SIZE (never content — see core/log rule 9) so a shared log
|
||||
* export shows whether a backup has grown into the ~2 MB CursorWindow danger
|
||||
* zone that used to make `loadFromDb` throw.
|
||||
* - Returns null instead of throwing, so a read failure degrades to "no backup"
|
||||
* (the existing snack) rather than an opaque "Error invoking loadFromDb" that
|
||||
* aborts the whole restore action.
|
||||
*/
|
||||
private async _loadAndroidDbValueSafe(key: string): Promise<string | null> {
|
||||
try {
|
||||
const val = await androidInterface.loadFromDbWrapped(key);
|
||||
Log.log(
|
||||
`LocalBackupService: read Android backup '${key}' (${val ? val.length : 0} chars)`,
|
||||
);
|
||||
return val;
|
||||
} catch (e) {
|
||||
Log.err(`LocalBackupService: failed to read Android backup '${key}'`, e);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async loadBackupIOS(): Promise<string> {
|
||||
const [primary, prev] = await Promise.all([
|
||||
this._readIOSFileOrNull(IOS_BACKUP_FILENAME),
|
||||
|
|
@ -379,14 +406,20 @@ export class LocalBackupService {
|
|||
// Returns true when a backup was actually written, false when the A3 guard
|
||||
// skipped it (so the caller knows whether to advance the last-backup time).
|
||||
private async _backupAndroid(data: AppDataComplete): Promise<boolean> {
|
||||
const existing = await androidInterface.loadFromDbWrapped(ANDROID_DB_KEY);
|
||||
const existing = await this._loadAndroidDbValueSafe(ANDROID_DB_KEY);
|
||||
if (this._guardNearEmptyOverwrite(data, existing, 'Android')) {
|
||||
return false;
|
||||
}
|
||||
if (existing) {
|
||||
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY_PREV, existing);
|
||||
}
|
||||
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY, JSON.stringify(data));
|
||||
const json = JSON.stringify(data);
|
||||
// Size only — never content (core/log rule 9). Surfaces in shared log exports
|
||||
// so we can see a backup approaching the ~2 MB native-read danger zone.
|
||||
if (json.length > ANDROID_BACKUP_SIZE_WARN_CHARS) {
|
||||
Log.warn(`LocalBackupService: Android backup is large (${json.length} chars)`);
|
||||
}
|
||||
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY, json);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue