Desktop,Mobile,Cli: Fixes #11630: Adjust how items are queried by ID

packages/lib/BaseModel.ts

@@ -55,6 +55,12 @@ export interface DeleteOptions {
 	toTrashParentId?: string;
 }
 
+interface IdsWhereClauseOptions {
+	ids: string[];
+	field?: string;
+	negate?: boolean;
+}
+
 class BaseModel {
 
 	// TODO: This ancient part of Joplin about model types is a bit of a
@@ -345,15 +351,24 @@ class BaseModel {
 		return this.modelSelectAll(q.sql, q.params);
 	}
 
+	public static whereIdsInSql({ ids, field = 'id', negate = false }: IdsWhereClauseOptions) {
+		const idsPlaceholders = ids.map(() => '?').join(',');
+		return {
+			sql: `${this.db().escapeField(field)} ${negate ? 'NOT' : ''} IN (${idsPlaceholders})`,
+			params: ids,
+		};
+	}
+
 	public static async byIds(ids: string[], options: LoadOptions = null) {
 		if (!ids.length) return [];
 		if (!options) options = {};
 		if (!options.fields) options.fields = '*';
 
 		let sql = `SELECT ${this.db().escapeFields(options.fields)} FROM \`${this.tableName()}\``;
-		sql += ` WHERE id IN ('${ids.join('\',\'')}')`;
-		const q = this.applySqlOptions(options, sql);
-		return this.modelSelectAll(q.sql);
+		const idsSql = this.whereIdsInSql({ ids });
+		sql += ` WHERE ${idsSql.sql}`;
+		const q = this.applySqlOptions(options, sql, idsSql.params);
+		return this.modelSelectAll(q);
 	}
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -750,8 +765,9 @@ class BaseModel {
 
 		options = this.modOptions(options);
 		const idFieldName = options.idFieldName ? options.idFieldName : 'id';
-		const sql = `DELETE FROM ${this.tableName()} WHERE ${idFieldName} IN ('${ids.join('\',\'')}')`;
-		await this.db().exec(sql);
+		const idsCondition = this.whereIdsInSql({ ids, field: idFieldName });
+		const sql = `DELETE FROM ${this.tableName()} WHERE ${idsCondition.sql}`;
+		await this.db().exec(sql, idsCondition.params);
 	}
 
 	public static db() {

packages/lib/models/BaseItem.test.ts

@@ -149,4 +149,14 @@ three line \\n no escape`)).toBe(0);
 		expect(await syncTime(note1.id)).toBe(newTime);
 	});
 
+	it.each([
+		'test-test!',
+		'This ID has    spaces\ttabs\nand newlines',
+		'Test`;',
+		'Test"',
+		'% test',
+	])('should support querying items with IDs containing special characters (id: %j)', async (id) => {
+		const note = await Note.save({ id }, { isNew: true });
+		expect(await BaseItem.loadItemById(note.id)).toMatchObject({ id });
+	});
 });

packages/lib/models/BaseItem.ts

