FaaPz / PDO

Just another PDO database library
MIT License
316 stars 103 forks source link

Escape column names #77

Closed nasyrov closed 3 years ago

nasyrov commented 7 years ago

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

shaq147 commented 7 years ago

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 5 years ago

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.

nasyrov commented 5 years ago

Hi @kwhat ,

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

kwhat commented 5 years ago

Hi @nasyrov,

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

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.

nasyrov commented 5 years ago

Hi @kwhat ,

No worries, thanks for looking at it!

kwhat commented 3 years ago

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