All: Improved: Better handling of items that cannot be decrypted, including those that cause crashes

CliClient/tests/services_KvStore.js

@@ -91,4 +91,17 @@ describe('services_KvStore', function() {
 		expect(await store.value('int')).toBe(20);
 	}));
 
+	it('should search by prefix', asyncTest(async () => {
+		const store = setupStore();
+		await store.setValue('testing:1', 1);
+		await store.setValue('testing:2', 2);
+
+		const results = await store.searchByPrefix('testing:');
+		expect(results.length).toBe(2);
+
+		const numbers = results.map(r => r.value).sort();
+		expect(numbers[0]).toBe(1);
+		expect(numbers[1]).toBe(2);
+	}));
+
 });

CliClient/tests/synchronizer.js

@@ -1,7 +1,7 @@
 require('app-module-path').addPath(__dirname);
 
 const { time } = require('lib/time-utils.js');
-const { setupDatabase, allSyncTargetItemsEncrypted, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, checkThrowAsync, asyncTest } = require('test-utils.js');
+const { setupDatabase, allSyncTargetItemsEncrypted, kvStore, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, checkThrowAsync, asyncTest } = require('test-utils.js');
 const { shim } = require('lib/shim.js');
 const fs = require('fs-extra');
 const Folder = require('lib/models/Folder.js');
@@ -1416,4 +1416,64 @@ describe('Synchronizer', function() {
 		expect((await Revision.all()).length).toBe(0);
 	}));
 
+	it('should stop trying to decrypt item after a few attempts', asyncTest(async () => {
+		let hasThrown;
+
+		const note = await Note.save({ title: 'ma note' });
+		const masterKey = await loadEncryptionMasterKey();
+		await encryptionService().enableEncryption(masterKey, '123456');
+		await encryptionService().loadMasterKeysFromSettings();
+		await synchronizer().start();
+
+		await switchClient(2);
+
+		await synchronizer().start();
+
+		// First, simulate a broken note and check that the decryption worker
+		// gives up decrypting after a number of tries. This is mainly relevant
+		// for data that crashes the mobile application - we don't want to keep
+		// decrypting these.
+
+		const encryptedNote = await Note.load(note.id);
+		const goodCipherText = encryptedNote.encryption_cipher_text;
+		await Note.save({ id: note.id, encryption_cipher_text: 'doesntlookright' });
+
+		Setting.setObjectKey('encryption.passwordCache', masterKey.id, '123456');
+		await encryptionService().loadMasterKeysFromSettings();
+
+		hasThrown = await checkThrowAsync(async () => await decryptionWorker().start({ errorHandler: 'throw' }));
+		expect(hasThrown).toBe(true);
+
+		hasThrown = await checkThrowAsync(async () => await decryptionWorker().start({ errorHandler: 'throw' }));
+		expect(hasThrown).toBe(true);
+
+		// Third time, an error is logged and no error is thrown
+		hasThrown = await checkThrowAsync(async () => await decryptionWorker().start({ errorHandler: 'throw' }));
+		expect(hasThrown).toBe(false);
+
+		const disabledItems = await decryptionWorker().decryptionDisabledItems();
+		expect(disabledItems.length).toBe(1);
+		expect(disabledItems[0].id).toBe(note.id);
+
+		expect((await kvStore().all()).length).toBe(1);
+		await kvStore().clear();
+
+		// Now check that if it fails once but succeed the second time, the note
+		// is correctly decrypted and the counters are cleared.
+
+		hasThrown = await checkThrowAsync(async () => await decryptionWorker().start({ errorHandler: 'throw' }));
+		expect(hasThrown).toBe(true);
+
+		await Note.save({ id: note.id, encryption_cipher_text: goodCipherText });
+
+		hasThrown = await checkThrowAsync(async () => await decryptionWorker().start({ errorHandler: 'throw' }));
+		expect(hasThrown).toBe(false);
+
+		const decryptedNote = await Note.load(note.id);
+		expect(decryptedNote.title).toBe('ma note');
+
+		expect((await kvStore().all()).length).toBe(0);
+		expect((await decryptionWorker().decryptionDisabledItems()).length).toBe(0);
+	}));
+
 });

