self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-24 08:12:24 +02:00
Code Issues Releases Activity

Server: Prevent crash when returning too many rows using SQLite

Browse Source
This commit is contained in:
Laurent Cozic 2021-08-18 12:47:05 +01:00
parent 9b27b3b1fb
commit 1efe3d3c6a
3 changed files with 31 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
packages/server/src/db.ts
View File

@ -16,6 +16,11 @@ require('pg').types.setTypeParser(20, function(val: any) {
const logger = Logger.create('db'); const logger = Logger.create('db');
// To prevent error "SQLITE_ERROR: too many SQL variables", SQL statements with
// "IN" clauses shouldn't contain more than the number of variables below.s
// https://www.sqlite.org/limits.html#max_variable_number
export const SqliteMaxVariableNum = 999;
const migrationDir = `${__dirname}/migrations`; const migrationDir = `${__dirname}/migrations`;
export const sqliteDefaultDir = pathUtils.dirname(__dirname); export const sqliteDefaultDir = pathUtils.dirname(__dirname);

27
packages/server/src/models/ChangeModel.test.ts
View File

@ -1,11 +1,11 @@
import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, expectThrow, createFolder, createItemTree3 } from '../utils/testing/testUtils'; import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, expectThrow, createFolder, createItemTree3, expectNotThrow } from '../utils/testing/testUtils';
import { ChangeType, Item, Uuid } from '../db'; import { ChangeType, Item, SqliteMaxVariableNum, Uuid } from '../db';
import { msleep } from '../utils/time'; import { msleep } from '../utils/time';
import { ChangePagination } from './ChangeModel'; import { ChangePagination } from './ChangeModel';
async function makeTestItem(userId: Uuid, num: number): Promise<Item> { async function makeTestItem(userId: Uuid, num: number): Promise<Item> {
return models().item().saveForUser(userId, { return models().item().saveForUser(userId, {
name: `0000000000000000000000000000000${num}.md`, name: `${num.toString().padStart(32, '0')}.md`,
}); });
} }
@ -164,4 +164,25 @@ describe('ChangeModel', function() {
expect(allFromIds3.has_more).toBe(false); expect(allFromIds3.has_more).toBe(false);
}); });
test('should not fail when retrieving many changes', async function() {
// Create many changes and verify that, by default, the SQL query that
// returns change doesn't fail. Before the max number of items was set
// to 1000 and it would fail with "SQLITE_ERROR: too many SQL variables"
// with SQLite. So now it's set to 999.
const { user } = await createUserAndSession(1, true);
for (let i = 0; i < 1010; i++) {
await makeTestItem(user.id, i);
}
let changeCount = 0;
await expectNotThrow(async () => {
const changes = await models().change().allFromId('');
changeCount = changes.items.length;
});
expect(changeCount).toBe(SqliteMaxVariableNum);
});
}); });

4
packages/server/src/models/ChangeModel.ts
View File

@ -1,5 +1,5 @@
import { Knex } from 'knex'; import { Knex } from 'knex';
import { Change, ChangeType, Item, Uuid } from '../db'; import { Change, ChangeType, Item, SqliteMaxVariableNum, Uuid } from '../db';
import { md5 } from '../utils/crypto'; import { md5 } from '../utils/crypto';
import { ErrorResyncRequired } from '../utils/errors'; import { ErrorResyncRequired } from '../utils/errors';
import BaseModel, { SaveOptions } from './BaseModel'; import BaseModel, { SaveOptions } from './BaseModel';
@ -59,7 +59,7 @@ export default class ChangeModel extends BaseModel<Change> {
return `${this.baseUrl}/changes`; return `${this.baseUrl}/changes`;
} }
public async allFromId(id: string, limit: number = 1000): Promise<PaginatedChanges> { public async allFromId(id: string, limit: number = SqliteMaxVariableNum): Promise<PaginatedChanges> {
const startChange: Change = id ? await this.load(id) : null; const startChange: Change = id ? await this.load(id) : null;
const query = this.db(this.tableName).select(...this.defaultFields); const query = this.db(this.tableName).select(...this.defaultFields);
if (startChange) void query.where('counter', '>', startChange.counter); if (startChange) void query.where('counter', '>', startChange.counter);