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

Add PDO::quoteName() method #167

Closed
wants to merge 1 commit into from
Closed

Add PDO::quoteName() method #167

wants to merge 1 commit into from

Conversation

vrana
Copy link
Contributor

@vrana vrana commented Aug 19, 2012

Includes MySQL, SQLite, PgSQL, Firebird, Oracle and MS SQL implementations.

This method allows creating higher level libraries building the SQL command with proper escaping of identifiers. For example $db->select('id', 'title')->from('article')->where('id', $id).

It also allows using user input for table or column names: 'SELECT * FROM t ORDER BY ' . $pdo->quoteName($_GET['order']) can cause SQL error but it couldn't cause SQL injection.

@travisbot
Copy link

This pull request fails (merged b417336 into b2a74b5).

@nikic
Copy link
Member

nikic commented Aug 19, 2012

Are these implementations safe against charset based attacks?

@lstrojny
Copy link
Contributor

@nikic doesn’t look like it is, as it doesn’t use DBMS specific name quoting functions but I’m not that sure there are any. @vrana I would like to see this improvement in PDO, as it helps DBAL implementations. Could you add tests for the other RDBMS as well?

@lstrojny
Copy link
Contributor

lstrojny commented Sep 6, 2012

And I would suggest renaming the method to quoteIdentifier().

@gsdevme
Copy link

gsdevme commented Sep 7, 2012

ZF 1 has its own version of this called quoteIdentifier(), be nice to keep the name the same

@realityking
Copy link
Contributor

I like the name - it's the same the Joomla platform uses :)

@weltling
Copy link
Contributor

Implementation looks good on windows. Nevertheless multibyte compatibility is also important as Johannes mentioned.

@lstrojny
Copy link
Contributor

@johannes @weltling can you add something definitive about multibyte support?

@johannes
Copy link
Member

It's not only multi-byte, might also bge fun like EBCDIC etc. I doubt anybody uses these things but there is a risk and there are databases supporting other charsets and anything getting into PDO core should be robust. On the other hand: Many parts (i.e. PS parameter parser) of PDO already have such assumptions builtin ...

@weltling
Copy link
Contributor

With the multibyte - there are definitely DBs in the outer world allowing multibyte chars for identifiers. May be it could be done reading internal_encoding and (if needed) iterating the string using mbstring as a dependency. Looks like that's a bit more global subject for the PDO improvement :)

@yohgaki
Copy link
Contributor

yohgaki commented Jun 27, 2013

Proper identifier escaping should be done by database provided lib.
PostgreSQL provides PQescapeIdentifier() for this.

It may be good to start from PostgreSQL, but not many DBMS provides API.

@johannes
Copy link
Member

We should try to add only truly robust things to core and fi issues in PDO before adding new ones. This functionality can be added to userspace libs easily. Closing for now.

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

Successfully merging this pull request may close these issues.

9 participants