Underscores are unnecessarily escaped in db_mysql

[hokamoto]

Architecture

No response

Operating System

No response

Disk

No response

Memory

No response

CPU Cores

No response

Internet Connection

No response

What is your web browser?

No response

Device

No response

What happened?

dbQuote() has been modified to escape special characters in #15234. Underscore(_) is a special character only in pattern-matching contexts for LIKE operator, so it should not be escaped anywhere else. Underscores in inserted values or expressions for WHERE clause without LIKE operator are unnecessarily escaped due to this bug.
https://dev.mysql.com/doc/refman/8.0/en/string-literals.html

Current Standard Output Logs

import db_mysql
echo dbQuote("SELECT * FROM foo WHERE col1 = 'bar_baz'")
echo dbQuote("SELECT * FROM foo WHERE col1 LIKE '%bar_baz%'")

'SELECT * FROM foo WHERE col1 = \'bar\_baz\''
'SELECT * FROM foo WHERE col1 LIKE \'%bar\_baz%\''

Expected Standard Output Logs

'SELECT * FROM foo WHERE col1 = \'bar_baz\''
'SELECT * FROM foo WHERE col1 LIKE \'%bar\_baz%\''

Possible Solution

1. Escape underscores only for LIKE operators
2. Do not escape underscores

For example, PHP does not seem to escape underscores.
https://www.php.net/manual/ja/mysqli.real-escape-string.php#46339

Additional Information

This issue could happen for percent signs (%), but it was fixed for percent sings in 8e181cb

[hokamoto]

$ nim -v
Nim Compiler Version 1.6.6 [Linux: amd64]
Compiled at 2022-05-05
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 0565a70eab02122ce278b98181c7d1170870865c
active boot switches: -d:release

[bung87]

that's weird example for use case of dbQuote, dbQuote is used to quote single param to replace placeholder, here is use for whole sql, the 8e181cb wasn't correct fix c16ee37#r44209990 in that use case should be %?% then quote param, not put user source value to it directly. for this one fix should take Solution 1 IMO.

[hokamoto]

As you pointed out, similar features in other languages are used for escaping parameters, not for escaping entire SQL statements, but there is a reason I mentioned the 'weird' example. dbFormat() function automatically performs dbQuote() on all parameters.
https://github.com/nim-lang/Nim/blob/devel/lib/impure/db_mysql.nim#L145
This causes a problem when the value for INSERT statements contain _ (underscore) character. I admit that the SELECT statement was not the best example, but in any case, dbQuote() was causing problems for INSERT statements.

The actual code of mine was as follows. Broken values got inserted when original_filepath contains _ (underscore) characters. This code worked before.

db.exec(sql"INSERT INTO files (hash, original_filepath, width, height) VALUES (?, ?, ?, ?)", hash, original_filepath, width, height)

[bung87]

so % and _ should quote when use like, need revert the revert waiting for correct fix and correct the test file that's misleading users.

[hokamoto]

I disagree with reverting this fix. If someone can implement Solution 1 correctly, I would welcome it (but I think it would be difficult), but Solution 2 recently made should not be reverted. Since exec() automatically calls dbQuote(), this problem can't be avoided by tweaking the caller code when parameter binding is used. In other words, there is no way to insert a value containing underscores if we revert the fix.

By updating this library recently without realizing that the escaping of underscores had been added, I kept getting broken values inserted into my database.

[hokamoto]

Or, dbFormat() should stop calling dbQuote() automatically. Leaving it up to the caller to decide whether to escape the value to be bound is an option. That is acceptable to me.

[bung87]

I disagree with reverting this fix. If someone can implement Solution 1 correctly, I would welcome it (but I think it would be difficult), but Solution 2 recently made should not be reverted. Since exec() automatically calls dbQuote(), this problem can't be avoided by tweaking the caller code when parameter binding is used. In other words, there is no way to insert a value containing underscores if we revert the fix.

By updating this library recently without realizing that the escaping of underscores had been added, I kept getting broken values inserted into my database.

that's may not difficult than you think , we have parsesql library(maybe just use string find to detect like statement), and as you concerns, just waiting correct fix. and sql query string should prefix with sql , plain string is wrong. so there's space tweaking the sql, for users use plain string they already taken risk.

[hokamoto]

exec() takes a SqlQuery argument, so I believe we can't pass a string
https://github.com/nim-lang/Nim/blob/devel/lib/impure/db_mysql.nim#L160

Am I missing something? In the following code, when the original_path has underscores in it, is there any way to correctly INSERT this value into the database even after reverting the fix?

db.exec(sql"INSERT INTO files (hash, original_filepath, width, height) VALUES (?, ?, ?, ?)", hash, original_filepath, width, height)

[bung87]

exec() takes a SqlQuery argument, so I believe we can't pass a string https://github.com/nim-lang/Nim/blob/devel/lib/impure/db_mysql.nim#L160

Am I missing something? In the following code, when the original_path has underscores in it, is there any way to correctly INSERT this value into the database even after reverting the fix?

db.exec(sql"INSERT INTO files (hash, original_filepath, width, height) VALUES (?, ?, ?, ?)", hash, original_filepath, width, height)

so there's no worry about, we can detect like statement and decide whether quote % and _ or not. I mean revet will comming along with correct fix in single PR.

[hokamoto]

I got it. I agree with you.