CliClient/tests/test-utils.js

@@ -33,6 +33,7 @@ const EncryptionService = require('lib/services/EncryptionService.js');
 const DecryptionWorker = require('lib/services/DecryptionWorker.js');
 const ResourceService = require('lib/services/ResourceService.js');
 const RevisionService = require('lib/services/RevisionService.js');
+const KvStore = require('lib/services/KvStore.js');
 const WebDavApi = require('lib/WebDavApi');
 const DropboxApi = require('lib/DropboxApi');
 
@@ -42,6 +43,7 @@ let encryptionServices_ = [];
 let revisionServices_ = [];
 let decryptionWorkers_ = [];
 let resourceServices_ = [];
+let kvStores_ = [];
 let fileApi_ = null;
 let currentClient_ = 1;
 
@@ -224,6 +226,7 @@ async function setupDatabaseAndSynchronizer(id = null) {
 	decryptionWorkers_[id] = new DecryptionWorker();
 	decryptionWorkers_[id].setEncryptionService(encryptionServices_[id]);
 	resourceServices_[id] = new ResourceService();
+	kvStores_[id] = new KvStore();
 
 	await fileApi().clearRoot();
 }
@@ -243,6 +246,13 @@ function encryptionService(id = null) {
 	return encryptionServices_[id];
 }
 
