diff --git a/lib/group/store/mysql.js b/lib/group/store/mysql.js index cb9422c..a8b99fd 100644 --- a/lib/group/store/mysql.js +++ b/lib/group/store/mysql.js @@ -83,7 +83,8 @@ class Group extends GroupBase { [args.id], ) const gids = [args.id, ...subgroupRows.map((r) => r.nt_subgroup_id)] - where.push(`g.nt_group_id IN (${gids.join(',')})`) + where.push(`g.nt_group_id IN (${gids.map(() => '?').join(',')})`) + params.push(...gids) } else { where.push('g.nt_group_id = ?') params.push(args.id) @@ -159,16 +160,20 @@ class Group extends GroupBase { if (Object.keys(args).length === 0) return true const r = await Mysql.execute( - ...Mysql.update(`nt_group`, `nt_group_id=${id}`, mapToDbColumn(args, groupDbMap)), + ...Mysql.update(`nt_group`, { nt_group_id: id }, mapToDbColumn(args, groupDbMap)), ) return r.changedRows === 1 } async delete(args) { const r = await Mysql.execute( - ...Mysql.update(`nt_group`, `nt_group_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_group`, + { nt_group_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 } diff --git a/lib/mysql.js b/lib/mysql.js index fd8cad2..8daa8c8 100644 --- a/lib/mysql.js +++ b/lib/mysql.js @@ -77,9 +77,12 @@ class Mysql { return shapeRows(query, rows) } + // Column names can come from payload keys, so they are quoted as + // identifiers; values are always bound. insert(table, params = {}) { + const columns = Object.keys(params).map(escapeId) return [ - `INSERT INTO ${table} (${Object.keys(params).join(',')}) VALUES(${Object.keys(params).map(() => '?')})`, + `INSERT INTO ${table} (${columns.join(',')}) VALUES(${columns.map(() => '?')})`, Object.values(params), ] } @@ -88,8 +91,15 @@ class Mysql { return this.whereConditions(query, params) } + // where is an object of column values, or SQL whose placeholders the + // caller binds after the SET values update(table, where, params = {}) { - return [`UPDATE ${table} SET ${Object.keys(params).join('=?,')}=? WHERE ${where}`, Object.values(params)] + const set = Object.keys(params).map((col) => `${escapeId(col)}=?`) + const [conditions, whereValues] = typeof where === 'string' ? [where, []] : equalities(where) + return [ + `UPDATE ${table} SET ${set.join(',')} WHERE ${conditions}`, + [...Object.values(params), ...whereValues], + ] } delete(table, params) { @@ -97,23 +107,10 @@ class Mysql { } whereConditions(query, params) { - let newQuery = query - let paramsArray = [] - - if (Array.isArray(params)) { - paramsArray = [...params] - } else if (typeof params === 'object' && !Array.isArray(params)) { - // Object to WHERE conditions - let first = true - for (const p in params) { - if (first) newQuery += ' WHERE' - if (!first) newQuery += ' AND' - newQuery += ` ${p}=?` - paramsArray.push(params[p]) - first = false - } - } - return [newQuery, paramsArray] + if (Array.isArray(params)) return [query, [...params]] + if (typeof params !== 'object' || Object.keys(params).length === 0) return [query, []] + const [conditions, values] = equalities(params) + return [`${query} WHERE ${conditions}`, values] } async disconnect(dbh) { @@ -133,4 +130,11 @@ class Mysql { } } +const { escapeId } = mysql + +function equalities(params) { + const columns = Object.keys(params).map((col) => `${escapeId(col)}=?`) + return [columns.join(' AND '), Object.values(params)] +} + export default new Mysql() diff --git a/lib/mysql.test.js b/lib/mysql.test.js index 95e1047..245e27b 100644 --- a/lib/mysql.test.js +++ b/lib/mysql.test.js @@ -25,7 +25,7 @@ describe('mysql', () => { Mysql.select(`SELECT * FROM nt_user`, { last_name: 'Test', }), - [`SELECT * FROM nt_user WHERE last_name=?`, ['Test']], + ['SELECT * FROM nt_user WHERE `last_name`=?', ['Test']], ) }) @@ -34,36 +34,68 @@ describe('mysql', () => { first_name: 'uNite', last_name: 'Test', }) - assert.deepEqual(r, [`INSERT INTO nt_user (first_name,last_name) VALUES(?,?)`, ['uNite', 'Test']]) + assert.deepEqual(r, ['INSERT INTO nt_user (`first_name`,`last_name`) VALUES(?,?)', ['uNite', 'Test']]) }) describe('update', () => { it('formats with one value', () => { - const r = Mysql.update(`nt_user`, `nt_user_id=4096`, { - first_name: 'uNite', - }) - assert.deepEqual(r, [`UPDATE nt_user SET first_name=? WHERE nt_user_id=4096`, ['uNite']]) + const r = Mysql.update( + `nt_user`, + { nt_user_id: 4096 }, + { + first_name: 'uNite', + }, + ) + assert.deepEqual(r, ['UPDATE nt_user SET `first_name`=? WHERE `nt_user_id`=?', ['uNite', 4096]]) }) it('formats with two values', () => { - const r = Mysql.update(`nt_user`, `nt_user_id=4096`, { - last_name: 'Teste', - is_admin: 1, - }) - assert.deepEqual(r, [`UPDATE nt_user SET last_name=?,is_admin=? WHERE nt_user_id=4096`, ['Teste', 1]]) + const r = Mysql.update( + `nt_user`, + { nt_user_id: 4096 }, + { + last_name: 'Teste', + is_admin: 1, + }, + ) + assert.deepEqual(r, [ + 'UPDATE nt_user SET `last_name`=?,`is_admin`=? WHERE `nt_user_id`=?', + ['Teste', 1, 4096], + ]) }) it('formats with three values', () => { - const r = Mysql.update(`nt_user`, `nt_user_id=4096`, { - first_name: 'Unit', - last_name: 'Test', - is_admin: 0, - }) + const r = Mysql.update( + `nt_user`, + { nt_user_id: 4096 }, + { + first_name: 'Unit', + last_name: 'Test', + is_admin: 0, + }, + ) assert.deepEqual(r, [ - `UPDATE nt_user SET first_name=?,last_name=?,is_admin=? WHERE nt_user_id=4096`, - ['Unit', 'Test', 0], + 'UPDATE nt_user SET `first_name`=?,`last_name`=?,`is_admin`=? WHERE `nt_user_id`=?', + ['Unit', 'Test', 0, 4096], ]) }) + + it('binds the SET values before a where clause the caller wrote', () => { + const r = Mysql.update(`nt_user`, 'nt_user_id=? AND deleted=?', { first_name: 'Unit' }) + assert.deepEqual(r, ['UPDATE nt_user SET `first_name`=? WHERE nt_user_id=? AND deleted=?', ['Unit']]) + }) + + it('quotes a column name that tries to be SQL', () => { + const [sql] = Mysql.update(`nt_user`, { nt_user_id: 4096 }, { 'is_admin=1,first_name': 'x' }) + assert.equal(sql, 'UPDATE nt_user SET `is_admin=1,first_name`=? WHERE `nt_user_id`=?') + }) + }) + + describe('insert', () => { + it('quotes a column name that tries to be SQL', () => { + const [sql] = Mysql.insert(`nt_user`, { 'first_name) VALUES (1); --': 'x' }) + assert.equal(sql, 'INSERT INTO nt_user (`first_name) VALUES (1); --`) VALUES(?)') + }) }) describe('delete', () => { @@ -73,7 +105,7 @@ describe('mysql', () => { it('with params', () => { assert.deepEqual(Mysql.delete(`nt_user`, { last_name: 'Test' }), [ - `DELETE FROM nt_user WHERE last_name=?`, + 'DELETE FROM nt_user WHERE `last_name`=?', ['Test'], ]) }) diff --git a/lib/nameserver/store/mysql.js b/lib/nameserver/store/mysql.js index 1246c97..3c04278 100644 --- a/lib/nameserver/store/mysql.js +++ b/lib/nameserver/store/mysql.js @@ -77,16 +77,20 @@ class Nameserver extends NameserverBase { delete args.id // Mysql.debug(1) const r = await Mysql.execute( - ...Mysql.update(`nt_nameserver`, `nt_nameserver_id=${id}`, mapToDbColumn(objectToDb(args), nsDbMap)), + ...Mysql.update(`nt_nameserver`, { nt_nameserver_id: id }, mapToDbColumn(objectToDb(args), nsDbMap)), ) return r.changedRows === 1 } async delete(args) { const r = await Mysql.execute( - ...Mysql.update(`nt_nameserver`, `nt_nameserver_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_nameserver`, + { nt_nameserver_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 } diff --git a/lib/permission/store/mysql.js b/lib/permission/store/mysql.js index a0281f2..e426cb0 100644 --- a/lib/permission/store/mysql.js +++ b/lib/permission/store/mysql.js @@ -121,7 +121,7 @@ class PermissionRepoMySQL extends PermissionBase { delete row.id if (Object.keys(row).length === 0) return false const r = await Mysql.execute( - ...Mysql.update(`nt_perm`, `nt_perm_id=${id}`, mapToDbColumn(row, permDbMap)), + ...Mysql.update(`nt_perm`, { nt_perm_id: id }, mapToDbColumn(row, permDbMap)), ) return r.changedRows === 1 } @@ -129,9 +129,13 @@ class PermissionRepoMySQL extends PermissionBase { async delete(args) { if (!args.id) return false const r = await Mysql.execute( - ...Mysql.update(`nt_perm`, `nt_perm_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_perm`, + { nt_perm_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 } diff --git a/lib/session/store/mysql.js b/lib/session/store/mysql.js index bc45390..94a0ea5 100644 --- a/lib/session/store/mysql.js +++ b/lib/session/store/mysql.js @@ -63,7 +63,7 @@ class SessionRepoMySQL { const id = args.id delete args.id const r = await Mysql.execute( - ...Mysql.update(`nt_user_session`, `nt_user_session_id=${id}`, mapToDbColumn(args, sessionDbMap)), + ...Mysql.update(`nt_user_session`, { nt_user_session_id: id }, mapToDbColumn(args, sessionDbMap)), ) return r.changedRows === 1 } diff --git a/lib/user/store/mysql.js b/lib/user/store/mysql.js index 2a0d56f..7e53c08 100644 --- a/lib/user/store/mysql.js +++ b/lib/user/store/mysql.js @@ -50,7 +50,7 @@ class UserRepoMySQL extends UserBase { if (needsUpgrade) { try { const creds = await Credentials.forStorage(authTry.password) - await Mysql.execute(...Mysql.update('nt_user', `nt_user_id=${u.id}`, creds)) + await Mysql.execute(...Mysql.update('nt_user', { nt_user_id: u.id }, creds)) } catch (err) { console.warn(`password hash upgrade failed for user ${authTry.username}`, err) } @@ -130,7 +130,8 @@ class UserRepoMySQL extends UserBase { [args.gid], ) const gids = [args.gid, ...subgroupRows.map((r) => r.nt_subgroup_id)] - where.push(`nt_group_id IN (${gids.join(',')})`) + where.push(`nt_group_id IN (${gids.map(() => '?').join(',')})`) + params.push(...gids) } else { where.push('nt_group_id = ?') params.push(args.gid) @@ -238,16 +239,20 @@ class UserRepoMySQL extends UserBase { if (Object.keys(args).length === 0) return true const r = await Mysql.execute( - ...Mysql.update(`nt_user`, `nt_user_id=${id}`, mapToDbColumn(args, userDbMap)), + ...Mysql.update(`nt_user`, { nt_user_id: id }, mapToDbColumn(args, userDbMap)), ) return r.changedRows === 1 } async delete(args) { const r = await Mysql.execute( - ...Mysql.update(`nt_user`, `nt_user_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_user`, + { nt_user_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 } diff --git a/lib/zone/store/mysql.js b/lib/zone/store/mysql.js index 39761aa..3bd4dfa 100644 --- a/lib/zone/store/mysql.js +++ b/lib/zone/store/mysql.js @@ -255,7 +255,7 @@ class ZoneRepoMySQL extends ZoneBase { if (!args.id) return false const { id, nameservers, ...changes } = args const update = Object.keys(changes).length - ? Mysql.update(`nt_zone`, `nt_zone_id=${id}`, mapToDbColumn(changes, zoneDbMap)) + ? Mysql.update(`nt_zone`, { nt_zone_id: id }, mapToDbColumn(changes, zoneDbMap)) : undefined const deleted = changes.deleted === undefined ? undefined : isTrue(changes.deleted) @@ -298,9 +298,13 @@ class ZoneRepoMySQL extends ZoneBase { async delete(args) { if (args.deleted === false || args.deleted === 0) return this.put({ id: args.id, deleted: false }) const r = await Mysql.execute( - ...Mysql.update(`nt_zone`, `nt_zone_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_zone`, + { nt_zone_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 } diff --git a/lib/zone_record/store/mysql.js b/lib/zone_record/store/mysql.js index 86027d4..16f25e1 100644 --- a/lib/zone_record/store/mysql.js +++ b/lib/zone_record/store/mysql.js @@ -203,9 +203,13 @@ class ZoneRecordMySQL extends ZoneRecordBase { async delete(args) { const r = await Mysql.execute( - ...Mysql.update(`nt_zone_record`, `nt_zone_record_id=${args.id}`, { - deleted: args.deleted ?? 1, - }), + ...Mysql.update( + `nt_zone_record`, + { nt_zone_record_id: args.id }, + { + deleted: args.deleted ?? 1, + }, + ), ) return r.changedRows === 1 }