fix(prefs): recover a preference store whose bytes are not valid UTF-8
`File.readAsString` reports a UTF-8 decode failure as a FileSystemException, not a FormatException, so three guards written for that case never ran. The preflight's `on FormatException` branch was unreachable and its `on FileSystemException` sibling waved the document through; the plugin then threw the same FileSystemException, which failed the FormatException/TypeError test that decides repairability; and quarantine's lossy-decode fallback sat dead behind a rethrow. A store with one bad high byte — a UTF-16 BOM, a stray 0x80 — therefore reached the user as a failure screen with no Repair button and no way forward at all. Read bytes and decode explicitly instead, at both sites. Classification moves into describeStoreDamage, so a failure that surfaces after the preflight passed is judged by re-reading the file rather than by the error's type: a denied or locked store is indistinguishable from a decode failure by type or message, and offering a destructive repair for a permissions problem would reset every setting and risk the vault key over something a chmod fixes. isCorruptStoreError went with it, having no remaining callers. A repair that quarantines the store and then cannot reopen it no longer strands the process either. The repaired future was built straight from the cache loader, bypassing the self-healing reset sharedCache installs, so a failed reopen parked a rejected future in _cacheFuture and every later attempt replayed that stale error — with the damaged file already moved aside, so a restart would have booted cleanly. CorruptPreferenceStoreException now carries reopenSafe and a derived, content-free shape: byte length, whether it decoded, whether every byte is zero. #1732 arrived as "FormatException at offset 0" and nothing else, which cannot separate an all-zero file from a non-JSON first character from bytes that are not UTF-8; these can, and never quote the document. Cover the loop against the real desktop backend rather than a fake. shared_preferences_linux is pure Dart, byte-identical to the Windows implementation, and exposes fs/pathProvider, so pointing it at a temp directory exercises the genuine read, parse, cache and write path on any host — the join between preflight, classification and reopen where every one of these defects lived, and which had no coverage at all.
This commit is contained in:
edde746
@@ -1,8 +1,12 @@
|
||||
import 'dart:async';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:path_provider_platform_interface/path_provider_platform_interface.dart';
|
||||
import 'package:plezy/services/base_shared_preferences_service.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import '../test_helpers/io_fakes.dart';
|
||||
import '../test_helpers/prefs.dart';
|
||||
|
||||
void main() {
|
||||
@@ -57,4 +61,56 @@ void main() {
|
||||
await cache.setString('loader', 'production');
|
||||
expect(cache.getString('loader'), 'production');
|
||||
});
|
||||
|
||||
_poisonedCacheRegression();
|
||||
}
|
||||
|
||||
/// A repair that quarantines the store and then cannot reopen it must not
|
||||
/// leave the process permanently unable to try again.
|
||||
///
|
||||
/// Before #1732's fix the repaired future was built straight from the cache
|
||||
/// loader, bypassing the self-healing `onError` reset that `sharedCache`
|
||||
/// installs. A reopen failure therefore parked a rejected future in
|
||||
/// `_cacheFuture`, and every later attempt replayed that stale error for the
|
||||
/// rest of the process — with the damaged file already moved aside, so a
|
||||
/// restart would have booted cleanly.
|
||||
void _poisonedCacheRegression() {
|
||||
group('repairCorruptStore', () {
|
||||
late Directory root;
|
||||
late PathProviderPlatform previousPathProvider;
|
||||
|
||||
setUp(() async {
|
||||
resetSharedPreferencesForTest();
|
||||
root = await Directory.systemTemp.createTemp('plezy_repair_reopen_');
|
||||
previousPathProvider = PathProviderPlatform.instance;
|
||||
PathProviderPlatform.instance = FakePathProvider(root);
|
||||
});
|
||||
|
||||
tearDown(() async {
|
||||
BaseSharedPreferencesService.resetForTesting();
|
||||
PathProviderPlatform.instance = previousPathProvider;
|
||||
if (await root.exists()) await root.delete(recursive: true);
|
||||
});
|
||||
|
||||
test('a reopen failure does not poison the shared cache', () async {
|
||||
final support = Directory(p.join(root.path, 'support'))..createSync(recursive: true);
|
||||
File(p.join(support.path, 'shared_preferences.json')).writeAsStringSync('{"theme":"dark"');
|
||||
|
||||
var loadCount = 0;
|
||||
BaseSharedPreferencesService.setCacheLoaderForTesting(() {
|
||||
loadCount++;
|
||||
if (loadCount == 1) return Future<SharedPreferencesWithCache>.error(StateError('reopen failed'));
|
||||
return SharedPreferencesWithCache.create(cacheOptions: const SharedPreferencesWithCacheOptions());
|
||||
});
|
||||
|
||||
await expectLater(BaseSharedPreferencesService.repairCorruptStore(), throwsA(isA<StateError>()));
|
||||
|
||||
// The damaged file is already quarantined, so the next attempt has a
|
||||
// clean slate and must actually be allowed to use it.
|
||||
final recovered = await BaseSharedPreferencesService.sharedCache();
|
||||
expect(loadCount, 2);
|
||||
await recovered.setBool('recovered', true);
|
||||
expect(recovered.getBool('recovered'), isTrue);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -190,6 +190,105 @@ void main() {
|
||||
throwsA(isA<CorruptPreferenceStoreException>()),
|
||||
);
|
||||
});
|
||||
|
||||
// #1732 was reported as "FormatException at offset 0" and nothing else.
|
||||
// These fix which byte shapes can produce that, because the diagnostic
|
||||
// deliberately discards the document and the offset is all a report has.
|
||||
group('byte-level damage', () {
|
||||
test('an all-zero document is rejected at offset 0', () async {
|
||||
// The shape an interrupted write leaves behind when the file system
|
||||
// extended the file's metadata but never flushed its contents.
|
||||
await store.writeAsBytes(List<int>.filled(64, 0));
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store);
|
||||
|
||||
expect(damage, isNotNull);
|
||||
expect(damage!.causeType, 'FormatException');
|
||||
expect(damage.offset, 0);
|
||||
expect(damage.shape?.length, 64);
|
||||
expect(damage.shape?.validUtf8, isTrue);
|
||||
expect(damage.shape?.allZero, isTrue);
|
||||
});
|
||||
|
||||
test('a leading NUL before an intact document is rejected at offset 0', () async {
|
||||
await store.writeAsBytes([0, ...utf8.encode('{"$credentialVaultKeyPref":"$_validVaultKey"}')]);
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store);
|
||||
|
||||
expect(damage?.offset, 0);
|
||||
// Byte-level garbage at the front, but the entries behind it are still
|
||||
// verbatim, so this is the offset-0 shape a salvage can still rescue.
|
||||
expect(damage?.shape?.allZero, isFalse);
|
||||
expect(PrefsRecovery.salvage(await store.readAsString()).vaultKey, _validVaultKey);
|
||||
});
|
||||
|
||||
test('bytes that are not UTF-8 are damage, not an unreadable file', () async {
|
||||
// `File.readAsString` reports this as a FileSystemException, which is
|
||||
// indistinguishable from a denied or locked file — so the preflight
|
||||
// used to wave it through and the app died with no Repair button.
|
||||
await store.writeAsBytes([0xFF, 0xFE, ...utf8.encode('{"theme":"dark"}')]);
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store);
|
||||
|
||||
expect(damage, isNotNull);
|
||||
expect(damage!.causeType, 'FormatException');
|
||||
expect(damage.shape?.validUtf8, isFalse);
|
||||
await expectLater(
|
||||
PrefsRecovery.assertStoreReadable(storeFileOverride: store),
|
||||
throwsA(isA<CorruptPreferenceStoreException>()),
|
||||
);
|
||||
});
|
||||
|
||||
test('a UTF-8 BOM is stripped by the decoder and accepted', () async {
|
||||
// Ruled out as a cause of #1732: the decoder consumes the BOM, so the
|
||||
// document behind it parses and the app boots.
|
||||
await store.writeAsBytes([0xEF, 0xBB, 0xBF, ...utf8.encode('{"theme":"dark"}')]);
|
||||
|
||||
expect(await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store), isNull);
|
||||
});
|
||||
|
||||
test('a whitespace-only document fails past offset 0', () async {
|
||||
// Also ruled out: the parser skips the whitespace first, so the offset
|
||||
// lands at the end of the document rather than at its first byte.
|
||||
await store.writeAsString(' \n');
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store);
|
||||
|
||||
expect(damage, isNotNull);
|
||||
expect(damage!.offset, isNot(0));
|
||||
});
|
||||
|
||||
test('a structural rejection carries no offset', () async {
|
||||
await store.writeAsString('{"theme":"dark","broken":null}');
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: store);
|
||||
|
||||
expect(damage?.offset, isNull);
|
||||
});
|
||||
|
||||
test('reopenSafe and the byte shape reach the rendered message', () async {
|
||||
// The whole point of carrying them: a report of this class should be
|
||||
// diagnosable without asking the user for the file.
|
||||
await store.writeAsBytes(List<int>.filled(8, 0));
|
||||
|
||||
final damage = await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: false, storeFileOverride: store);
|
||||
|
||||
expect(damage.toString(), contains('reopenSafe: false'));
|
||||
expect(damage.toString(), contains('8 bytes'));
|
||||
expect(damage.toString(), contains('every byte zero'));
|
||||
// Still never the document itself.
|
||||
expect(damage.toString(), isNot(contains(_validVaultKey)));
|
||||
});
|
||||
|
||||
test('an unreadable store is not classified as damage', () async {
|
||||
// A denied or locked file must keep its own error and its own
|
||||
// non-repairable path; offering a destructive repair for it would be
|
||||
// worse than reporting it.
|
||||
final missing = File('${store.parent.path}/definitely-absent.json');
|
||||
|
||||
expect(await PrefsRecovery.describeCurrentStoreDamage(reopenSafe: true, storeFileOverride: missing), isNull);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('quarantine', () {
|
||||
@@ -213,6 +312,18 @@ void main() {
|
||||
|
||||
expect(await File(result.backupPath!).exists(), isFalse);
|
||||
});
|
||||
|
||||
test('salvages a store whose bytes are not valid UTF-8', () async {
|
||||
// `readAsString` raises FileSystemException for a decode failure, so the
|
||||
// old strict read aborted the whole repair and left the damaged store
|
||||
// live — the app stayed dead with a "Repair failed" snackbar.
|
||||
await store.writeAsBytes([0xFF, 0xFE, ...utf8.encode('{"$credentialVaultKeyPref":"$_validVaultKey"}')]);
|
||||
|
||||
final result = await PrefsRecovery.quarantine(storeFileOverride: store);
|
||||
|
||||
expect(await store.exists(), isFalse);
|
||||
expect(result.salvaged.vaultKey, _validVaultKey);
|
||||
});
|
||||
});
|
||||
|
||||
group('backupStore', () {
|
||||
|
||||
@@ -0,0 +1,202 @@
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:path_provider_linux/path_provider_linux.dart';
|
||||
import 'package:path_provider_platform_interface/path_provider_platform_interface.dart';
|
||||
import 'package:plezy/services/base_shared_preferences_service.dart';
|
||||
import 'package:plezy/services/prefs_recovery.dart';
|
||||
import 'package:plezy/services/sensitive_prefs.dart';
|
||||
import 'package:plezy/services/settings_service.dart';
|
||||
import 'package:shared_preferences_linux/shared_preferences_linux.dart';
|
||||
import 'package:shared_preferences_platform_interface/shared_preferences_async_platform_interface.dart';
|
||||
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';
|
||||
|
||||
import '../test_helpers/io_fakes.dart';
|
||||
|
||||
/// The whole #1732 loop, driven against the **real** desktop preference
|
||||
/// backend rather than an in-memory fake.
|
||||
///
|
||||
/// The two halves of the existing coverage never met: file-level tests called
|
||||
/// `PrefsRecovery` statics with a `storeFileOverride` and never built a gate,
|
||||
/// while gate-level tests threw synthetic exceptions and never touched a file.
|
||||
/// Every defect in #1732 lived in the join — the preflight's callsite, the
|
||||
/// classification of what the plugin threw, and the reopen after a repair.
|
||||
///
|
||||
/// `shared_preferences_linux` is pure Dart and byte-identical to the Windows
|
||||
/// implementation; both expose `fs`/`pathProvider` for exactly this. Pointing
|
||||
/// its path provider at a temp directory runs the genuine read, parse, cache
|
||||
/// and write code on any host.
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
late Directory root;
|
||||
late Directory support;
|
||||
late File store;
|
||||
|
||||
/// A 32-byte key, the only length `AesGcm.with256bits()` accepts.
|
||||
final validVaultKey = base64Encode(List<int>.generate(32, (i) => i));
|
||||
|
||||
setUp(() async {
|
||||
root = await Directory.systemTemp.createTemp('plezy_prefs_repair_flow_');
|
||||
support = Directory(p.join(root.path, 'support'))..createSync(recursive: true);
|
||||
store = File(p.join(support.path, prefsStoreFileName));
|
||||
|
||||
PathProviderPlatform.instance = FakePathProvider(root);
|
||||
// The desktop store path is Windows/Linux only; without this the preflight
|
||||
// is a no-op on a macOS host and the test would assert nothing.
|
||||
PrefsRecovery.debugSetSupportedPlatformOverride(true);
|
||||
|
||||
final pathProvider = _TempPathProviderLinux(support.path);
|
||||
SharedPreferencesStorePlatform.instance = SharedPreferencesLinux()..pathProvider = pathProvider;
|
||||
SharedPreferencesAsyncPlatform.instance = SharedPreferencesAsyncLinux()..pathProvider = pathProvider;
|
||||
|
||||
BaseSharedPreferencesService.resetForTesting();
|
||||
SettingsService.resetForTesting();
|
||||
// `CredentialVault` memoises the first key it sees; the repair reseeds
|
||||
// before anything can read, which is what makes the reseed safe.
|
||||
});
|
||||
|
||||
tearDown(() async {
|
||||
BaseSharedPreferencesService.resetForTesting();
|
||||
SettingsService.resetForTesting();
|
||||
PrefsRecovery.debugSetSupportedPlatformOverride(null);
|
||||
if (await root.exists()) await root.delete(recursive: true);
|
||||
});
|
||||
|
||||
test('a store the backend can read opens normally', () async {
|
||||
await store.writeAsString(jsonEncode({'theme': 'dark'}));
|
||||
|
||||
await expectLater(SettingsService.getInstance(), completes);
|
||||
});
|
||||
|
||||
test('an offset-0 store fails the gate as a repairable corruption', () async {
|
||||
// The exact shape reported in #1732: valid UTF-8, non-empty, and a first
|
||||
// byte that is not a legal JSON value start.
|
||||
await store.writeAsBytes([
|
||||
0,
|
||||
...utf8.encode(jsonEncode({'theme': 'dark'})),
|
||||
]);
|
||||
|
||||
final error = await SettingsService.getInstance().then<Object?>((_) => null, onError: (Object e) => e);
|
||||
|
||||
expect(error, isA<CorruptPreferenceStoreException>());
|
||||
final corruption = error! as CorruptPreferenceStoreException;
|
||||
expect(corruption.offset, 0);
|
||||
// Caught by the preflight, before either backend memoised anything, so the
|
||||
// repair can reopen in this same process.
|
||||
expect(corruption.reopenSafe, isTrue);
|
||||
});
|
||||
|
||||
test('repairing an offset-0 store reopens it in the same process and keeps the vault key', () async {
|
||||
await store.writeAsBytes([
|
||||
0,
|
||||
...utf8.encode(jsonEncode({credentialVaultKeyPref: validVaultKey, 'theme': 'dark'})),
|
||||
]);
|
||||
|
||||
await expectLater(SettingsService.getInstance(), throwsA(isA<CorruptPreferenceStoreException>()));
|
||||
|
||||
final outcome = await BaseSharedPreferencesService.repairCorruptStore();
|
||||
|
||||
expect(outcome.vaultKeySalvaged, isTrue);
|
||||
expect(outcome.requiresRestart, isFalse);
|
||||
expect(outcome.backupPath, isNotNull);
|
||||
expect(await File(outcome.backupPath!).exists(), isTrue);
|
||||
|
||||
// The point of the whole exercise: this process, no restart.
|
||||
final settings = await SettingsService.getInstance();
|
||||
expect(settings, isNotNull);
|
||||
|
||||
// The salvaged key is on disk under the async key name, so every token
|
||||
// stored as ciphertext in the database stays decryptable.
|
||||
final reopened = jsonDecode(await store.readAsString()) as Map<String, dynamic>;
|
||||
expect(reopened[credentialVaultKeyPref], validVaultKey);
|
||||
// Settings were reset, which is the cost the consent dialog names.
|
||||
expect(reopened.containsKey('theme'), isFalse);
|
||||
});
|
||||
|
||||
test('a store whose bytes are not UTF-8 is repairable, not a dead end', () async {
|
||||
// `File.readAsString` reports a decode failure as FileSystemException, so
|
||||
// this used to slip past the preflight, fail the type check that decides
|
||||
// repairability, and reach the user as a failure screen with no Repair
|
||||
// button and no way forward.
|
||||
await store.writeAsBytes([
|
||||
0xFF,
|
||||
0xFE,
|
||||
...utf8.encode(jsonEncode({credentialVaultKeyPref: validVaultKey})),
|
||||
]);
|
||||
|
||||
final error = await SettingsService.getInstance().then<Object?>((_) => null, onError: (Object e) => e);
|
||||
|
||||
expect(error, isA<CorruptPreferenceStoreException>());
|
||||
expect((error! as CorruptPreferenceStoreException).shape?.validUtf8, isFalse);
|
||||
|
||||
final outcome = await BaseSharedPreferencesService.repairCorruptStore();
|
||||
|
||||
expect(outcome.vaultKeySalvaged, isTrue);
|
||||
await expectLater(SettingsService.getInstance(), completes);
|
||||
});
|
||||
|
||||
test('an empty store is a first launch, not damage', () async {
|
||||
// Both backends skip `json.decode` for an empty document, so rejecting it
|
||||
// would brick a launch the plugin handles perfectly well.
|
||||
await store.writeAsBytes(const []);
|
||||
|
||||
await expectLater(SettingsService.getInstance(), completes);
|
||||
});
|
||||
|
||||
test('a genuine read failure keeps its own type and stays non-repairable', () async {
|
||||
// A store the process cannot read is not a damaged *document*. Offering a
|
||||
// destructive repair for a permissions problem would reset every setting
|
||||
// and risk the vault key over something a chmod fixes, so the failure has
|
||||
// to keep its own type and its own non-repairable path.
|
||||
await store.writeAsString(jsonEncode({'theme': 'dark'}));
|
||||
if (!_run('chmod', ['000', store.path])) {
|
||||
// No `chmod` on a Windows PATH, and its ACL model would need a different
|
||||
// probe entirely.
|
||||
markTestSkipped('cannot make a file unreadable here');
|
||||
return;
|
||||
}
|
||||
addTearDown(() => _run('chmod', ['600', store.path]));
|
||||
|
||||
var readable = true;
|
||||
try {
|
||||
store.readAsBytesSync();
|
||||
} on FileSystemException {
|
||||
readable = false;
|
||||
}
|
||||
// Running as root (some container images) defeats the mode bits entirely.
|
||||
if (readable) {
|
||||
markTestSkipped('the store stayed readable after chmod 000; cannot stage an I/O failure here');
|
||||
return;
|
||||
}
|
||||
|
||||
final error = await SettingsService.getInstance().then<Object?>((_) => null, onError: (Object e) => e);
|
||||
|
||||
expect(error, isNotNull);
|
||||
expect(error, isNot(isA<CorruptPreferenceStoreException>()));
|
||||
});
|
||||
}
|
||||
|
||||
class _TempPathProviderLinux extends PathProviderLinux {
|
||||
_TempPathProviderLinux(this.supportPath);
|
||||
|
||||
final String supportPath;
|
||||
|
||||
@override
|
||||
Future<String?> getApplicationSupportPath() async => supportPath;
|
||||
}
|
||||
|
||||
/// Runs a POSIX helper, reporting whether it succeeded.
|
||||
///
|
||||
/// Returns false rather than throwing when the binary is missing, so a Windows
|
||||
/// run reaches `markTestSkipped` instead of failing the whole suite on a
|
||||
/// ProcessException before the test body can decide anything.
|
||||
bool _run(String executable, List<String> arguments) {
|
||||
try {
|
||||
return Process.runSync(executable, arguments).exitCode == 0;
|
||||
} on ProcessException {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user