diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f0195e..789bfa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## 2.1.0 - Implement permanently removing entries and groups. +- Fix merging of files with incoming deleted objects. ## 2.0.0+1 diff --git a/lib/src/kdbx_dao.dart b/lib/src/kdbx_dao.dart index be832a8..0f49545 100644 --- a/lib/src/kdbx_dao.dart +++ b/lib/src/kdbx_dao.dart @@ -1,5 +1,4 @@ import 'package:clock/clock.dart'; -import 'package:kdbx/src/kdbx_deleted_object.dart'; import 'package:kdbx/src/kdbx_entry.dart'; import 'package:kdbx/src/kdbx_file.dart'; import 'package:kdbx/src/kdbx_group.dart'; diff --git a/lib/src/kdbx_deleted_object.dart b/lib/src/kdbx_deleted_object.dart index c2ce77a..ade4df0 100644 --- a/lib/src/kdbx_deleted_object.dart +++ b/lib/src/kdbx_deleted_object.dart @@ -18,7 +18,8 @@ class KdbxDeletedObject extends KdbxNode implements KdbxNodeContext { @override final KdbxReadWriteContext ctx; - KdbxUuid? get uuid => _uuid.get(); + // all objects have to have a UUID. + KdbxUuid get uuid => _uuid.get()!; UuidNode get _uuid => UuidNode(this, KdbxXml.NODE_UUID); DateTimeUtcNode get deletionTime => DateTimeUtcNode(this, 'DeletionTime'); } diff --git a/lib/src/kdbx_format.dart b/lib/src/kdbx_format.dart index c018614..1a13998 100644 --- a/lib/src/kdbx_format.dart +++ b/lib/src/kdbx_format.dart @@ -16,6 +16,7 @@ import 'package:kdbx/src/internal/crypto_utils.dart'; import 'package:kdbx/src/internal/extension_utils.dart'; import 'package:kdbx/src/kdbx_deleted_object.dart'; import 'package:kdbx/src/kdbx_entry.dart'; +import 'package:kdbx/src/kdbx_group.dart'; import 'package:kdbx/src/kdbx_header.dart'; import 'package:kdbx/src/kdbx_xml.dart'; import 'package:kdbx/src/utils/byte_utils.dart'; @@ -338,7 +339,25 @@ class KdbxBody extends KdbxNode { // remove deleted objects for (final incomingDelete in incomingDeleted.values) { - final object = mergeContext.objectIndex![incomingDelete.uuid!]; + final object = mergeContext.objectIndex[incomingDelete.uuid]; + if (object == null) { + mergeContext.trackWarning( + 'Incoming deleted object not found locally ${incomingDelete.uuid}'); + continue; + } + final parent = object.parent; + if (parent == null) { + mergeContext.trackWarning('Unable to delete object $object - ' + 'already deleted? (${incomingDelete.uuid})'); + continue; + } + if (object is KdbxGroup) { + parent.internalRemoveGroup(object); + } else if (object is KdbxEntry) { + parent.internalRemoveEntry(object); + } else { + throw StateError('Invalid object type $object'); + } mergeContext.trackChange(object, debug: 'was deleted.'); } @@ -347,11 +366,11 @@ class KdbxBody extends KdbxNode { _logger.info('Finished merging:\n${mergeContext.debugChanges()}'); final incomingObjects = other._createObjectIndex(); _logger.info('Merged: ${mergeContext.merged} vs. ' - '(local objects: ${mergeContext.objectIndex!.length}, ' + '(local objects: ${mergeContext.objectIndex.length}, ' 'incoming objects: ${incomingObjects.length})'); // sanity checks - if (mergeContext.merged.keys.length != mergeContext.objectIndex!.length) { + if (mergeContext.merged.keys.length != mergeContext.objectIndex.length) { // TODO figure out what went wrong. } return mergeContext; @@ -429,12 +448,24 @@ class MergeChange { } } +class MergeWarning { + MergeWarning(this.debug); + + final String debug; + + @override + String toString() { + return debug; + } +} + class MergeContext implements OverwriteContext { - MergeContext({this.objectIndex, this.deletedObjects}); - final Map? objectIndex; - final Map? deletedObjects; + MergeContext({required this.objectIndex, required this.deletedObjects}); + final Map objectIndex; + final Map deletedObjects; final Map merged = {}; final List changes = []; + final List warnings = []; void markAsMerged(KdbxObject object) { if (merged.containsKey(object.uuid)) { @@ -453,6 +484,11 @@ class MergeContext implements OverwriteContext { )); } + void trackWarning(String warning) { + _logger.warning(warning, StackTrace.current); + warnings.add(MergeWarning(warning)); + } + String debugChanges() { final group = changes.groupBy((element) => element.object, valueTransform: (x) => x); diff --git a/lib/src/kdbx_group.dart b/lib/src/kdbx_group.dart index 240d36a..25af7e2 100644 --- a/lib/src/kdbx_group.dart +++ b/lib/src/kdbx_group.dart @@ -149,7 +149,7 @@ class KdbxGroup extends KdbxObject { if (meObj == null) { // moved or deleted. - final movedObj = mergeContext.objectIndex![otherObj.uuid]; + final movedObj = mergeContext.objectIndex[otherObj.uuid]; if (movedObj == null) { // item was created in the other file. we have to import it final newMeObject = importToHere(otherObj); diff --git a/test/kdbx_binaries_test.dart b/test/kdbx_binaries_test.dart index 6fc4b41..000fef1 100644 --- a/test/kdbx_binaries_test.dart +++ b/test/kdbx_binaries_test.dart @@ -2,8 +2,6 @@ import 'dart:convert'; import 'dart:typed_data'; import 'package:kdbx/kdbx.dart'; -import 'package:logging/logging.dart'; -import 'package:logging_appenders/logging_appenders.dart'; import 'package:test/test.dart'; import 'internal/test_utils.dart'; diff --git a/test/kdbx_test.dart b/test/kdbx_test.dart index defa489..a2c0280 100644 --- a/test/kdbx_test.dart +++ b/test/kdbx_test.dart @@ -5,7 +5,6 @@ import 'dart:typed_data'; import 'package:kdbx/kdbx.dart'; import 'package:kdbx/src/crypto/protected_salt_generator.dart'; import 'package:logging/logging.dart'; -import 'package:logging_appenders/logging_appenders.dart'; import 'package:synchronized/synchronized.dart'; import 'package:test/test.dart'; diff --git a/test/merge/kdbx_merge_test.dart b/test/merge/kdbx_merge_test.dart index 5a24b40..7bd46c7 100644 --- a/test/merge/kdbx_merge_test.dart +++ b/test/merge/kdbx_merge_test.dart @@ -1,10 +1,13 @@ import 'package:clock/clock.dart'; import 'package:kdbx/kdbx.dart'; +import 'package:kdbx/src/kdbx_xml.dart'; import 'package:kdbx/src/utils/print_utils.dart'; import 'package:logging/logging.dart'; import 'package:test/test.dart'; +import 'package:xml/xml.dart'; import '../internal/test_utils.dart'; +import '../kdbx_test.dart'; final _logger = Logger('kdbx_merge_test'); @@ -97,6 +100,32 @@ void main() { hasLength(2)); }), ); + test( + 'permanently delete an entry', + () async => await withClock(fakeClock, () async { + final file = await createSimpleFile(); + final objCount = file.body.rootGroup.getAllGroupsAndEntries().length; + final fileMod = await testUtil.saveAndRead(file); + final entryDelete = fileMod.body.rootGroup.entries.first; + fileMod.deletePermanently(entryDelete); + expect(fileMod.body.rootGroup.getAllGroupsAndEntries(), + hasLength(objCount - 1)); + + final file2 = await testUtil.saveAndRead(fileMod); + final merge = file.merge(file2); + _logger.info('Merged file:\n' + '${KdbxPrintUtils().catGroupToString(file.body.rootGroup)}'); + expect(merge.deletedObjects, hasLength(1)); + expect( + file.body.rootGroup.getAllGroupsAndEntries().length, objCount - 1); + final xml = file.body.generateXml(FakeProtectedSaltGenerator()); + final deleted = xml.findAllElements(KdbxXml.NODE_DELETED_OBJECT); + expect(deleted, hasLength(1)); + expect( + deleted.first.findAllElements(KdbxXml.NODE_UUID).map((e) => e.text), + [entryDelete.uuid.uuid]); + }), + ); }); }