From e145322da92749be7749f9ade5b5f5a66d6586a4 Mon Sep 17 00:00:00 2001 From: Anton Savchenko Date: Fri, 6 Jan 2023 00:27:49 +0200 Subject: [PATCH] 1227: add assertion for basic where clause values (#5417) --- CHANGELOG.md | 3 + .../mysql/query/mysql-querycompiler.js | 28 +++++- lib/query/querybuilder.js | 1 + test/db-less-test-suite.js | 4 + test/unit/dialects/mysql.js | 91 +++++++++++++++++++ test/unit/query/builder.js | 40 ++++++-- 6 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 test/unit/dialects/mysql.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 6335837221..f9a59b373a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Master (Unreleased) +### Bug fixes +- Add assertion for basic where clause not to be object or array #1227 + # 2.3.0 - 31 August, 2022 ### New features: diff --git a/lib/dialects/mysql/query/mysql-querycompiler.js b/lib/dialects/mysql/query/mysql-querycompiler.js index a235c64f04..1c1a1c0981 100644 --- a/lib/dialects/mysql/query/mysql-querycompiler.js +++ b/lib/dialects/mysql/query/mysql-querycompiler.js @@ -1,6 +1,9 @@ // MySQL Query Compiler // ------ +const assert = require('assert'); const identity = require('lodash/identity'); +const isPlainObject = require('lodash/isPlainObject'); +const isEmpty = require('lodash/isEmpty'); const QueryCompiler = require('../../../query/querycompiler'); const { wrapAsIdentifier } = require('../../../formatter/formatterUtils'); const { @@ -8,6 +11,9 @@ const { wrap: wrap_, } = require('../../../formatter/wrappingFormatter'); +const isPlainObjectOrArray = (value) => + isPlainObject(value) || Array.isArray(value); + class QueryCompiler_MySQL extends QueryCompiler { constructor(client, builder, formatter) { super(client, builder, formatter); @@ -131,7 +137,8 @@ class QueryCompiler_MySQL extends QueryCompiler { output(resp) { const out = resp.reduce(function (columns, val) { columns[val.COLUMN_NAME] = { - defaultValue: val.COLUMN_DEFAULT === 'NULL' ? null : val.COLUMN_DEFAULT, + defaultValue: + val.COLUMN_DEFAULT === 'NULL' ? null : val.COLUMN_DEFAULT, type: val.DATA_TYPE, maxLength: val.CHARACTER_MAXIMUM_LENGTH, nullable: val.IS_NULLABLE === 'YES', @@ -156,6 +163,25 @@ class QueryCompiler_MySQL extends QueryCompiler { return `limit ${limit}`; } + whereBasic(statement) { + assert( + !isPlainObjectOrArray(statement.value), + 'The values in where clause must not be object or array.' + ); + + return super.whereBasic(statement); + } + + whereRaw(statement) { + assert( + isEmpty(statement.value.bindings) || + !Object.values(statement.value.bindings).some(isPlainObjectOrArray), + 'The values in where clause must not be object or array.' + ); + + return super.whereRaw(statement); + } + whereLike(statement) { return `${this._columnClause(statement)} ${this._not( statement, diff --git a/lib/query/querybuilder.js b/lib/query/querybuilder.js index 947d6c408e..096e35ee9e 100644 --- a/lib/query/querybuilder.js +++ b/lib/query/querybuilder.js @@ -527,6 +527,7 @@ class Builder extends EventEmitter { // Adds a raw `where` clause to the query. whereRaw(sql, bindings) { const raw = sql.isRawInstance ? sql : this.client.raw(sql, bindings); + this._statements.push({ grouping: 'where', type: 'whereRaw', diff --git a/test/db-less-test-suite.js b/test/db-less-test-suite.js index 5a5b400363..8cac595595 100644 --- a/test/db-less-test-suite.js +++ b/test/db-less-test-suite.js @@ -46,6 +46,10 @@ if (config.postgres) { require('./unit/dialects/postgres'); } +if (config.mysql) { + require('./unit/dialects/mysql'); +} + describe('CLI tests', function () { this.timeout(process.env.KNEX_TEST_TIMEOUT || 5000); require('./cli/help.spec'); diff --git a/test/unit/dialects/mysql.js b/test/unit/dialects/mysql.js new file mode 100644 index 0000000000..f8afe833d7 --- /dev/null +++ b/test/unit/dialects/mysql.js @@ -0,0 +1,91 @@ +'use strict'; +const expect = require('chai').expect; +const knex = require('../../../knex'); + +describe('MySQL unit tests', () => { + const qb = knex({ + client: 'mysql', + connection: { + port: 1433, + host: '127.0.0.1', + password: 'yourStrong(!)Password', + user: 'sa', + }, + }); + + it('should pass with plain values or with emtpy raw bindings', () => { + expect(qb('users').select('*').where('id', '=', 1).toString()).to.equal( + 'select * from `users` where `id` = 1' + ); + expect(qb('users').select('*').where({ id: 1 }).toString()).to.equal( + 'select * from `users` where `id` = 1' + ); + expect(qb('users').select('*').whereRaw('?? = ?').toString()).to.equal( + 'select * from `users` where ?? = ?' + ); + }); + + it('should fail if provided value is array in basic where query #1227', () => { + try { + qb('users').select('*').where('id', '=', [0]).toString(); + throw new Error('Should not reach this point'); + } catch (error) { + expect(error.message).to.equal( + 'The values in where clause must not be object or array.' + ); + } + }); + + it('should fail if provided value is object in basic where query #1227', () => { + try { + qb('users').select('*').where('id', '=', { test: 'test ' }).toString(); + throw new Error('Should not reach this point'); + } catch (error) { + expect(error.message).to.equal( + 'The values in where clause must not be object or array.' + ); + } + }); + + it('should fail if provided raw query with array value in bindings #1227', () => { + try { + qb('users') + .select('*') + .where(qb.raw('?? = ?', ['id', [0]])) + .toString(); + throw new Error('Should not reach this point'); + } catch (error) { + expect(error.message).to.equal( + 'The values in where clause must not be object or array.' + ); + } + }); + + it('should fail if provided raw query with object value in bindings #1227', () => { + try { + qb('users') + .select('*') + .where(qb.raw('?? = ?', ['id', { a: 1 }])) + .toString(); + throw new Error('Should not reach this point'); + } catch (error) { + expect(error.message).to.equal( + 'The values in where clause must not be object or array.' + ); + } + }); + + it('should fail if value in bindings is object in whereRaw #1227', () => { + try { + qb('users') + .select('*') + .whereRaw('?? = ?', ['id', { test: 'test' }]) + .toString(); + throw new Error('Should not reach this point'); + } catch (error) { + expect(error.message).to.equal( + 'The values in where clause must not be object or array.' + ); + } + }); +}); diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js index db02e2a95b..712c00ae1f 100644 --- a/test/unit/query/builder.js +++ b/test/unit/query/builder.js @@ -9304,18 +9304,27 @@ describe('QueryBuilder', () => { .from('accounts') .where({ Login: ['1', '2', '3', void 0] }), undefinedColumns: ['Login'], + clientError: { + mysql: 'The values in where clause must not be object or array.', + }, }, { builder: qb() .from('accounts') .where({ Login: { Test: '123', Value: void 0 } }), undefinedColumns: ['Login'], + clientError: { + mysql: 'The values in where clause must not be object or array.', + }, }, { builder: qb() .from('accounts') .where({ Login: ['1', ['2', [void 0]]] }), undefinedColumns: ['Login'], + clientError: { + mysql: 'The values in where clause must not be object or array.', + }, }, { builder: qb() @@ -9329,10 +9338,6 @@ describe('QueryBuilder', () => { try { //Must be present, but makes no difference since it throws. testsql(builder, { - mysql: { - sql: '', - bindings: [], - }, oracledb: { sql: '', bindings: [], @@ -9350,10 +9355,7 @@ describe('QueryBuilder', () => { bindings: [], }, }); - expect(true).to.equal( - false, - 'Expected to throw error in compilation about undefined bindings.' - ); + throw new Error('Should not reach this point'); } catch (error) { expect(error.message).to.contain( 'Undefined binding(s) detected when compiling ' + @@ -9362,6 +9364,28 @@ describe('QueryBuilder', () => { ); //This test is not for asserting correct queries } }); + qbuilders.forEach(({ builder, undefinedColumns, clientError }) => { + try { + //Must be present, but makes no difference since it throws. + testsql(builder, { + mysql: { + sql: '', + bindings: [], + }, + }); + throw new Error('Should not reach this point'); + } catch (error) { + if (clientError && clientError.mysql) { + expect(error.message).to.contain(clientError.mysql); + } else { + expect(error.message).to.contain( + 'Undefined binding(s) detected when compiling ' + + builder._method.toUpperCase() + + `. Undefined column(s): [${undefinedColumns.join(', ')}] query:` + ); //This test is not for asserting correct queries + } + } + }); }); it('Any undefined binding in a RAW query should throw an error', () => {