Skip to content
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.

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

Updating from knex 2.3.0 to knex 2.4.2 breaks working code #5580

Open
perry-autofi opened this issue May 18, 2023 · 6 comments
Open

Updating from knex 2.3.0 to knex 2.4.2 breaks working code #5580

perry-autofi opened this issue May 18, 2023 · 6 comments

Comments

@perry-autofi
Copy link

Environment

Knex version: 2.4.2
Database + version: MySQL v8
OS: Mac OS Ventura 13.2.1

@smorey2

Bug

  1. Explain what kind of behaviour you are getting and how you think it should do
    The following code which is working with knex v2.3.0 now throws the error below after updating only the knex module to 2.4.2 in my project.
    [this.#productGroup] = await db
            .select('*')
            .from('ProductGroup')
            .where({ dealerId: this.#dealerId, groupId: this.#groupId });

Upgrading a minor version or patch should not break working code.

  1. Error message
    Message:
      The values in where clause must not be object or array.

      42 | 		if (!this.#productGroup) {
      43 | 			// TODO - Merge into a single join query
    > 44 | 			[this.#productGroup] = await db
         | 			                       ^
      45 | 				.select('*')
      46 | 				.from('ProductGroup')
      47 | 				.where({ dealerId: this.#dealerId, groupId: this.#groupId });

      at QueryCompiler_MySQL.whereBasic (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/dialects/mysql/query/mysql-querycompiler.js:167:5)
      at QueryCompiler_MySQL.where (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/query/querycompiler.js:573:34)
      at ../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/query/querycompiler.js:133:40
          at Array.forEach (<anonymous>)
      at QueryCompiler_MySQL.select (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/query/querycompiler.js:132:16)
      at QueryCompiler_MySQL.toSQL (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/query/querycompiler.js:73:29)
      at Builder.toSQL (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/query/querybuilder.js:83:44)
      at ensureConnectionCallback (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/execution/internal/ensure-connection-callback.js:4:30)
      at Runner.ensureConnection (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/execution/runner.js:300:20)
      at Runner.run (../../node_modules/.pnpm/knex@2.4.2_mysql2@2.3.3/node_modules/knex/lib/execution/runner.js:30:19)
      at ProductGroup.exists (src/lib/dao/ProductGroup.js:44:27)
      at ProductGroup.create (src/lib/dao/ProductGroup.js:100:7)
      at Dealer.createGroups (src/lib/dao/Dealer.js:126:4)
      at Object.<anonymous> (test/lib/dao/Dealer.test.js:323:21)
  1. Reduced test code, for example in https://npm.runkit.com/knex or if it needs real
    database connection to MySQL or PostgreSQL, then single file example which initializes
    needed data and demonstrates the problem.

Based on the stack trace I don't believe this requires a live connection in order to reproduce.

@castarco
Copy link
Contributor

castarco commented May 22, 2023

Disclaimer: I'm not related to the Knex project.

Have you checked which is the minimum version that makes your code break? (2.4.0 vs 2.4.1 vs 2.4.2). That can help to pinpoint the cause.

It also seems that you didn't link a scenario with code to reproduce the problem, only a "default" runkit, with no instructions or whatsoever (for example, the where clauses I can see there are not using objects, as you do).

@jukkaleh-atoz
Copy link

See #1227

@perry-autofi
Copy link
Author

perry-autofi commented May 22, 2023

@jukkaleh-atoz That's not my issue. Also I'm using the latest mysql2 "driver". I don't think the query has even made it to the "driver" at this point, it's blowing up while generating the query. You can see from the stack trace that it's still in knex code.

@jukkaleh-atoz
Copy link

@jukkaleh-atoz That's not my issue. Also I'm using the latest mysql2 "driver". I don't think the query has even made it to the "driver" at this point, it's blowing up while generating the query. You can see from the stack trace that it's still in knex code.

Knex added anti sql injection checks that are very strict and partially broken. Previously working code was affected regardless of mysql driver.

@perry-autofi
Copy link
Author

My point exactly. A minor version or patch version should not break existing code. It appears that 2.4.2 breaks existing code. Not good.

@jukkaleh-atoz
Copy link

jukkaleh-atoz commented May 24, 2023

Neither should any project use knex these days, but here we are. Knex isn't what you would call well maintained and actively developed.
SQL injection checks were added to address CVE vulnerability so you win some and lose some. If you revert back to previous version you risk exposing your [mysql] database to the internet. If you don't you need to rewrite queries into something else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants