quoteIdentifier() & quoteIdentifierChain() bug #2670

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@JaredWilliams
Contributor

Identifier escaping is done means of doubling the delimiter character, and not using a backslash.

JaredWilliams added some commits Oct 4, 2012
@JaredWilliams JaredWilliams quoteIdentifier() & quoteIdentifierChain() bug
Identifier escaping is done means of doubling the delimiter character, and not using a backslash.
3ea563e
@JaredWilliams JaredWilliams quoteIdentifierInFragment() also has wrong escaping 2008dba
@ralphschindler
Member

Thats a good catch, can you link the place in the mysql manual that talks about this? Also, any chance you can update the tests associated with qouteIdentifier() and quoteIdentifierChain ?

https://github.com/zendframework/zf2/blob/master/tests/ZendTest/Db/Adapter/Platform/MysqlTest.php

@JaredWilliams
Contributor

http://dev.mysql.com/doc/refman/5.6/en/identifiers.html

Section that starts with "Identifier quote characters can be included within an identifier if you quote the identifier."

Also it's not clear what quoteIdentifierInFragment() should be doing. The preg_split() doesn't take into account ` marks in identifiers. IMHO it's a horrid function and should be deprecated/removed.

@JaredWilliams
Contributor

Also want to look at the SQL Server identifier quoting as that is wrong.

Should be doubling any closing bracket.

http://msdn.microsoft.com/en-us/library/bb399786.aspx

@ralphschindler
Member

Also it's not clear what quoteIdentifierInFragment() should be doing. The preg_split() doesn't take into account ` marks in identifiers. IMHO it's a horrid function and should be deprecated/removed.

The part of me that favors strictness over usability agrees with you. On the other hand, the not-so-strictly typed PHP developer in me understands that in some workflows, you want your components to help you get a project done over getting in your way while doing it.

The art in balancing strictness and usefulness. quoteIdentifierInFragment() is one of those places.

At times, a developer might just want to say "take this and quote it for me". The this I am referring to might be

foo AS bar

The place where this is used is primarily inside Zend\Db\Sql\Select when dealing with fragments of SQL, particularly when dealing with columns and when dealing with join experessions, and group/order expressions where you don't want to create a full expression object, you just want a SQL fragment, with a limited set of keywords, to be quoted as best as it can.

If you have a better idea, I am all ears. If you want to avoid this function, then use Expression objects everywhere. There you get full control over quoting.

You can see some of the various Select unit tests where this is used: (see the m = n) https://github.com/zendframework/zf2/blob/master/tests/ZendTest/Db/Sql/SelectTest.php#L586

I'll try to merge this in as soon as I can.

@akrabat akrabat added a commit that closed this pull request Nov 15, 2012
@akrabat akrabat Merge branch 'hotfix/2670'
Close #2670
2d1b40b
@akrabat akrabat closed this in 2d1b40b Nov 15, 2012
@akrabat akrabat added a commit that referenced this pull request Nov 15, 2012
@akrabat akrabat Merge branch 'hotfix/2670' into develop
Forward port #2670
8943900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment