Skip to content

Commit

Permalink
Merge pull request #1087 from atsign-foundation/fix_malformed_keys
Browse files Browse the repository at this point in the history
fix: remove invalid keys on server start-up
  • Loading branch information
sitaram-kalluri authored Dec 8, 2022
2 parents 28c3c45 + c44573e commit e1c6b4b
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 21 deletions.
4 changes: 4 additions & 0 deletions packages/at_secondary_server/config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ hive:
expiringRunFrequencyMins: 12
# List of malformed keys that will be deleted on server startup
malformedKeys: 'public:publickey'
# Delete keys that start with "public:cached" on server startup
# This is set to true by default. To retain the malformed keys on server start-up,
# set the flag to false.
shouldRemoveMalformedKeys: true

# The @Protocol connection configurations.
connection:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ class AtSecondaryConfig {
//Sync Configurations
static final int _syncBufferSize = 5242880;
static final int _syncPageLimit = 100;

// Malformed Keys
static final List<String> _malformedKeys = [];
static final bool _shouldRemoveMalformedKeys = true;

//version
static final String? _secondaryServerVersion =
Expand Down Expand Up @@ -633,6 +636,18 @@ class AtSecondaryConfig {
}
}

static bool get shouldRemoveMalformedKeys {
var result = _getBoolEnvVar('shouldRemoveMalformedKeys');
if (result != null) {
return result;
}
try {
return getConfigFromYaml(['hive', 'shouldRemoveMalformedKeys']);
} on ElementNotFoundException {
return _shouldRemoveMalformedKeys;
}
}