+function kvStore(id = null) {
+	if (id === null) id = currentClient_;
+	const o = kvStores_[id];
+	o.setDb(db(id));
+	return o;
+}
+
 function revisionService(id = null) {
 	if (id === null) id = currentClient_;
 	return revisionServices_[id];
@@ -250,7 +260,9 @@ function revisionService(id = null) {
 
 function decryptionWorker(id = null) {
 	if (id === null) id = currentClient_;
-	return decryptionWorkers_[id];
+	const o = decryptionWorkers_[id];
+	o.setKvStore(kvStore(id));
+	return o;
 }
 
 function resourceService(id = null) {
@@ -380,4 +392,4 @@ async function allSyncTargetItemsEncrypted() {
 	return totalCount === encryptedCount;
 }
 
-module.exports = { resourceService, allSyncTargetItemsEncrypted, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest };
+module.exports = { kvStore, resourceService, allSyncTargetItemsEncrypted, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest };

ElectronClient/app/app.js

@@ -1075,6 +1075,7 @@ class Application extends BaseApplication {
 		// Make it available to the console window - useful to call revisionService.collectRevisions()
 		window.revisionService = RevisionService.instance();
 		window.migrationService = MigrationService.instance();
+		window.decryptionWorker = DecryptionWorker.instance();
 	}
 
 }

ElectronClient/app/gui/StatusScreen.jsx

@@ -48,6 +48,7 @@ class StatusScreenComponent extends React.Component {
 		const style = this.props.style;
 
 		const headerStyle = Object.assign({}, theme.headerStyle, { width: style.width });
+		const retryStyle = Object.assign({}, theme.urlStyle, {marginLeft: 5});
 
 		const containerPadding = 10;
 
@@ -60,16 +61,34 @@ class StatusScreenComponent extends React.Component {
 			return <h2 key={'section_' + key} style={theme.h2Style}>{title}</h2>
 		}
 
-		function renderSectionHtml(key, section) {
+		const renderSectionHtml = (key, section) => {
 			let itemsHtml = [];
 
 			itemsHtml.push(renderSectionTitleHtml(section.title, section.title));
 
 			for (let n in section.body) {
 				if (!section.body.hasOwnProperty(n)) continue;
-				let text = section.body[n];
+				let item = section.body[n];
+				let text = '';
+
+				let retryLink = null;
+				if (typeof item === 'object') {
+					if (item.canRetry) {
+						const onClick = async () => {
+							await item.retryHandler();
+							this.resfreshScreen();
+						}
+
+						retryLink = <a href="#" onClick={onClick} style={retryStyle}>{_('Retry')}</a>;
+					}
+					text = item.text;
+				} else {
+					text = item;
+				}
+
 				if (!text) text = '\xa0';
-				itemsHtml.push(<div style={theme.textStyle} key={'item_' + n}>{text}</div>);
+
+				itemsHtml.push(<div style={theme.textStyle} key={'item_' + n}><span>{text}</span>{retryLink}</div>);
 			}
 
 			return (

ReactNativeClient/lib/BaseApplication.js

@@ -39,6 +39,7 @@ const RevisionService = require('lib/services/RevisionService');
 const DecryptionWorker = require('lib/services/DecryptionWorker');
 const BaseService = require('lib/services/BaseService');
 const SearchEngine = require('lib/services/SearchEngine');
+const KvStore = require('lib/services/KvStore');
 const MigrationService = require('lib/services/MigrationService');
 
 SyncTargetRegistry.addClass(SyncTargetFilesystem);
@@ -617,11 +618,14 @@ class BaseApplication {
 
 		BaseItem.revisionService_ = RevisionService.instance();
 
+		KvStore.instance().setDb(reg.db());
+
 		BaseService.logger_ = this.logger_;
 		EncryptionService.instance().setLogger(this.logger_);
 		BaseItem.encryptionService_ = EncryptionService.instance();
 		DecryptionWorker.instance().setLogger(this.logger_);
 		DecryptionWorker.instance().setEncryptionService(EncryptionService.instance());
+		DecryptionWorker.instance().setKvStore(KvStore.instance());
 		await EncryptionService.instance().loadMasterKeysFromSettings();
 		DecryptionWorker.instance().on('resourceMetadataButNotBlobDecrypted', this.decryptionWorker_resourceMetadataButNotBlobDecrypted);
 

ReactNativeClient/lib/components/screens/status.js

@@ -47,7 +47,7 @@ class StatusScreenComponent extends BaseScreenComponent {
 	render() {
 		const theme = themeStyle(this.props.theme);
 
-		function renderBody(report) {
+		const renderBody = report => {
 			let output = [];
 			let baseStyle = {
 				paddingLeft: 6,
@@ -72,7 +72,24 @@ class StatusScreenComponent extends BaseScreenComponent {
 				for (let n in section.body) {
 					if (!section.body.hasOwnProperty(n)) continue;
 					style = Object.assign({}, baseStyle);
-					lines.push({ key: 'item_' + i + '_' + n, text: section.body[n] });
+					const item = section.body[n];
+
+					let text = '';
+
+					let retryHandler = null;
+					if (typeof item === 'object') {
+						if (item.canRetry) {
+							retryHandler = async () => {
+								await item.retryHandler();
+								this.resfreshScreen();
+							}
+						}
+						text = item.text;
+					} else {
+						text = item;
+					}
+
+					lines.push({ key: 'item_' + i + '_' + n, text: text, retryHandler: retryHandler });
 				}
 
 				lines.push({ key: 'divider2_' + i, isDivider: true });
@@ -82,14 +99,25 @@ class StatusScreenComponent extends BaseScreenComponent {
 				data={lines}
 				renderItem={({item}) => {
 					let style = Object.assign({}, baseStyle);
+
 					if (item.isSection === true) {
 						style.fontWeight = 'bold';
 						style.marginBottom = 5;
 					}
+
+					style.flex = 1;
+
+					const retryButton = item.retryHandler ? <View style={{flex:0}}><Button title={_('Retry')} onPress={item.retryHandler}/></View> : null;
+
 					if (item.isDivider) {
-						return (<View style={{borderBottomWidth: 1, borderBottomColor: 'white', marginTop: 20, marginBottom: 20}}/>);
+						return (<View style={{borderBottomWidth: 1, borderBottomColor: theme.dividerColor, marginTop: 20, marginBottom: 20}}/>);
 					} else {
-						return (<Text style={style}>{item.text}</Text>);
+						return (
+							<View style={{flex:1, flexDirection:'row'}}>
+								<Text style={style}>{item.text}</Text>
+								{retryButton}
+							</View>
+						);
 					}
 				}}
 			/>);

ReactNativeClient/lib/joplin-database.js

@@ -178,6 +178,7 @@ class JoplinDatabase extends Database {
 			'notes_normalized',
 			'revisions',
 			'resources_to_download',
+			'key_values',
 		];
 
 		const queries = [];
@@ -186,6 +187,12 @@ class JoplinDatabase extends Database {
 			queries.push('DELETE FROM sqlite_sequence WHERE name="' + n + '"'); // Reset autoincremented IDs
 		}
 
+		queries.push('DELETE FROM settings WHERE key="sync.1.context"');
+		queries.push('DELETE FROM settings WHERE key="sync.2.context"');
+		queries.push('DELETE FROM settings WHERE key="sync.3.context"');
+		queries.push('DELETE FROM settings WHERE key="sync.4.context"');
+		queries.push('DELETE FROM settings WHERE key="sync.5.context"');
+		queries.push('DELETE FROM settings WHERE key="sync.6.context"');
 		queries.push('DELETE FROM settings WHERE key="sync.7.context"');
 
 		await this.transactionExecBatch(queries);

ReactNativeClient/lib/services/DecryptionWorker.js

@@ -2,6 +2,7 @@ const BaseItem = require('lib/models/BaseItem');
 const MasterKey = require('lib/models/MasterKey');
 const Resource = require('lib/models/Resource');
 const ResourceService = require('lib/services/ResourceService');
+const KvStore = require('lib/services/KvStore');
 const { Logger } = require('lib/logger.js');
 const EventEmitter = require('events');
 
@@ -17,6 +18,8 @@ class DecryptionWorker {
 
 		this.scheduleId_ = null;
 		this.eventEmitter_ = new EventEmitter();
+		this.kvStore_ = null;
+		this.maxDecryptionAttempts_ = 2;
 	}
 
 	setLogger(l) {
@@ -45,11 +48,20 @@ class DecryptionWorker {
 		this.encryptionService_ = v;
 	}
 
+	setKvStore(v) {
+		this.kvStore_ = v;
+	}
+
 	encryptionService() {
 		if (!this.encryptionService_) throw new Error('DecryptionWorker.encryptionService_ is not set!!');
 		return this.encryptionService_;
 	}
 
+	kvStore() {
+		if (!this.kvStore_) throw new Error('DecryptionWorker.kvStore_ is not set!!');
+		return this.kvStore_;
+	}
+
 	async scheduleStart() {
 		if (this.scheduleId_) return;
 
@@ -61,6 +73,23 @@ class DecryptionWorker {
 		}, 1000);
 	}
 
+	async decryptionDisabledItems() {
+		let items = await this.kvStore().searchByPrefix('decrypt:');
+		items = items.filter(item => item.value > this.maxDecryptionAttempts_);
+		items = items.map(item => {
+			const s = item.key.split(':');
+			return {
+				type_: Number(s[1]),
+				id: s[2],
+			};
+		});
+		return items;
+	}
+
+	async clearDisabledItem(typeId, itemId) {
+		await this.kvStore().deleteValue('decrypt:' + typeId + ':' + itemId);
+	}
+
 	dispatchReport(report) {
 		const action = Object.assign({}, report);
 		action.type = 'DECRYPTION_WORKER_SET';
@@ -70,6 +99,7 @@ class DecryptionWorker {
 	async start(options = null) {
 		if (options === null) options = {};
 		if (!('masterKeyNotLoadedHandler' in options)) options.masterKeyNotLoadedHandler = 'throw';
+		if (!('errorHandler' in options)) options.errorHandler = 'log';
 
 		if (this.state_ !== 'idle') {
 			this.logger().info('DecryptionWorker: cannot start because state is "' + this.state_ + '"');
@@ -114,12 +144,27 @@ class DecryptionWorker {
 						itemIndex: i,
 						itemCount: items.length,
 					});
+
+					const counterKey = 'decrypt:' + item.type_ + ':' + item.id;
+
+					const clearDecryptionCounter = async () => {
+						await this.kvStore().deleteValue(counterKey);
+					}
 					
 					// Don't log in production as it results in many messages when importing many items
 					// this.logger().debug('DecryptionWorker: decrypting: ' + item.id + ' (' + ItemClass.tableName() + ')');
 					try {
+						const decryptCounter = await this.kvStore().incValue(counterKey);
+						if (decryptCounter > this.maxDecryptionAttempts_) {
+							this.logger().warn('DecryptionWorker: ' + item.id + ' decryption has failed more than 2 times - skipping it');
+							excludedIds.push(item.id);
+							continue;
+						}
+
 						const decryptedItem = await ItemClass.decrypt(item);
 
+						await clearDecryptionCounter();
+
 						if (decryptedItem.type_ === Resource.modelType() && !!decryptedItem.encryption_blob_encrypted) {
 							// itemsThatNeedDecryption() will return the resource again if the blob has not been decrypted,
 							// but that will result in an infinite loop if the blob simply has not been downloaded yet.
@@ -145,14 +190,20 @@ class DecryptionWorker {
 								});
 								notLoadedMasterKeyDisptaches.push(error.masterKeyId);
 							}
+							await clearDecryptionCounter();
 							continue;
 						}
 
 						if (error.code === 'masterKeyNotLoaded' && options.masterKeyNotLoadedHandler === 'throw') {
+							await clearDecryptionCounter();
 							throw error;
 						}
 
-						this.logger().warn('DecryptionWorker: error for: ' + item.id + ' (' + ItemClass.tableName() + ')', error, item);
+						if (options.errorHandler === 'log') {
+							this.logger().warn('DecryptionWorker: error for: ' + item.id + ' (' + ItemClass.tableName() + ')', error, item);
+						} else {
+							throw error;
+						}
 					}
 				}
 

ReactNativeClient/lib/services/KvStore.js

@@ -29,6 +29,15 @@ class KvStore extends BaseService {
 		throw new Error('Unsupported value type: ' + (typeof value));
 	}
 
+	formatValues_(kvs) {
+		const output = [];
+		for (const kv of kvs) {
+			kv.value = this.formatValue_(kv.value, kv.type);
+			output.push(kv);
+		}
+		return output;
+	}
+
 	formatValue_(value, type) {
 		if (type === KvStore.TYPE_INT) return Number(value);
 		if (type === KvStore.TYPE_TEXT) return value + '';
@@ -50,6 +59,14 @@ class KvStore extends BaseService {
 		await this.db().exec('DELETE FROM key_values WHERE `key` = ?', [key]);
 	}
 
+	async clear() {
+		await this.db().exec('DELETE FROM key_values');
+	}
+
+	async all() {
+		return this.formatValues_(await this.db().selectAll('SELECT * FROM key_values'));
+	}
+
 	// Note: atomicity is done at application level so two difference instances
 	// accessing the db at the same time could mess up the increment.
 	async incValue(key, inc = 1) {
@@ -67,6 +84,11 @@ class KvStore extends BaseService {
 		}		
 	}
 
+	async searchByPrefix(prefix) {
+		let results = await this.db().selectAll('SELECT `key`, `value`, `type` FROM key_values WHERE `key` LIKE ?', [prefix + '%']);
+		return this.formatValues_(results);
+	}
+
 	async countKeys() {
 		const r = await this.db().selectOne('SELECT count(*) as total FROM key_values');
 		return r.total ? r.total : 0;

ReactNativeClient/lib/services/report.js

@@ -3,7 +3,10 @@ const BaseItem = require('lib/models/BaseItem.js');
 const Alarm = require('lib/models/Alarm');
 const Folder = require('lib/models/Folder.js');
 const Note = require('lib/models/Note.js');
+const BaseModel = require('lib/BaseModel.js');
+const DecryptionWorker = require('lib/services/DecryptionWorker');
 const { _ } = require('lib/locale.js');
+const { toTitleCase } = require('lib/string-utils.js');
 
 class ReportService {
 
@@ -116,6 +119,10 @@ class ReportService {
 		if (disabledItems.length) {
 			section = { title: _('Items that cannot be synchronised'), body: [] };
 
+			section.body.push(_('These items will remain on the device but will not be uploaded to the sync target. In order to find these items, either search for the title or the ID (which is displayed in brackets above).'));
+
+			section.body.push('');
+
 			for (let i = 0; i < disabledItems.length; i++) {
 				const row = disabledItems[i];
 				if (row.location === BaseItem.SYNC_ITEM_LOCATION_LOCAL) {
@@ -125,8 +132,25 @@ class ReportService {
 				}
 			}
 
+			sections.push(section);
+		}
+
+		const decryptionDisabledItems = await DecryptionWorker.instance().decryptionDisabledItems();
+
+		if (decryptionDisabledItems.length) {
+			section = { title: _('Items that cannot be decrypted'), body: [], name: 'failedDecryption' };
+
+			section.body.push(_('Joplin failed to decrypt these items multiple times, possibly because they are corrupted or too large. These items will remain on the device but Joplin will no longer attempt to decrypt them.'));
+
 			section.body.push('');
-			section.body.push(_('These items will remain on the device but will not be uploaded to the sync target. In order to find these items, either search for the title or the ID (which is displayed in brackets above).'));
+			
+			for (let i = 0; i < decryptionDisabledItems.length; i++) {
+				const row = decryptionDisabledItems[i];
+				section.body.push({ text: _('%s: %s', toTitleCase(BaseModel.modelTypeToName(row.type_)), row.id), canRetry: true, retryHandler: async () => {
+					await DecryptionWorker.instance().clearDisabledItem(row.type_, row.id);
+					DecryptionWorker.instance().scheduleStart();
+				}});
+			}
 
 			sections.push(section);
 		}

ReactNativeClient/root.js

@@ -27,6 +27,7 @@ const BaseModel = require('lib/BaseModel.js');
 const BaseService = require('lib/services/BaseService.js');
 const ResourceService = require('lib/services/ResourceService');
 const RevisionService = require('lib/services/RevisionService');
+const KvStore = require('lib/services/KvStore');
 const { JoplinDatabase } = require('lib/joplin-database.js');
 const { Database } = require('lib/database.js');
 const { NotesScreen } = require('lib/components/screens/notes.js');
@@ -388,6 +389,8 @@ async function initialize(dispatch) {
 	NavService.dispatch = dispatch;
 	BaseModel.db_ = db;
 
+	KvStore.instance().setDb(reg.db());
+
 	BaseItem.loadClass('Note', Note);
 	BaseItem.loadClass('Folder', Folder);
 	BaseItem.loadClass('Resource', Resource);
@@ -455,6 +458,7 @@ async function initialize(dispatch) {
 		BaseItem.encryptionService_ = EncryptionService.instance();
 		DecryptionWorker.instance().dispatch = dispatch;
 		DecryptionWorker.instance().setLogger(mainLogger);
+		DecryptionWorker.instance().setKvStore(KvStore.instance());
 		DecryptionWorker.instance().setEncryptionService(EncryptionService.instance());
 		await EncryptionService.instance().loadMasterKeysFromSettings();