1
0
mirror of https://github.com/laurent22/joplin.git synced 2025-11-26 22:41:17 +02:00

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

This commit is contained in:
Henry Heino
2025-01-27 10:34:46 -08:00
committed by GitHub
parent aa6348a127
commit cc1582d535
15 changed files with 59 additions and 32 deletions

View File

@@ -345,13 +345,17 @@ class BaseModel {
return this.modelSelectAll(q.sql, q.params);
}
public static escapeIdsForSql(ids: string[]) {
return this.db().escapeValues(ids).join(', ');
}
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('\',\'')}')`;
sql += ` WHERE id IN (${this.escapeIdsForSql(ids)})`;
const q = this.applySqlOptions(options, sql);
return this.modelSelectAll(q.sql);
}
@@ -750,7 +754,7 @@ class BaseModel {
options = this.modOptions(options);
const idFieldName = options.idFieldName ? options.idFieldName : 'id';
const sql = `DELETE FROM ${this.tableName()} WHERE ${idFieldName} IN ('${ids.join('\',\'')}')`;
const sql = `DELETE FROM ${this.tableName()} WHERE ${idFieldName} IN (${this.escapeIdsForSql(ids)})`;
await this.db().exec(sql);
}

View File

@@ -84,6 +84,13 @@ export default class Database {
return output;
}
public escapeValues(values: string[]) {
return values.map(value => {
// See https://www.sqlite.org/printf.html#percentq
return `'${value.replace(/[']/g, '\'\'')}'`;
});
}
public escapeFieldsToString(fields: string[] | string): string {
if (typeof fields === 'string') {
if (fields === '*') return '*';

View File

@@ -149,4 +149,16 @@ 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\'',
'Test\'\'\'a\'\'',
'% 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 });
});
});

View File

@@ -243,11 +243,12 @@ export default class BaseItem extends BaseModel {
if (!ids.length) return [];
const classes = this.syncItemClassNames();
// 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 sql = `SELECT * FROM ${ItemClass.tableName()} WHERE id IN (${this.escapeIdsForSql(ids)})`;
const models = await ItemClass.modelSelectAll(sql);
output = output.concat(models);
}
@@ -261,7 +262,7 @@ 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('\',\'')}')`;
const sql = `SELECT ${fieldsSql} FROM ${ItemClass.tableName()} WHERE id IN (${this.escapeIdsForSql(ids)})`;
return ItemClass.modelSelectAll(sql);
}
@@ -300,7 +301,7 @@ 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 conflictNotes = await this.db().selectAll(`SELECT id FROM notes WHERE id IN (${this.escapeIdsForSql(ids)}) AND is_conflict = 1`);
conflictNoteIds = conflictNotes.map((n: NoteEntity) => {
return n.id;
});
@@ -661,7 +662,9 @@ export default class BaseItem extends BaseModel {
whereSql = [`(encryption_applied = 1 OR (${blobDownloadedButEncryptedSql})`];
}
if (exclusions.length) whereSql.push(`id NOT IN ('${exclusions.join('\',\'')}')`);
if (exclusions.length) {
whereSql.push(`id NOT IN (${this.escapeIdsForSql(exclusions)})`);
}
const sql = sprintf(
`
@@ -943,7 +946,7 @@ 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('\',\'')}')`);
await this.db().exec(`UPDATE sync_items SET force_sync = 1 WHERE item_id IN (${this.escapeIdsForSql(ids)})`);
}
}

View File

@@ -433,7 +433,7 @@ export default class Folder extends BaseItem {
const sql = ['SELECT id, parent_id FROM folders WHERE share_id != \'\''];
if (sharedFolderIds.length) {
sql.push(` AND id NOT IN ('${sharedFolderIds.join('\',\'')}')`);
sql.push(` AND id NOT IN (${Folder.escapeIdsForSql(sharedFolderIds)})`);
}
const foldersToUnshare: FolderEntity[] = await this.db().selectAll(sql.join(' '));
@@ -544,7 +544,7 @@ export default class Folder extends BaseItem {
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 resource_id IN (${this.escapeIdsForSql(resourceIds)})
AND is_associated = 1
`) as NoteResourceRow[];
@@ -650,7 +650,7 @@ export default class Folder extends BaseItem {
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 share_id NOT IN (${this.escapeIdsForSql(activeShareIds)})
` : `
SELECT ${this.db().escapeFields(fields)} FROM ${tableName}
WHERE share_id != ''

View File

@@ -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[] = [
const params: (string|number)[] = [
now,
now,
];
@@ -907,7 +906,7 @@ export default class Note extends BaseItem {
const sql = `
UPDATE notes
SET ${updateSql.join(', ')}
WHERE id IN ('${processIds.join('\',\'')}')
WHERE id IN (${this.escapeIdsForSql(processIds)})
`;
await this.db().exec({ sql, params });

View File

@@ -91,7 +91,7 @@ export default class NoteResource extends BaseModel {
FROM note_resources
LEFT JOIN notes
ON notes.id = note_resources.note_id
WHERE resource_id IN ('${resourceIds.join('\', \'')}') AND is_associated = 1
WHERE resource_id IN (${this.escapeIdsForSql(resourceIds)}) AND is_associated = 1
`);
const output: Record<string, NoteEntity[]> = {};

View File

