Skip to content
This repository has been archived by the owner on May 16, 2018. It is now read-only.

Zend Framework 1.12.7 breaks functions in the order clause of Zend_Db_Select #381

Closed
joefresco opened this issue Jun 16, 2014 · 2 comments
Closed

Comments

@joefresco
Copy link

The following code fails:

$where = '`task`.`status` = "'.$this->appSpace()->task_filter.'"';
$order = 'IF(`task`.`urgent`=1,1,0) DESC, `task`.`created_on` ASC';
$tSelect = $taskTable->select(array('*'))
                    ->setIntegrityCheck(false)
                    ->joinLeft('partner', 'partner.id = task.assigned_to', array('assigned_to_name' => 'name'))
                    ->where($where)
                    ->order($order)
                    ->limit($resultsPerPage, $offSet);

$taskRowSet = $taskTable->fetchAll($tSelect);

The output SQL is:

SELECT `task`.*, `partner`.`name` AS `assigned_to_name` 
FROM `task`
LEFT JOIN `partner` ON partner.id = task.assigned_to 
WHERE (`task`.`status` = "pending") 
ORDER BY `IF(``task```.```urgent``=1,1,0) DESC, ``task```.```created_on``` ASC 
LIMIT 20

It should be

SELECT `task`.*, `partner`.`name` AS `assigned_to_name` 
FROM `task`
LEFT JOIN `partner` ON partner.id = task.assigned_to 
WHERE (`task`.`status` = "pending") 
ORDER BY IF(`task`.`urgent`=1,1,0) DESC, `task`.`created_on` ASC 
LIMIT 20

My workaround was to change line 604 of Zend_Db_Select back to what it was in 1.12.6 (I'd already made 1.12.7 live before discovering the change)

from this

if (preg_match('/^[\w]*\(.*\)$/', $val)) {

to this

if (preg_match('/\(.*\)/', $val)) {

I realize that this undoes the security fixes of 1.12.7

@joefresco joefresco changed the title Zend Framework 1.12.7 breaks with functions in the order clause Zend Framework 1.12.7 breaks functions in the order clause of Zend_Db_Select Jun 16, 2014
@weierophinney
Copy link
Member

Honestly, I think in this case you'd be better served using a Zend_Db_Expr with your ORDER clause:

$order = array(
    new Zend_Db_Expr('IF(`task`.`urgent`=1,1,0) DESC'),
    new Zend_Db_Expr('`task`.`created_on` ASC')
);

A Zend_Db_Expr is more appropriate here, due to the fact that you're using a combination of:

  • function call (IF)
  • table + column name quoting
  • array values for comparison

One other note: use the database adapter's quoting mechanism, and the table name stored in the gateway!

$adapter = $taskTable->getAdapter();
$table = $adapter->quote($taskTable->info('name')); // this is portable now!
$order = array(
    new Zend_Db_Expr('IF(' . $table . '.' . $adapter->quote('urgent') . '=1,1,0) DESC'),
    new Zend_Db_Expr($table . '.' . $adapter->quote('created_on') . ' ASC')
);

The above will be quoted correctly based on the current adapter, and will work even with 1.12.7.

@ezimuel
Copy link
Contributor

ezimuel commented Jun 18, 2014

@joefresco The $order in your SQL should be an array because it contains a list of function and column. The correct one is:

$order = array('IF(`task`.`urgent`=1,1,0) DESC','task.created_on ASC');

Your SQL statement worked before ZF 1.12.7 because the regular expression was wrong and accepted all kind of string containing parenthesis, see ZF2014-04 for more details.

@ezimuel ezimuel closed this as completed Jun 18, 2014
ezimuel added a commit that referenced this issue Jun 19, 2014
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

3 participants