@@ -243,12 +243,14 @@ export default class BaseItem extends BaseModel {
 		if (!ids.length) return [];
 
 		const classes = this.syncItemClassNames();
+		const idsSql = this.whereIdsInSql({ ids });
+
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 		let output: any[] = [];
 		for (let i = 0; i < classes.length; i++) {
 			const ItemClass = this.getClass(classes[i]);
-			const sql = `SELECT * FROM ${ItemClass.tableName()} WHERE id IN ('${ids.join('\',\'')}')`;
-			const models = await ItemClass.modelSelectAll(sql);
+			const sql = `SELECT * FROM ${ItemClass.tableName()} WHERE ${idsSql.sql}`;
+			const models = await ItemClass.modelSelectAll(sql, idsSql.params);
 			output = output.concat(models);
 		}
 		return output;
@@ -261,8 +263,9 @@ export default class BaseItem extends BaseModel {
 		const fields = options && options.fields ? options.fields : [];
 		const ItemClass = this.getClassByItemType(itemType);
 		const fieldsSql = fields.length ? this.db().escapeFields(fields) : '*';
-		const sql = `SELECT ${fieldsSql} FROM ${ItemClass.tableName()} WHERE id IN ('${ids.join('\',\'')}')`;
-		return ItemClass.modelSelectAll(sql);
+		const idsSql = this.whereIdsInSql({ ids });
+		const sql = `SELECT ${fieldsSql} FROM ${ItemClass.tableName()} WHERE ${idsSql.sql}`;
+		return ItemClass.modelSelectAll(sql, idsSql.params);
 	}
 
 	public static async loadItemByTypeAndId(itemType: ModelType, id: string, options: LoadOptions = null) {
@@ -300,7 +303,8 @@ export default class BaseItem extends BaseModel {
 		// since no other client have (or should have) them.
 		let conflictNoteIds: string[] = [];
 		if (this.modelType() === BaseModel.TYPE_NOTE) {
-			const conflictNotes = await this.db().selectAll(`SELECT id FROM notes WHERE id IN ('${ids.join('\',\'')}') AND is_conflict = 1`);
+			const idsSql = this.whereIdsInSql({ ids });
+			const conflictNotes = await this.db().selectAll(`SELECT id FROM notes WHERE ${idsSql.sql} AND is_conflict = 1`, idsSql.params);
 			conflictNoteIds = conflictNotes.map((n: NoteEntity) => {
 				return n.id;
 			});
@@ -655,13 +659,18 @@ export default class BaseItem extends BaseModel {
 			const ItemClass = this.getClass(className);
 
 			let whereSql = ['encryption_applied = 1'];
+			let params: string[] = [];
 
 			if (className === 'Resource') {
 				const blobDownloadedButEncryptedSql = 'encryption_blob_encrypted = 1 AND id IN (SELECT resource_id FROM resource_local_states WHERE fetch_status = 2))';
 				whereSql = [`(encryption_applied = 1 OR (${blobDownloadedButEncryptedSql})`];
 			}
 
-			if (exclusions.length) whereSql.push(`id NOT IN ('${exclusions.join('\',\'')}')`);
+			if (exclusions.length) {
+				const idSql = this.whereIdsInSql({ ids: exclusions, negate: true });
+				whereSql.push(idSql.sql);
+				params = params.concat(idSql.params);
+			}
 
 			const sql = sprintf(
 				`
@@ -675,7 +684,7 @@ export default class BaseItem extends BaseModel {
 				limit,
 			);
 
-			const items = await ItemClass.modelSelectAll(sql);
+			const items = await ItemClass.modelSelectAll(sql, params);
 
 			if (i >= classNames.length - 1) {
 				return { hasMore: items.length >= limit, items: items };
@@ -943,7 +952,8 @@ export default class BaseItem extends BaseModel {
 			});
 			if (!ids.length) continue;
 
-			await this.db().exec(`UPDATE sync_items SET force_sync = 1 WHERE item_id IN ('${ids.join('\',\'')}')`);
+			const idsSql = this.whereIdsInSql({ ids, field: 'item_id' });
+			await this.db().exec(`UPDATE sync_items SET force_sync = 1 WHERE ${idsSql.sql}`, idsSql.params);
 		}
 	}
 

packages/lib/models/Folder.ts

@@ -432,11 +432,14 @@ export default class Folder extends BaseItem {
 		// await this.unshareItems(ModelType.Folder, sharedFolderIds);
 
 		const sql = ['SELECT id, parent_id FROM folders WHERE share_id != \'\''];
+		let params: string[] = [];
 		if (sharedFolderIds.length) {
-			sql.push(` AND id NOT IN ('${sharedFolderIds.join('\',\'')}')`);
+			const placeholders = sharedFolderIds.map(() => '?').join(', ');
+			sql.push(` AND id NOT IN (${placeholders})`);
+			params = params.concat(sharedFolderIds);
 		}
 
-		const foldersToUnshare: FolderEntity[] = await this.db().selectAll(sql.join(' '));
+		const foldersToUnshare: FolderEntity[] = await this.db().selectAll(sql.join(' '), params);
 
 		report.unshareUpdateCount += foldersToUnshare.length;
 
@@ -540,13 +543,14 @@ export default class Folder extends BaseItem {
 			// one note. If it is not, we create duplicate resources so that
 			// each note has its own separate resource.
 
+			const idsSql = this.whereIdsInSql({ ids: resourceIds, field: 'resource_id' });
 			const noteResourceAssociations = await this.db().selectAll(`
 				SELECT resource_id, note_id, notes.share_id
 				FROM note_resources
 				LEFT JOIN notes ON notes.id = note_resources.note_id
-				WHERE resource_id IN ('${resourceIds.join('\',\'')}')
+				WHERE ${idsSql.sql}
 				AND is_associated = 1
-			`) as NoteResourceRow[];
+			`, idsSql.params) as NoteResourceRow[];
 
 			const resourceIdToNotes: Record<string, NoteResourceRow[]> = {};
 
@@ -648,15 +652,16 @@ export default class Folder extends BaseItem {
 			const fields = ['id'];
 			if (hasParentId) fields.push('parent_id');
 
+			const idsSql = this.whereIdsInSql({ ids: activeShareIds, negate: true, field: 'share_id' });
 			const query = activeShareIds.length ? `
 				SELECT ${this.db().escapeFields(fields)} FROM ${tableName}
-				WHERE share_id != '' AND share_id NOT IN ('${activeShareIds.join('\',\'')}')
+				WHERE share_id != '' AND ${idsSql.sql}
 			` : `
 				SELECT ${this.db().escapeFields(fields)} FROM ${tableName}
 				WHERE share_id != ''
 			`;
 
-			const rows = await this.db().selectAll(query);
+			const rows = await this.db().selectAll(query, idsSql.params);
 
 			report[tableName] = rows.length;
 

packages/lib/models/Note.ts

@@ -29,7 +29,7 @@ export interface PreviewsOrder {
 	dir: string;
 }
 
-interface PreviewsOptions {
+export interface PreviewsOptions {
 	order?: PreviewsOrder[];
 	conditions?: string[];
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -893,8 +893,7 @@ export default class Note extends BaseItem {
 					'updated_time = ?',
 				];
 
-				// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-				const params: any[] = [
+				let params: (string|number)[] = [
 					now,
 					now,
 				];
@@ -904,10 +903,12 @@ export default class Note extends BaseItem {
 					params.push(options.toTrashParentId);
 				}
 
+				const idsSql = this.whereIdsInSql({ ids: processIds });
+				params = params.concat(idsSql.params);
 				const sql = `
 					UPDATE notes
 					SET	${updateSql.join(', ')}						
-					WHERE id IN ('${processIds.join('\',\'')}')
+					WHERE ${idsSql.sql}
 				`;
 
 				await this.db().exec({ sql, params });

packages/lib/models/NoteResource.ts

@@ -86,13 +86,14 @@ export default class NoteResource extends BaseModel {
 		fields.push('resource_id');
 		fields.push('note_id');
 
+		const idsSql = this.whereIdsInSql({ ids: resourceIds, field: 'resource_id' });
 		const rows = await this.modelSelectAll(`
 			SELECT ${this.selectFields({ ...options, fields })}
 			FROM note_resources
 			LEFT JOIN notes
 			ON notes.id = note_resources.note_id
-			WHERE resource_id IN ('${resourceIds.join('\', \'')}') AND is_associated = 1
-		`);
+			WHERE ${idsSql.sql} AND is_associated = 1
+		`, idsSql.params);
 
 		const output: Record<string, NoteEntity[]> = {};
 		for (const row of rows) {

packages/lib/models/NoteTag.ts

@@ -12,7 +12,8 @@ export default class NoteTag extends BaseItem {
 
 	public static async byNoteIds(noteIds: string[]) {
 		if (!noteIds.length) return [];
-		return this.modelSelectAll(`SELECT * FROM note_tags WHERE note_id IN ('${noteIds.join('\',\'')}')`);
+		const idsSql = this.whereIdsInSql({ ids: noteIds, field: 'note_id' });
+		return this.modelSelectAll(`SELECT * FROM note_tags WHERE ${idsSql.sql}`, idsSql.params);
 	}
 
 	public static async tagIdsByNoteId(noteId: string) {

packages/lib/models/Resource.ts

@@ -76,7 +76,8 @@ export default class Resource extends BaseItem {
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	public static fetchStatuses(resourceIds: string[]): Promise<any[]> {
 		if (!resourceIds.length) return Promise.resolve([]);
-		return this.db().selectAll(`SELECT resource_id, fetch_status FROM resource_local_states WHERE resource_id IN ('${resourceIds.join('\',\'')}')`);
+		const idsSql = this.whereIdsInSql({ ids: resourceIds, field: 'resource_id' });
+		return this.db().selectAll(`SELECT resource_id, fetch_status FROM resource_local_states WHERE ${idsSql.sql}`, idsSql.params);
 	}
 
 	public static sharedResourceIds(): Promise<string[]> {
@@ -367,8 +368,11 @@ export default class Resource extends BaseItem {
 
 	public static async downloadedButEncryptedBlobCount(excludedIds: string[] = null) {
 		let excludedSql = '';
+		let whereParams: string[] = [];
 		if (excludedIds && excludedIds.length) {
-			excludedSql = `AND resource_id NOT IN ('${excludedIds.join('\',\'')}')`;
+			const idsSql = this.whereIdsInSql({ ids: excludedIds, field: 'resource_id', negate: true });
+			excludedSql = `AND ${idsSql.sql}`;
+			whereParams = whereParams.concat(idsSql.params);
 		}
 
 		const r = await this.db().selectOne(`
@@ -377,7 +381,7 @@ export default class Resource extends BaseItem {
 			WHERE fetch_status = ?
 			AND resource_id IN (SELECT id FROM resources WHERE encryption_blob_encrypted = 1)
 			${excludedSql}
-		`, [Resource.FETCH_STATUS_DONE]);
+		`, [Resource.FETCH_STATUS_DONE, ...whereParams]);
 
 		return r ? r.total : 0;
 	}
@@ -536,14 +540,21 @@ export default class Resource extends BaseItem {
 
 	public static async needOcr(supportedMimeTypes: string[], skippedResourceIds: string[], limit: number, options: LoadOptions): Promise<ResourceEntity[]> {
 		const query = this.baseNeedOcrQuery(this.selectFields(options), supportedMimeTypes);
-		const skippedResourcesSql = skippedResourceIds.length ? `AND resources.id NOT IN  ('${skippedResourceIds.join('\',\'')}')` : '';
+
+		let skippedResourcesSql = '';
+		let params = query.params;
+		if (skippedResourceIds.length) {
+			const idsSql = this.whereIdsInSql({ ids: skippedResourceIds, field: 'resources.id', negate: true });
+			skippedResourcesSql = `AND ${idsSql.sql}`;
+			params = params.concat(idsSql.params);
+		}
 
 		return await this.db().selectAll(`
 			${query.sql}
 			${skippedResourcesSql}			
 			ORDER BY updated_time DESC
 			LIMIT ${limit}
-		`, query.params);
+		`, params);
 	}
 
 	private static async resetOcrStatus(resourceId: string) {
@@ -576,7 +587,8 @@ export default class Resource extends BaseItem {
 	public static async resourceOcrTextsByIds(ids: string[]): Promise<ResourceEntity[]> {
 		if (!ids.length) return [];
 		ids = unique(ids);
-		return this.modelSelectAll(`SELECT id, ocr_text FROM resources WHERE id IN ('${ids.join('\',\'')}')`);
+		const idsSql = this.whereIdsInSql({ ids });
+		return this.modelSelectAll(`SELECT id, ocr_text FROM resources WHERE ${idsSql.sql}`, idsSql.params);
 	}
 
 	public static async allForNormalization(updatedTime: number, id: string, limit = 100, options: LoadOptions = null) {

packages/lib/models/Revision.ts

@@ -214,7 +214,8 @@ export default class Revision extends BaseItem {
 
 	public static async itemsWithRevisions(itemType: ModelType, itemIds: string[]) {
 		if (!itemIds.length) return [];
-		const rows = await this.db().selectAll(`SELECT distinct item_id FROM revisions WHERE item_type = ? AND item_id IN ('${itemIds.join('\',\'')}')`, [itemType]);
+		const idsSql = this.whereIdsInSql({ ids: itemIds, field: 'item_id' });
+		const rows = await this.db().selectAll(`SELECT distinct item_id FROM revisions WHERE item_type = ? AND ${idsSql.sql}`, [itemType, ...idsSql.params]);
 
 		return rows.map((r: RevisionEntity) => r.item_id);
 	}

packages/lib/models/Tag.ts

@@ -37,9 +37,10 @@ export default class Tag extends BaseItem {
 		const noteIds = await this.noteIds(tagId);
 		if (!noteIds.length) return [];
 
+		const idsSql = this.whereIdsInSql({ ids: noteIds });
 		return Note.previews(
 			null,
-			{ ...options, conditions: [`id IN ('${noteIds.join('\',\'')}')`] },
+			{ ...options, conditions: [idsSql.sql], conditionsParams: idsSql.params },
 		);
 	}
 
@@ -153,7 +154,8 @@ export default class Tag extends BaseItem {
 
 		const tagIds = await NoteTag.tagIdsByNoteId(noteId);
 		if (!tagIds.length) return [];
-		return this.modelSelectAll(`SELECT ${options.fields ? this.db().escapeFields(options.fields) : '*'} FROM tags WHERE id IN ('${tagIds.join('\',\'')}')`);
+		const idsSql = this.whereIdsInSql({ ids: tagIds });
+		return this.modelSelectAll(`SELECT ${options.fields ? this.db().escapeFields(options.fields) : '*'} FROM tags WHERE ${idsSql.sql}`, idsSql.params);
 	}
 
 	public static async commonTagsByNoteIds(noteIds: string[]) {
@@ -168,7 +170,8 @@ export default class Tag extends BaseItem {
 				break;
 			}
 		}
-		return this.modelSelectAll(`SELECT * FROM tags WHERE id IN ('${commonTagIds.join('\',\'')}')`);
+		const idsSql = this.whereIdsInSql({ ids: commonTagIds });
+		return this.modelSelectAll(`SELECT * FROM tags WHERE ${idsSql.sql}`, idsSql.params);
 	}
 
 	public static async loadByTitle(title: string): Promise<TagEntity> {

packages/lib/services/ResourceService.ts

@@ -54,7 +54,8 @@ export default class ResourceService extends BaseService {
 
 				// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 				const noteIds = changes.map((a: any) => a.item_id);
-				const notes = await Note.modelSelectAll(`SELECT id, title, body, encryption_applied FROM notes WHERE id IN ('${noteIds.join('\',\'')}')`);
+				const idsSql = Note.whereIdsInSql({ ids: noteIds });
+				const notes = await Note.modelSelectAll(`SELECT id, title, body, encryption_applied FROM notes WHERE ${idsSql.sql}`, idsSql.params);
 
 				const noteById = (noteId: string) => {
 					for (let i = 0; i < notes.length; i++) {

packages/lib/services/RevisionService.ts

@@ -138,7 +138,11 @@ export default class RevisionService extends BaseService {
 				if (!changes.length) break;
 
 				const noteIds = changes.map((a) => a.item_id);
-				const notes = await Note.modelSelectAll(`SELECT * FROM notes WHERE is_conflict = 0 AND encryption_applied = 0 AND id IN ('${noteIds.join('\',\'')}')`);
+				const idPlaceholders = noteIds.map(() => '?').join(',');
+				const notes = await Note.modelSelectAll(
+					`SELECT * FROM notes WHERE is_conflict = 0 AND encryption_applied = 0 AND id IN (${idPlaceholders})`,
+					noteIds,
+				);
 
 				for (let i = 0; i < changes.length; i++) {
 					const change = changes[i];