diff --git a/lib/features/labels/adapters/labels_repository_adapter.dart b/lib/features/labels/adapters/labels_repository_adapter.dart index 8586b546ae..7ecea15550 100644 --- a/lib/features/labels/adapters/labels_repository_adapter.dart +++ b/lib/features/labels/adapters/labels_repository_adapter.dart @@ -1,4 +1,5 @@ import 'package:bb_mobile/core/storage/storage.dart'; +import 'package:bb_mobile/core/utils/logger.dart'; import 'package:bb_mobile/features/labels/adapters/label_mapper.dart'; import 'package:bb_mobile/features/labels/application/labels_repository_port.dart'; import 'package:bb_mobile/features/labels/domain/label_entity.dart'; @@ -11,6 +12,21 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort { @override Future store(NewLabel newLabel) async { + // Validate BEFORE writing: constructing a LabelEntity is what enforces + // its invariants (see LabelEntity._validateReference), and it must + // throw here — before the insert below — or a caller told the store + // failed has in fact already had its row persisted (the previous shape + // built the companion from the unvalidated newLabel directly and only + // constructed a LabelEntity afterwards, purely to shape the return + // value, by which point the row was already committed). + LabelEntity( + id: 0, // unknown before insert; only the validation side effect matters + type: newLabel.type, + label: newLabel.label, + reference: newLabel.reference, + origin: newLabel.origin, + ); + final companion = LabelMapper.newLabelEntityToCompanion(newLabel); final id = await _database .into(_database.labels) @@ -36,7 +52,7 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort { final rows = await _database.managers.labels .filter((l) => l.label(label)) .get(); - return rows.map((row) => LabelMapper.toLabelEntity(row)).toList(); + return _mapRowsTolerantly(rows); } @override @@ -44,7 +60,7 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort { final rows = await _database.managers.labels .filter((l) => l.reference(reference)) .get(); - return rows.map((row) => LabelMapper.toLabelEntity(row)).toList(); + return _mapRowsTolerantly(rows); } @override @@ -63,6 +79,29 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort { @override Future> fetchAll() async { final rows = await _database.managers.labels.get(); - return rows.map((row) => LabelMapper.toLabelEntity(row)).toList(); + return _mapRowsTolerantly(rows); + } + + /// Maps each row independently and drops (with a log) any row that fails + /// [LabelEntity]'s validation, instead of letting `.map().toList()` + /// propagate the first bad row's exception and discard every valid label + /// in the same query. Every fetch method here feeds every label lookup in + /// the app (including the wallet transaction list's per-input/output + /// label enrichment), so one corrupt row used to silently blank out label + /// data everywhere it was read. + List _mapRowsTolerantly(List rows) { + final entities = []; + for (final row in rows) { + try { + entities.add(LabelMapper.toLabelEntity(row)); + } catch (e) { + log.warning( + 'Skipping corrupt label row id=${row.id}: failed to map to a ' + 'LabelEntity', + error: e, + ); + } + } + return entities; } } diff --git a/lib/features/labels/domain/label_entity.dart b/lib/features/labels/domain/label_entity.dart index 09e15d1bf1..9887266737 100644 --- a/lib/features/labels/domain/label_entity.dart +++ b/lib/features/labels/domain/label_entity.dart @@ -42,14 +42,20 @@ class LabelEntity { } void _validateTxid(String input) { - if (reference.length != 64) { + // Validates the passed slice, not the full `reference` field: for + // LabelType.input/output/publicKey, reference is `txid:vout` and would + // never pass a 64-hex-char check on its own — every well-formed label + // of those three types was being rejected unconditionally before this + // fix (the txid slice was already split out by the caller into + // `input`, but validation looked at the untouched full field instead). + if (input.length != 64) { throw LabelValidationException( 'Invalid transaction reference: must be 64 hex characters', ); } try { - hex.decode(reference); + hex.decode(input); } catch (e) { throw LabelValidationException( 'Invalid transaction reference: must be valid hex', diff --git a/test/features/labels/adapters/labels_repository_adapter_test.dart b/test/features/labels/adapters/labels_repository_adapter_test.dart new file mode 100644 index 0000000000..e63e2d134e --- /dev/null +++ b/test/features/labels/adapters/labels_repository_adapter_test.dart @@ -0,0 +1,97 @@ +import 'package:bb_mobile/core/storage/sqlite_database.dart'; +import 'package:bb_mobile/core/storage/tables/labels_table.dart'; +import 'package:bb_mobile/features/labels/adapters/labels_repository_adapter.dart'; +import 'package:bb_mobile/features/labels/domain/label_entity.dart'; +import 'package:bb_mobile/features/labels/domain/new_label.dart'; +import 'package:bb_mobile/features/labels/domain/primitive/label_type.dart'; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + late SqliteDatabase db; + late DriftLabelsRepositoryAdapter adapter; + + setUp(() { + db = SqliteDatabase(NativeDatabase.memory()); + adapter = DriftLabelsRepositoryAdapter(database: db); + }); + + tearDown(() async => db.close()); + + group('store', () { + test('never persists a row when the label fails validation', () async { + // A transaction reference must be 64 hex chars — this one isn't. + await expectLater( + () => adapter.store( + NewLabel( + type: LabelType.transaction, + label: 'test', + reference: 'too-short', + ), + ), + throwsA(isA()), + ); + + final rows = await db.select(db.labels).get(); + expect( + rows, + isEmpty, + reason: + 'a rejected store must never reach the DB — validating before ' + 'the insert is the whole point of this fix', + ); + }); + + test('persists a valid label and returns it with its id', () async { + final stored = await adapter.store( + NewLabel( + type: LabelType.transaction, + label: 'payjoin', + reference: 'a' * 64, + ), + ); + + expect(stored.id, greaterThan(0)); + final rows = await db.select(db.labels).get(); + expect(rows, hasLength(1)); + }); + }); + + group('fetchAll / fetchByReference tolerate a corrupt row', () { + test('a single corrupt row is skipped and logged, not letting it discard ' + 'every valid label in the same query', () async { + // Insert one valid row through the adapter (validated). + await adapter.store( + NewLabel( + type: LabelType.transaction, + label: 'payjoin', + reference: 'a' * 64, + ), + ); + + // Insert a corrupt row directly at the DB level, bypassing + // LabelEntity's validation (simulates data that predates a + // validation fix, or any other source of a malformed row). + await db + .into(db.labels) + .insert( + LabelsCompanion.insert( + label: 'corrupt', + reference: 'not-a-valid-64-char-txid', + type: LabelTypeColumn.tx, + ), + ); + + final all = await adapter.fetchAll(); + expect( + all, + hasLength(1), + reason: 'the corrupt row must be dropped, not poison the whole batch', + ); + expect(all.single.label, 'payjoin'); + + final byReference = await adapter.fetchByReference('a' * 64); + expect(byReference, hasLength(1)); + }); + }); +} diff --git a/test/features/labels/domain/label_entity_test.dart b/test/features/labels/domain/label_entity_test.dart new file mode 100644 index 0000000000..8b746355ba --- /dev/null +++ b/test/features/labels/domain/label_entity_test.dart @@ -0,0 +1,95 @@ +import 'package:bb_mobile/features/labels/domain/label_entity.dart'; +import 'package:bb_mobile/features/labels/domain/primitive/label_type.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('LabelEntity reference validation', () { + final validTxid = 'a' * 64; + + test('accepts a well-formed transaction reference', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.transaction, + label: 'test', + reference: validTxid, + ), + returnsNormally, + ); + }); + + test('accepts a well-formed input reference (txid:vout) — regression for ' + 'the bug where the full reference was validated instead of the txid ' + 'slice, rejecting every well-formed input/output/publicKey label', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.input, + label: 'test', + reference: '$validTxid:0', + ), + returnsNormally, + ); + }); + + test('accepts a well-formed output reference (txid:vout)', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.output, + label: 'test', + reference: '$validTxid:12', + ), + returnsNormally, + ); + }); + + test('accepts a well-formed publicKey reference (txid:vout)', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.publicKey, + label: 'test', + reference: '$validTxid:1', + ), + returnsNormally, + ); + }); + + test('rejects an input reference with a non-hex txid slice', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.input, + label: 'test', + reference: '${'z' * 64}:0', + ), + throwsA(isA()), + ); + }); + + test('rejects an input reference with a negative vout', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.input, + label: 'test', + reference: '$validTxid:-1', + ), + throwsA(isA()), + ); + }); + + test('rejects a transaction reference that is not 64 hex characters', () { + expect( + () => LabelEntity( + id: 1, + type: LabelType.transaction, + label: 'test', + reference: 'tooshort', + ), + throwsA(isA()), + ); + }); + }); +}