@@ -12,7 +12,7 @@ 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('\',\'')}')`);
return this.modelSelectAll(`SELECT * FROM note_tags WHERE note_id IN (${this.escapeIdsForSql(noteIds)})`);
}
public static async tagIdsByNoteId(noteId: string) {

View File

@@ -76,7 +76,7 @@ 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('\',\'')}')`);
return this.db().selectAll(`SELECT resource_id, fetch_status FROM resource_local_states WHERE resource_id IN (${this.escapeIdsForSql(resourceIds)})`);
}
public static sharedResourceIds(): Promise<string[]> {
@@ -368,7 +368,7 @@ export default class Resource extends BaseItem {
public static async downloadedButEncryptedBlobCount(excludedIds: string[] = null) {
let excludedSql = '';
if (excludedIds && excludedIds.length) {
excludedSql = `AND resource_id NOT IN ('${excludedIds.join('\',\'')}')`;
excludedSql = `AND resource_id NOT IN (${this.escapeIdsForSql(excludedIds)})`;
}
const r = await this.db().selectOne(`
@@ -536,7 +536,7 @@ 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('\',\'')}')` : '';
const skippedResourcesSql = skippedResourceIds.length ? `AND resources.id NOT IN (${this.escapeIdsForSql(skippedResourceIds)})` : '';
return await this.db().selectAll(`
${query.sql}
@@ -576,7 +576,7 @@ 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('\',\'')}')`);
return this.modelSelectAll(`SELECT id, ocr_text FROM resources WHERE id IN (${this.escapeIdsForSql(ids)})`);
}
public static async allForNormalization(updatedTime: number, id: string, limit = 100, options: LoadOptions = null) {

View File

@@ -214,7 +214,7 @@ 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 rows = await this.db().selectAll(`SELECT distinct item_id FROM revisions WHERE item_type = ? AND item_id IN (${this.escapeIdsForSql(itemIds)})`, [itemType]);
return rows.map((r: RevisionEntity) => r.item_id);
}

View File

@@ -39,7 +39,7 @@ export default class Tag extends BaseItem {
return Note.previews(
null,
{ ...options, conditions: [`id IN ('${noteIds.join('\',\'')}')`] },
{ ...options, conditions: [`id IN (${this.escapeIdsForSql(noteIds)})`] },
);
}
@@ -153,7 +153,7 @@ 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('\',\'')}')`);
return this.modelSelectAll(`SELECT ${options.fields ? this.db().escapeFields(options.fields) : '*'} FROM tags WHERE id IN (${this.escapeIdsForSql(tagIds)})`);
}
public static async commonTagsByNoteIds(noteIds: string[]) {
@@ -168,7 +168,7 @@ export default class Tag extends BaseItem {
break;
}
}
return this.modelSelectAll(`SELECT * FROM tags WHERE id IN ('${commonTagIds.join('\',\'')}')`);
return this.modelSelectAll(`SELECT * FROM tags WHERE id IN (${this.escapeIdsForSql(commonTagIds)})`);
}
public static async loadByTitle(title: string): Promise<TagEntity> {

View File

@@ -54,7 +54,7 @@ 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 notes = await Note.modelSelectAll(`SELECT id, title, body, encryption_applied FROM notes WHERE id IN (${Note.escapeIdsForSql(noteIds)})`);
const noteById = (noteId: string) => {
for (let i = 0; i < notes.length; i++) {

View File

@@ -138,7 +138,9 @@ 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 notes = await Note.modelSelectAll(`SELECT * FROM notes WHERE is_conflict = 0 AND encryption_applied = 0 AND id IN (${
Note.escapeIdsForSql(noteIds)
})`);
for (let i = 0; i < changes.length; i++) {
const change = changes[i];

View File

@@ -147,7 +147,7 @@ export default class SearchEngine {
const notes = await Note.modelSelectAll(`
SELECT ${SearchEngine.relevantFields}
FROM notes
WHERE id IN ('${currentIds.join('\',\'')}') AND is_conflict = 0 AND encryption_applied = 0 AND deleted_time = 0`);
WHERE id IN (${BaseModel.escapeIdsForSql(currentIds)}) AND is_conflict = 0 AND encryption_applied = 0 AND deleted_time = 0`);
const queries = [];
for (let i = 0; i < notes.length; i++) {
@@ -230,7 +230,7 @@ export default class SearchEngine {
const noteIds = changes.map(a => a.item_id);
const notes = await Note.modelSelectAll(`
SELECT ${SearchEngine.relevantFields}
FROM notes WHERE id IN ('${noteIds.join('\',\'')}') AND is_conflict = 0 AND encryption_applied = 0 AND deleted_time = 0`,
FROM notes WHERE id IN (${Note.escapeIdsForSql(noteIds)}) AND is_conflict = 0 AND encryption_applied = 0 AND deleted_time = 0`,
);
for (let i = 0; i < changes.length; i++) {

View File

@@ -1,5 +1,5 @@
import SearchEngine from './SearchEngine';
import Note from '../../models/Note';
import Note, { PreviewsOptions } from '../../models/Note';
import Setting from '../../models/Setting';
export interface NotesForQueryOptions {
@@ -55,10 +55,10 @@ export default class SearchEngineUtils {
todoCompletedAutoAdded = true;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const previewOptions: any = { order: [],
const previewOptions: PreviewsOptions = { order: [],
fields: fields,
conditions: [`id IN ('${noteIds.join('\',\'')}')`], ...options };
conditions: [`id IN (${Note.escapeIdsForSql(noteIds)})`],
...options };
const notes = await Note.previews(null, previewOptions);