zendframework / zend-db

Db component from Zend Framework
BSD 3-Clause "New" or "Revised" License
101 stars 122 forks source link

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

Open rarog opened 7 years ago

rarog commented 7 years ago

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

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

michalbundyra commented 4 years ago

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at https://github.com/laminas/laminas-db/issues/66.