knex / knex

A query builder for PostgreSQL, MySQL, CockroachDB, SQL Server, SQLite3 and Oracle, designed to be flexible, portable, and fun to use.
https://knexjs.org/
MIT License
19.21k stars 2.12k forks source link

MySQL Back-end does not appear to escape ' in enum values. #4481

Open acrosman opened 3 years ago

acrosman commented 3 years ago

Environment

Knex version: 0.95.4 Database + version: MariaDB 10.5.9 OS: Windows 10

Bug

When building tables with enum columns, providing a value list that contains ' within the values results in a SQL Syntax error triggered by the '. Taking the generated query and escaping the values results in a functional query.

Resulting error:

Standard SQL Syntax error

ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near [then the text immediately following the '].

Reproducing code sample

With a valid connection:

(async function run() {
  await knexSqlite.schema.createTable('test', (t) => {
    t.enu('foo', ["inavlid's value", "valid value"]);
  });
})();
ramos-ph commented 3 years ago

Hi, @acrosman ! Have you been able to solve your problem already? I were able to reproduce the issue. It seems Knex itself is not handling the error, as it is thrown from the dialect module. I'm trying to investigate and come to a solution as soon as possible!

ramos-ph commented 3 years ago

Found the issue on the compiler. Here's an console.log:

[
  {
    sql: "create table `test` (`foo` enum('invalid's value', 'valid value'))", // notice the abscence of \'
    bindings: []
  }
]
acrosman commented 3 years ago

I slapped a solution into my project cause I needed a quick solution, but it is not nearly as good as having it fixed in knex.

ramos-ph commented 3 years ago

Just fixed it! It's working properly now. I'll open a PR and let you know ASAP.

ramos-ph commented 3 years ago

@acrosman Just opened a Pull Request to fix this. Mind giving it a look? Thanks in advance! :smile:

acrosman commented 3 years ago

Thank you for taking a look at this. I offered you some feedback on #4484. I noticed from the PR's processing that this is your first contribution to this project (which is great as far as I'm concerned), so you may want to wait to hear from one of the maintainers before going too much further on this fix to see what thoughts they have. They may suggest a totally different direction.