FaaPz/PDO

Escape column names

Closed this issue · 6 comments

Hi there,

When it compiles the query it doesn't escape column names in where section.

Ex,

$stmt = $pdo
    ->select()
    ->from('region')
    ->where('default', '=', true)
    ->execute()

To fix I have to prefix the column names,

$stmt = $pdo
    ->select()
    ->from('region')
    ->where('region.default', '=', true)
    ->execute()

Guess, it would be nice if it could output something like

SELECT * FROM `region` WHERE `default` = 1

Cheers,
Evgenii

I'm guessing you want it to automate prefixing in the table name (or table alias name) on the whereClause?

I don't think this is possible since if you are doing a join with other tables, the function wouldn't know which, for example, id column refers to which table. You'll have to prefix it manually when writing the statement.

If there were no joins with other tables, then it is totally possible and easy to implement, but then it would be pointless because you don't need to escape the column names (since column names within one table must be unique)

kwhat commented

I had thought about this and decided not to escape the columns anywhere in the library. Generally speaking, you probably shouldn't be accepting column names from user input or at least verifying that the columns are in a pre-approved list. Adding the columns to the prepared statement would be a bit of a pain but is possible. If you or anyone else really wants it, please make a compelling argument as to why.

Hi @kwhat ,

That was a long long thinking :)
Anyway cheers for that!

kwhat commented

Hi @nasyrov,

I did play around with some implementations for escaping some of this stuff. Here are the highlights:

  • Prepared statements can only be used for "parameter values" (where/limit/on)
  • Statements can escape extra stuff manually via PDO::quote()
  • Clauses cannot escape stuff easily as they do not have a PDO reference.

I am not ruling this out, I am just not going to hold up 2.x for it. The 3rd bullet point is really the big issue here.

Hi @kwhat ,

No worries, thanks for looking at it!

kwhat commented

I have decided against implementing this. It is escaping column names is unreliable and would probably introduce injection issues for those relying on it.