From 3adb6af91af54491fd71745983acb6fc8884f757 Mon Sep 17 00:00:00 2001 From: Herbert Poul Date: Thu, 10 Sep 2020 10:55:32 +0200 Subject: [PATCH] fix concurrent save bug: only allow one save at a time. --- CHANGELOG.md | 2 ++ lib/src/kdbx_file.dart | 10 +++++++++ lib/src/kdbx_format.dart | 7 +++++++ pubspec.yaml | 3 ++- test/kdbx_test.dart | 45 +++++++++++++++++++++++++++++++++------- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b192127..c689641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ - Use kdbx 4.x by default when creating new files. - Implemented support for custom icons. +- Implemented file merging/synchronization. +- Fixed threading problem on save: only allow one save at a time for each file. ## 0.4.1 diff --git a/lib/src/kdbx_file.dart b/lib/src/kdbx_file.dart index 888e422..1292e54 100644 --- a/lib/src/kdbx_file.dart +++ b/lib/src/kdbx_file.dart @@ -11,6 +11,7 @@ import 'package:kdbx/src/kdbx_header.dart'; import 'package:kdbx/src/kdbx_object.dart'; import 'package:logging/logging.dart'; import 'package:quiver/check.dart'; +import 'package:synchronized/synchronized.dart'; import 'package:xml/xml.dart' as xml; final _logger = Logger('kdbx_file'); @@ -44,6 +45,11 @@ class KdbxFile { final StreamController> _dirtyObjectsChanged = StreamController>.broadcast(); + /// lock used by [KdbxFormat] to synchronize saves, + /// because save actions are not thread save. + /// see [KdbxFileInternal.saveLock]. + final Lock _saveLock = Lock(); + Stream> get dirtyObjectsChanged => _dirtyObjectsChanged.stream; @@ -132,6 +138,10 @@ class KdbxFile { } } +extension KdbxInternal on KdbxFile { + Lock get saveLock => _saveLock; +} + class CachedValue { CachedValue.withNull() : value = null; CachedValue.withValue(this.value) : assert(value != null); diff --git a/lib/src/kdbx_format.dart b/lib/src/kdbx_format.dart index e46c769..040b053 100644 --- a/lib/src/kdbx_format.dart +++ b/lib/src/kdbx_format.dart @@ -517,7 +517,14 @@ class KdbxFormat { } } + /// Saves the given file. Future save(KdbxFile file) async { + _logger.finer('Saving ${file.body.rootGroup.uuid} ' + '(locked: ${file.saveLock.locked})'); + return file.saveLock.synchronized(() => _saveSynchronized(file)); + } + + Future _saveSynchronized(KdbxFile file) async { final body = file.body; final header = file.header; diff --git a/pubspec.yaml b/pubspec.yaml index aa02dd3..77b336a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: kdbx description: KeepassX format implementation in pure dart. (kdbx 3.x and 4.x support). -version: 0.4.1 +version: 0.4.2 homepage: https://github.com/authpass/kdbx.dart environment: @@ -24,6 +24,7 @@ dependencies: quiver: '>=2.1.0 <3.0.0' archive: '>=2.0.13 <3.0.0' supercharged_dart: '>=1.2.0 <2.0.0' + synchronized: '>=2.2.0 <3.0.0' collection: '>=1.14.0 <2.0.0' diff --git a/test/kdbx_test.dart b/test/kdbx_test.dart index 45fa15f..8b40c75 100644 --- a/test/kdbx_test.dart +++ b/test/kdbx_test.dart @@ -1,6 +1,7 @@ @Tags(['kdbx3']) import 'dart:io'; +import 'dart:typed_data'; import 'package:kdbx/kdbx.dart'; import 'package:kdbx/src/crypto/protected_salt_generator.dart'; @@ -8,10 +9,13 @@ import 'package:kdbx/src/crypto/protected_value.dart'; import 'package:kdbx/src/kdbx_format.dart'; import 'package:logging/logging.dart'; import 'package:logging_appenders/logging_appenders.dart'; +import 'package:synchronized/synchronized.dart'; import 'package:test/test.dart'; import 'internal/test_utils.dart'; +final _logger = Logger('kdbx_test'); + class FakeProtectedSaltGenerator implements ProtectedSaltGenerator { @override String decryptBase64(String protectedValue) => 'fake'; @@ -23,13 +27,13 @@ class FakeProtectedSaltGenerator implements ProtectedSaltGenerator { void main() { Logger.root.level = Level.ALL; PrintAppender().attachToLogger(Logger.root); - final kdbxForamt = KdbxFormat(); + final kdbxFormat = KdbxFormat(); group('Reading', () { setUp(() {}); test('First Test', () async { final data = await File('test/FooBar.kdbx').readAsBytes(); - await kdbxForamt.read( + await kdbxFormat.read( data, Credentials(ProtectedValue.fromString('FooBar'))); }); }); @@ -41,7 +45,7 @@ void main() { final cred = Credentials.composite( ProtectedValue.fromString('asdf'), keyFileBytes); final data = await File('test/password-and-keyfile.kdbx').readAsBytes(); - final file = await kdbxForamt.read(data, cred); + final file = await kdbxFormat.read(data, cred); expect(file.body.rootGroup.entries, hasLength(2)); }); test('Read with PW and hex keyfile', () async { @@ -50,14 +54,14 @@ void main() { final cred = Credentials.composite( ProtectedValue.fromString('testing99'), keyFileBytes); final data = await File('test/keyfile/newdatabase2.kdbx').readAsBytes(); - final file = await kdbxForamt.read(data, cred); + final file = await kdbxFormat.read(data, cred); expect(file.body.rootGroup.entries, hasLength(3)); }); }); group('Creating', () { test('Simple create', () { - final kdbx = kdbxForamt.create( + final kdbx = kdbxFormat.create( Credentials(ProtectedValue.fromString('FooBar')), 'CreateTest'); expect(kdbx, isNotNull); expect(kdbx.body.rootGroup, isNotNull); @@ -68,7 +72,7 @@ void main() { .toXmlString(pretty: true)); }); test('Create Entry', () { - final kdbx = kdbxForamt.create( + final kdbx = kdbxFormat.create( Credentials(ProtectedValue.fromString('FooBar')), 'CreateTest'); final rootGroup = kdbx.body.rootGroup; final entry = KdbxEntry.create(kdbx, rootGroup); @@ -113,7 +117,7 @@ void main() { test('Simple save and load', () async { final credentials = Credentials(ProtectedValue.fromString('FooBar')); final saved = await (() async { - final kdbx = kdbxForamt.create(credentials, 'CreateTest'); + final kdbx = kdbxFormat.create(credentials, 'CreateTest'); final rootGroup = kdbx.body.rootGroup; final entry = KdbxEntry.create(kdbx, rootGroup); rootGroup.addEntry(entry); @@ -124,7 +128,7 @@ void main() { // print(ByteUtils.toHexList(saved)); - final kdbx = await kdbxForamt.read(saved, credentials); + final kdbx = await kdbxFormat.read(saved, credentials); expect( kdbx.body.rootGroup.entries.first .getString(KdbxKeyCommon.PASSWORD) @@ -132,5 +136,30 @@ void main() { 'LoremIpsum'); File('test.kdbx').writeAsBytesSync(saved); }); + test('concurrent save test', () async { + final file = await TestUtil.readKdbxFile('test/keepass2test.kdbx'); + final readLock = Lock(); + Future doSave( + Future byteFuture, String debug) async { + _logger.fine('$debug: Waiting...'); + final bytes = await byteFuture; + return await readLock.synchronized(() { + try { + final ret = TestUtil.readKdbxFileBytes(bytes); + _logger.fine('$debug FINISHED: success'); + return ret; + } catch (e, stackTrace) { + _logger.shout( + '$debug FINISHED: error while reading file', e, stackTrace); + rethrow; + } + }); + } + + final save1 = doSave(file.save(), 'first '); + final save2 = doSave(file.save(), 'second'); + expect((await save1).body.meta.databaseName.get(), isNotNull); + expect((await save2).body.meta.databaseName.get(), isNotNull); + }); }); }