Added validation and sanitisation for sql queries passed into runQuery

lib/DatabaseConnectionPool.js

@@ -24,6 +24,7 @@ class DatabaseConnectionPool {
 
 	/**
 	 * Create a database connection pool
+	 *
 	 * @param {object} [dbOptions] Optional database params to connect with
 	 */
 	constructor(dbOptions) {
@@ -40,26 +41,53 @@ class DatabaseConnectionPool {
 	}
 
 	/**
-	 * Run a query against the database connection
+	 * Sanitise and validate an sql query
+	 *
 	 * @param {string} sql The query to be executed
 	 * @param {(Array<string|number>)} Values to replace prepared statement
-	 * @return {(Array<object>|object)} Data returned from the database
+	 *
+	 * @return {string} Sanitised and validated sql query
 	 */
-	async runQuery(sql, params) {
+	static validateQuery(sql, params) {
 		sql = sql.trim();
 
 		if (sql.slice(-1) !== ';')
-			throw new Error('Invalid query, needs trailing ;');
+			throw new Error('Query needs trailing ;');
 
-		// Execute as non-prepared if no params are supplied
+		const expectedParams = sql.split('?').length - 1;
-		if (typeof params === 'undefined') {
+		const prepared = expectedParams > 0;
-			const [ data ] =
+
-				await this.#connectionPool.execute(sql);
+		if (!prepared && typeof params !== 'undefined') {
-			return data;
+			throw new Error('Can not pass in parameters ' +
+				'for a non-prepared statement');
+		} else if (prepared && params.length !== expectedParams) {
+			throw new Error('Number of params does not equal ' +
+				'expected number');
+		}
+
+		return sql;
+	}
+
+	/**
+	 * Run a query against the database connection
+	 *
+	 * @param {string} sql The query to be executed
+	 * @param {(Array<string|number>)} Values to replace prepared statement
+	 *
+	 * @return {(Array<object>|object)} Data returned from the database
+	 */
+	async runQuery(sql, params) {
+		sql = this.validateQuery(sql, params);
+		const prepared = sql.includes('?');
+
+		let data;
+		if (!prepared) {
+			[ data ] = await this.#connectionPool.execute(sql);
+		} else {
+			[ data ] =
+				await this.#connectionPool.execute(sql, params);
 		}
 
-		const [ data ] =
-			await this.#connectionPool.execute(sql, params);
 		return data;
 	}
 

lib/DatabaseConnectionPool.test.js

@@ -47,11 +47,46 @@ describe('DatabaseConnectionPool', () => {
 	test('Query with whitespace after ; should not fail', ()=> {
 		const dbp = new DatabaseConnectionPool();
 
-		const sql = `SELECT * FROM class;`;
+		const sql = `SELECT * FROM class;		`;
-		dbp.runQuery(sql);
 
-		expect(dbp.runQuery.mock.results[0].value).toEqual({
+		expect(() => dbp.runQuery(sql)).not.toThrow();
-			sql: sql
+	});
-		});
+
+	test('Prepared query should fail if no params are given', () => {
+		const dbp = new DatabaseConnectionPool();
+
+		const sql = `SELECT * FROM class where name = ?;`;
+
+		expect(() => dbp.runQuery(sql)).toThrow();
+	});
+
+	test('Prepared query should fail if too many params given', () => {
+		const dbp = new DatabaseConnectionPool();
+
+		const sql = `SELECT * FROM class where name = ?;`;
+		const params = [ 'bob', 'jim' ];
+
+		expect(() => dbp.runQuery(sql, params)).toThrow();
+	});
+
+	test('Prepared query should fail if too few params given', () => {
+		const dbp = new DatabaseConnectionPool();
+
+		const sql = `SELECT * FROM class where name = ?
+			and classId = ? and subjectId = ?;`;
+
+		const params = [ 'bob' ];
+
+		expect(() => dbp.runQuery(sql, params)).toThrow();
+	});
+
+	test('Non-prepared query should fail if params given', () => {
+		const dbp = new DatabaseConnectionPool();
+
+		const sql = `SELECT * FROM class;`;
+
+		const params = [ 'bob', 'jim' ];
+
+		expect(() => dbp.runQuery(sql, params)).toThrow();
 	});
 });

lib/__mocks__/DatabaseConnectionPool.js

@@ -2,23 +2,51 @@
 
 const DatabaseConnectionPool = require('./DatabaseConnectionPool');
 
-const mockRunQuery = jest.fn((sql, params) => {
+/**
+ * Sanitise and validate an sql query
+ *
+ * @param {string} sql The query to be executed
+ * @param {(Array<string|number>)} Values to replace prepared statement
+ *
+ * @return {string} Sanitised and validated sql query
+ */
+function validateQuery(sql, params) {
 	sql = sql.trim();
 
 	if (sql.slice(-1) !== ';')
-		throw new Error('Invalid query, needs trailing ;');
+		throw new Error('Query needs trailing ;');
 
-	// Execute as non-prepared if no params are supplied
+	const expectedParams = sql.split('?').length - 1;
-	if (typeof params === 'undefined') {
+	const prepared = expectedParams > 0;
-		return {
+
+	if (!prepared && typeof params !== 'undefined') {
+		throw new Error('Can not pass in parameters ' +
+			'for a non-prepared statement');
+	} else if (prepared && params.length !== expectedParams) {
+		throw new Error('Number of params does not equal ' +
+			'expected number');
+	}
+
+	return sql;
+}
+
+const mockRunQuery = jest.fn((sql, params) => {
+	sql = validateQuery(sql, params);
+	const prepared = sql.includes('?');
+
+	let data;
+	if (!prepared) {
+		data = {
 			sql: sql
 		};
+	} else {
+		data = {
+			sql: sql,
+			params: params
+		};
 	}
 
-	return {
+	return data;
-		sql: sql,
-		params: params
-	};
 });
 
 const mockClose = jest.fn();