8000 1227: add assertion for basic where clause values (#5417) · knex/knex@e145322 · GitHub
[go: up one dir, main page]
Skip to content

Commit e145322

Browse files
littlemaneuverlittlemaneuver
authored
1227: add assertion for basic where clause values (#5417)
1 parent 962bb0a commit e145322

File tree

6 files changed

+158
-9
lines changed

6 files changed

+158
-9
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Master (Unreleased)
22

3+
### Bug fixes
4+
- Add assertion for basic where clause not to be object or array #1227
5+
36
# 2.3.0 - 31 August, 2022
47

58
### New features:

‎lib/dialects/mysql/query/mysql-querycompiler.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
// MySQL Query Compiler
22
// ------
3+
const assert = require('assert');
34
const identity = require('lodash/identity');
5+
const isPlainObject = require('lodash/isPlainObject');
6+
const isEmpty = require('lodash/isEmpty');
47
const QueryCompiler = require('../../../query/querycompiler');
58
const { wrapAsIdentifier } = require('../../../formatter/formatterUtils');
69
const {
710
columnize: columnize_,
811
wrap: wrap_,
912
} = require('../../../formatter/wrappingFormatter');
1013

14+
const isPlainObjectOrArray = (value) =>
15+
isPlainObject(value) || Array.isArray(value);
16+
1117
class QueryCompiler_MySQL extends QueryCompiler {
1218
constructor(client, builder, formatter) {
1319
super(client, builder, formatter);
@@ -131,7 +137,8 @@ class QueryCompiler_MySQL extends QueryCompiler {
131137
output(resp) {
132138
const out = resp.reduce(function (columns, val) {
133139
columns[val.COLUMN_NAME] = {
134-
defaultValue: val.COLUMN_DEFAULT === 'NULL' ? null : val.COLUMN_DEFAULT,
140+
defaultValue:
141+
val.COLUMN_DEFAULT === 'NULL' ? null : val.COLUMN_DEFAULT,
135142
type: val.DATA_TYPE,
136143
maxLength: val.CHARACTER_MAXIMUM_LENGTH,
137144
nullable: val.IS_NULLABLE === 'YES',
@@ -156,6 +163,25 @@ class QueryCompiler_MySQL extends QueryCompiler {
156163
return `limit ${limit}`;
157164
}
158165

166+
whereBasic(statement) {
167+
assert(
168+
!isPlainObjectOrArray(statement.value),
169+
'The values in where clause must not be object or array.'
170+
);
171+
172+
return super.whereBasic(statement);
173+
}
174+
175+
whereRaw(statement) {
176+
assert(
177+
isEmpty(statement.value.bindings) ||
178+
!Object.values(statement.value.bindings).some(isPlainObjectOrArray),
179+
'The values in where clause must not be object or array.'
180+
);
181+
182+
return super.whereRaw(statement);
183+
}
184+
159185
whereLike(statement) {
160186
return `${this._columnClause(statement)} ${this._not(
161187
statement,

‎lib/query/querybuilder.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,7 @@ class Builder extends EventEmitter {
527527
// Adds a raw `where` clause to the query.
528528
whereRaw(sql, bindings) {
529529
const raw = sql.isRawInstance ? sql : this.client.raw(sql, bindings);
530+
530531
this._statements.push({
531532
grouping: 'where',
532533
type: 'whereRaw',

‎test/db-less-test-suite.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ if (config.postgres) {
4646
require('./unit/dialects/postgres');
4747
F438 }
4848

49+
if (config.mysql) {
50+
require('./unit/dialects/mysql');
51+
}
52+
4953
describe('CLI tests', function () {
5054
this.timeout(process.env.KNEX_TEST_TIMEOUT || 5000);
5155
require('./cli/help.spec');

‎test/unit/dialects/mysql.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
'use strict';
2+
const expect = require('chai').expect;
3+
const knex = require('../../../knex');
4+
5+
describe('MySQL unit tests', () => {
6+
const qb = knex({
7+
client: 'mysql',
8+
connection: {
9+
port: 1433,
10+
host: '127.0.0.1',
11+
password: 'yourStrong(!)Password',
12+
user: 'sa',
13+
},
14+
});
15+
16+
it('should pass with plain values or with emtpy raw bindings', () => {
17+
expect(qb('users').select('*').where('id', '=', 1).toString()).to.equal(
18+
'select * from `users` where `id` = 1'
19+
);
20+
expect(qb('users').select('*').where({ id: 1 }).toString()).to.equal(
21+
'select * from `users` where `id` = 1'
22+
);
23+
expect(qb('users').select('*').whereRaw('?? = ?').toString()).to.equal(
24+
'select * from `users` where ?? = ?'
25+
);
26+
});
27+
28+
it('should fail if provided value is array in basic where query #1227', () => {
29+
try {
30+
qb('users').select('*').where('id', '=', [0]).toString();
31+
throw new Error('Should not reach this point');
32+
} catch (error) {
33+
expect(error.message).to.equal(
34+
'The values in where clause must not be object or array.'
35+
);
36+
}
37+
});
38+
39+
it('should fail if provided value is object in basic where query #1227', () => {
40+
try {
41+
qb('users').select('*').where('id', '=', { test: 'test ' }).toString();
42+
throw new Error('Should not reach this point');
43+
} catch (error) {
44+
expect(error.message).to.equal(
45+
'The values in where clause must not be object or array.'
46+
);
47+
}
48+
});
49+
50+
it('should fail if provided raw query with array value in bindings #1227', () => {
51+
try {
52+
qb('users')
53+
.select('*')
54+
.where(qb.raw('?? = ?', ['id', [0]]))
55+
.toString();
56+
throw new Error('Should not reach this point');
57+
} catch (error) {
58+
expect(error.message).to.equal(
59+
'The values in where clause must not be object or array.'
60+
);
61+
}
62+
});
63+
64+
it('should fail if provided raw query with object value in bindings #1227', () => {
65+
try {
66+
qb('users')
67+
.select('*')
68+
.where(qb.raw('?? = ?', ['id', { a: 1 }]))
69+
.toString();
70+
throw new Error('Should not reach this point');
71+
} catch (error) {
72+
expect(error.message).to.equal(
73+
'The values in where clause must not be object or array.'
74+
);
75+
}
76+
});
77+
78+
it('should fail if value in bindings is object in whereRaw #1227', () => {
79+
try {
80+
qb('users')
81+
.select('*')
82+
.whereRaw('?? = ?', ['id', { test: 'test' }])
83+
.toString();
84+
throw new Error('Should not reach this point');
85+
} catch (error) {
86+
expect(error.message).to.equal(
87+
'The values in where clause must not be object or array.'
88+
);
89+
}
90+
});
91+
});

‎test/unit/query/builder.js

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9304,18 +9304,27 @@ describe('QueryBuilder', () => {
93049304
.from('accounts')
93059305
.where({ Login: ['1', '2', '3', void 0] }),
93069306
undefinedColumns: ['Login'],
9307+
clientError: {
9308+
mysql: 'The values in where clause must not be object or array.',
9309+
},
93079310
},
93089311
{
93099312
builder: qb()
93109313
.from('accounts')
93119314
.where({ Login: { Test: '123', Value: void 0 } }),
93129315
undefinedColumns: ['Login'],
9316+
clientError: {
9317+
mysql: 'The values in where clause must not be object or array.',
9318+
},
93139319
},
93149320
{
93159321
builder: qb()
93169322
.from('accounts')
93179323
.where({ Login: ['1', ['2', [void 0]]] }),
93189324
undefinedColumns: ['Login'],
9325+
clientError: {
9326+
mysql: 'The values in where clause must not be object or array.',
9327+
},
93199328
},
93209329
{
93219330
builder: qb()
@@ -9329,10 +9338,6 @@ describe('QueryBuilder', () => {
93299338
try {
93309339
//Must be present, but makes no difference since it throws.
93319340
testsql(builder, {
9332-
mysql: {
9333-
sql: '',
9334-
bindings: [],
9335-
},
93369341
oracledb: {
93379342
sql: '',
93389343
bindings: [],
@@ -9350,10 +9355,7 @@ describe('QueryBuilder', () => {
93509355
bindings: [],
93519356
},
93529357
});
9353-
expect(true).to.equal(
9354-
false,
9355-
'Expected to throw error in compilation about undefined bindings.'
9356-
);
9358+
throw new Error('Should not reach this point');
93579359
} catch (error) {
93589360
expect(error.message).to.contain(
93599361
'Undefined binding(s) detected when compiling ' +
@@ -9362,6 +9364,28 @@ describe('QueryBuilder', () => {
93629364
); //This test is not for asserting correct queries
93639365
}
93649366
});
9367+
qbuilders.forEach(({ builder, undefinedColumns, clientError }) => {
9368+
try {
9369+
//Must be present, but makes no difference since it throws.
9370+
testsql(builder, {
9371+
mysql: {
9372+
sql: '',
9373+
bindings: [],
9374+
},
9375+
});
9376+
throw new Error('Should not reach this point');
9377+
} catch (error) {
9378+
if (clientError && clientError.mysql) {
9379+
expect(error.message).to.contain(clientError.mysql);
9380+
} else {
9381+
expect(error.message).to.contain(
9382+
'Undefined binding(s) detected when compiling ' +
9383+
builder._method.toUpperCase() +
9384+
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
9385+
); //This test is not for asserting correct queries
9386+
}
9387+
}
9388+
});
93659389
});
93669390

93679391
it('Any undefined binding in a RAW query should throw an error', () => {

0 commit comments

Comments
 (0)
0