//implementation for config:set. This method returns a data stream which subscribers listen to for updates
static Stream<dynamic>? subscribe(ModifiableConfigs configName) {
if (testingMode) {
Expand Down
45 changes: 24 additions & 21 deletions packages/at_secondary_server/lib/src/server/at_secondary_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import 'package:at_server_spec/at_verb_spec.dart';
import 'package:at_utils/at_utils.dart';
import 'package:crypton/crypton.dart';
import 'package:uuid/uuid.dart';
import 'package:meta/meta.dart';

/// [AtSecondaryServerImpl] is a singleton class which implements [AtSecondaryServer]
class AtSecondaryServerImpl implements AtSecondaryServer {
Expand Down Expand Up @@ -82,7 +83,8 @@ class AtSecondaryServerImpl implements AtSecondaryServer {
late var commitLogCompactionJobInstance;
late var accessLogCompactionJobInstance;
late var notificationKeyStoreCompactionJobInstance;
late SecondaryPersistenceStore _secondaryPersistenceStore;
@visibleForTesting
late SecondaryPersistenceStore secondaryPersistenceStore;
late var atCommitLogCompactionConfig;
late var atAccessLogCompactionConfig;
late var atNotificationCompactionConfig;
Expand Down Expand Up @@ -148,12 +150,12 @@ class AtSecondaryServerImpl implements AtSecondaryServer {
throw AtServerException('Secondary keystore is not initialized');
}

_secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance()
secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance()
.getSecondaryPersistenceStore(currentAtSign)!;

//Commit Log Compaction
commitLogCompactionJobInstance =
AtCompactionJob(_commitLog, _secondaryPersistenceStore);
AtCompactionJob(_commitLog, secondaryPersistenceStore);
atCommitLogCompactionConfig = AtCompactionConfig(
commitLogSizeInKB!,
commitLogExpiryInDays!,
Expand All @@ -164,7 +166,7 @@ class AtSecondaryServerImpl implements AtSecondaryServer {

//Access Log Compaction
accessLogCompactionJobInstance =
AtCompactionJob(_accessLog, _secondaryPersistenceStore);
AtCompactionJob(_accessLog, secondaryPersistenceStore);
atAccessLogCompactionConfig = AtCompactionConfig(
accessLogSizeInKB!,
accessLogExpiryInDays!,
Expand All @@ -175,7 +177,7 @@ class AtSecondaryServerImpl implements AtSecondaryServer {

// Notification keystore compaction
notificationKeyStoreCompactionJobInstance = AtCompactionJob(
AtNotificationKeystore.getInstance(), _secondaryPersistenceStore);
AtNotificationKeystore.getInstance(), secondaryPersistenceStore);
atNotificationCompactionConfig = AtCompactionConfig(
AtSecondaryConfig.notificationKeyStoreSizeInKB!,
AtSecondaryConfig.notificationKeyStoreExpiryInDays!,
Expand Down Expand Up @@ -212,7 +214,7 @@ class AtSecondaryServerImpl implements AtSecondaryServer {
}

// clean up malformed keys from keystore
await _removeMalformedKeys();
await removeMalformedKeys();

try {
_isRunning = true;
Expand Down Expand Up @@ -548,7 +550,7 @@ class AtSecondaryServerImpl implements AtSecondaryServer {
_commitLog = atCommitLog;
keyStoreManager.keyStore = hiveKeyStore;
// Initialize the hive store
hiveKeyStore.init();
await hiveKeyStore.init();
serverContext!.isKeyStoreInitialized =
true; //TODO check hive for sample data
var keyStore = keyStoreManager.getKeyStore();
Expand All @@ -575,28 +577,29 @@ class AtSecondaryServerImpl implements AtSecondaryServer {
await keyStore.deleteExpiredKeys();
}

Future<void> _removeMalformedKeys() async {
try {
Future<void> removeMalformedKeys() async {
// The below code removes the invalid keys on server start-up
// Intended to remove only keys that starts with "public:cached:" or key is "public:publickey"
// Fix for the git issue: https://github.com/atsign-foundation/at_server/issues/865

// [AtSecondaryConfig.shouldRemoveMalformedKeys] is set to true by default.
// To retain the invalid keys on server start-up, set the flag to false.
if (AtSecondaryConfig.shouldRemoveMalformedKeys) {
List<String> malformedKeys = AtSecondaryConfig.malformedKeysList;
final keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!;
List<String> keys = keyStore.getKeys();
logger.finest('malformed keys from config: $malformedKeys');
for (String key in malformedKeys) {
final keyStore = _secondaryPersistenceStore.getSecondaryKeyStore()!;
if (keyStore.isKeyExists(key)) {
for (String key in keys) {
if (key.startsWith('public:cached:') || (malformedKeys.contains(key))) {
try {
int? commitId = await keyStore.remove(key);
logger.warning('commitId for removed key $key: $commitId');
// do not sync back the deleted malformed key. remove from commit log if commitId is not null
if (commitId != null) {
await _commitLog.commitLogKeyStore.remove(commitId);
}
} on KeyNotFoundException {
logger.warning('malformed $key does not exist in keystore');
continue;
} on KeyNotFoundException catch (e) {
logger
.severe('Exception in removing malformed key: ${e.toString()}');
}
}
}
} on Exception catch (e) {
logger.severe('Exception in removing malformed key: ${e.toString()}');
}
}

Expand Down
54 changes: 54 additions & 0 deletions packages/at_secondary_server/test/remove_malformed_keys_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import 'dart:collection';
import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart';
import 'package:at_secondary/src/server/at_secondary_impl.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';
import 'package:at_persistence_secondary_server/src/keystore/hive_keystore.dart';

HashMap<String, String> dummyKeyStore = HashMap();

class MockHiveKeyStore extends Mock implements HiveKeystore {
@override
List<String> getKeys({String? regex}) {
return dummyKeyStore.keys.toList();
}

@override
Future<int?> remove(String key) async {
dummyKeyStore.remove(key);
return 1;
}
}

class MockSecondaryPersistenceStore extends Mock
implements SecondaryPersistenceStore {
@override
HiveKeystore? getSecondaryKeyStore() {
return MockHiveKeyStore();
}
}

void main() {
group(
'A group of test to verify remove the malformed keys on server start-up',
() {
setUp(() {
dummyKeyStore.putIfAbsent(
'public:cached:public:publickey@alice', () => 'dummy_value');
dummyKeyStore.putIfAbsent('public:publickey@alice', () => 'dummy_value');
dummyKeyStore.putIfAbsent('public:publickey', () => 'dummy_value');
dummyKeyStore.putIfAbsent('@alice:phone@bob', () => 'dummy_value');
});
test('A test to verify only malformed keys are removed', () async {
AtSecondaryServerImpl.getInstance().secondaryPersistenceStore =
MockSecondaryPersistenceStore();
await AtSecondaryServerImpl.getInstance().removeMalformedKeys();
expect(dummyKeyStore.length, 2);
expect(dummyKeyStore.containsKey('public:publickey@alice'), true);
expect(dummyKeyStore.containsKey('@alice:phone@bob'), true);
expect(dummyKeyStore.containsKey('public:cached:public:publickey@alice'),
false);
expect(dummyKeyStore.containsKey('public:publickey'), false);
});
});
}

0 comments on commit e1c6b4b

Please sign in to comment.