Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Escaping of table, column and index name is wrong #267

Open
rarog opened this issue Sep 26, 2017 · 2 comments
Open

Escaping of table, column and index name is wrong #267

rarog opened this issue Sep 26, 2017 · 2 comments

Comments

@rarog
Copy link

rarog commented Sep 26, 2017

It seems that escaping is wrong for at least DDLs, I didn't test SELECT and other commands.
It goes wrong with (MySQL) and without (SQLite) decorators. The example for SQLite is flawed in terms of syntax, as the basic index creation isn't supported within CREATE TABLE command, but this should be a separate task similar to #163. This bug is only for the escaping issue.
Instead of just doubling the escape character there seems to be done some wrong escaping which is additionally run multiple times.

I tried following:

$table = new CreateTable('t\'e"s`t');
$table->addColumn(new BigInteger('t\'e"s`tCol'))
    ->addColumn(new BigInteger('t\'e"s`tCol2'))
    ->addConstraint(new Constraint\PrimaryKey('t\'e"s`tCol'));
    ->addConstraint(new Index('t\'e"s`tCol2', 't\'e"s`tIndex'));

Expected output for MySQL:

CREATE TABLE `t'e"s``t` ( 
    `t'e"s``tCol` BIGINT NOT NULL AUTO_INCREMENT,
    `t'e"s``tCol2` BIGINT NOT NULL,
    PRIMARY KEY (`t'e"s``tCol`),
    INDEX `t'e"s``tIndex`(`t'e"s``tCol2`)
)

Actual output for MySQL:

CREATE TABLE `t'e"s``t` ( 
    `t``'``e``"``s``````tCol` BIGINT NOT NULL AUTO_INCREMENT,
    `t``'``e``"``s``````tCol2` BIGINT NOT NULL,
    PRIMARY KEY (`t``'``e``"``s``````tCol`),
    INDEX `t``'``e``"``s``````tIndex`(`t``'``e``"``s``````tCol2`)
)

Expected output for SQLite:

CREATE TABLE "t'e""s`t" ( 
    "t'e""s`tCol" BIGINT NOT NULL,
    "t'e""s`tCol2" BIGINT NOT NULL,
    PRIMARY KEY ("t'e""s`tCol"),
    INDEX "t'e""s`tCol2")
)

Actual output for SQLite:

CREATE TABLE "t'e's`t" ( 
    "t""'""e""'""s""`""tCol" BIGINT NOT NULL,
    "t""'""e""'""s""`""tCol2" BIGINT NOT NULL,
    PRIMARY KEY ("t""'""e""'""s""`""tCol"),
    INDEX "t""'""e""'""s""`""tIndex"("t""'""e""'""s""`""tCol2")
)

Not shown in the above example - the escaping also differs if for example the quotation mark is in the middle or in the beginning or end of the name.

Edit1:
Removed above the part with stylers, this problem is completely independent. At least the problem with table name misquotation can be fixed in Zend\Db\Adapter\Platform\AbstractPlatform class by changing $quoteIdentifierTo to

    protected $quoteIdentifierTo = '""';

In the same time overwriting of $quoteIdentifierTo in Zend\Db\Adapter\Platform\Postgresql and overwriting of $quoteIdentifier and $quoteIdentifierTo in Zend\Db\Adapter\Platform\Sqlite could be removed.

Edit2:

The other problem with the broken column and index names comes from function processExpression in Zend\Db\Sql\AbstractSql

                } elseif ($type == ExpressionInterface::TYPE_IDENTIFIER) {
                    $values[$vIndex] = $platform->quoteIdentifierInFragment($value);

function quoteIdentifierInFragment tries to split the string on all possible special chars, which breaks the column and index names. This parsing part is probably needed for true expressions where the code wasn't assembled by Zend\Db components but by hand in string. But in case of Ddl functions and probably all other classes like Select, that assemble the SQLs in correct way, this definitely breaks the functionality.
Either there should be special check for those classes (which I consider a dirty and too hardcoded way) or there should be introduced a new type named ExpressionInterface::TYPE_IDENTIFIER_UNBREAKABLE (or _UNSPLITTABLE or _WHATEVER) and be used by function getExpression() in all classes derived from Zend\Db\Sql\Expression and in that case function processExpression should call quoteIdentifier.

@rarog
Copy link
Author

rarog commented Sep 26, 2017

Added notes about further analysis of the origin of this problem to the initial text.

@michalbundyra
Copy link
Member

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at laminas/laminas-db#66.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants