Skip to content

mysql: quote column names and bind every WHERE value #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
msimerson merged 1 commit into from
Oct 16, 2026
Merged

mysql: quote column names and bind every WHERE value #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/group/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
42 changes: 23 additions & 19 deletions lib/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]
}
Expand All @@ -88,32 +91,26 @@ 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) {
return this.whereConditions(`DELETE FROM ${table}`, params)
}

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) {
Expand All @@ -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()
70 changes: 51 additions & 19 deletions lib/mysql.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']],
)
})

Expand All @@ -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', () => {
Expand All @@ -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'],
])
})
Expand Down
12 changes: 8 additions & 4 deletions lib/nameserver/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 8 additions & 4 deletions lib/permission/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,21 @@ 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
}

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
}
Expand Down
2 changes: 1 addition & 1 deletion lib/session/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
17 changes: 11 additions & 6 deletions lib/user/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 8 additions & 4 deletions lib/zone/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
10 changes: 7 additions & 3 deletions lib/zone_record/store/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading