1227: add assertion for basic where clause values (#5417)

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:

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

@@ -1,13 +1,19 @@
 // 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 {
   columnize: columnize_,
   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,

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',

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');

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.'
+      );
+    }
+  });
+});

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', () => {