Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
CodeIgniter 2.1.0 alias escaping problems
CI 2.1-stable Active Record tries to escape db tables, fields and aliases with backticks, however it can trip up on queries such as...
$this->CI->db->where("last_activity < {$expire}");
... which is an example actually used in the Session class in 2.1-stable.
So out of the box you will get an SQL error when using database sessions.
Of course that should probably be rewritten:
$this->CI->db->where("last_activity <", $expire);
However, this type of query also generates an SQL error:
$this->CI->db->select('DATE_FORMAT(news.date_start, "%a, %d %b %Y %T") as news_date');
Note that passing a second parameter as FALSE does not disable the attempted escaping of field aliases
(it only stops backticks around table names and fields). This also throws an SQL error:
$this->CI->db->select('DATE_FORMAT(news.date_start, "%a, %d %b %Y %T") as news_date', FALSE);
The following is a quick workaround which allows the above to work as expected, but only by disabling backticks
for select / where statements like the above. Note that it will only work if your table names, field names
and aliases use only alphanumeric characters and underscores and you are not using reserved names for any of these:
function _protect_identifiers() in /system/database/DB_driver.php
------- Replace this -------
// If the item has an alias declaration we remove it and set it aside.
// Basically we remove everything to the right of the first space
if (preg_match('/^([^\s]+) (AS )*(.+)$/i', $item, $matches))
{
$item = $matches[1];
// Escape the alias
$alias = ' '.$matches[2].$this->_escape_identifiers($matches[3]);
}
else
{
$alias = '';
}
// This is basically a bug fix for queries that use MAX, MIN, etc.
// If a parenthesis is found we know that we do not need to
// escape the data or add a prefix. There's probably a more graceful
// way to deal with this, but I'm not thinking of it -- Rick
if (strpos($item, '(') !== FALSE)
{
return $item.$alias;
}
------- With this -------
// If the item has an alias declaration we remove it and set it aside.
// Basically we remove everything to the right of the first space
if (preg_match('/^([^\s]+) (AS )*([a-zA-Z0-9_]+)$/i', $item, $matches))
{
$item = $matches[1];
// escape the alias
if ($protect_identifiers === TRUE)
{
$alias = ' '.$matches[2].$this->_escape_identifiers($matches[3]);
}
else
{
$alias = ' '.$matches[2].$matches[3];
}
}
else
{
$alias = '';
}
// This is a bug fix for queries that use MAX, MIN, DATE_FORMAT, <= etc.
// If the the identifier contains any character that is NOT alphanumeric or underscore
// we assume that we do not need to escape the identifier or add a prefix.
if (preg_match('/^([a-zA-Z0-9_\.]+)$/', $item) == 0)
{
return $item.$alias;
}
@philsturgeon

This comment has been minimized.

Copy link

philsturgeon commented Jun 13, 2012

So 2.1 seems like it is breaking things, but is really enforcing more correct syntax as some options were previously ignoring the $escape options, etc.

 $this->CI->db->where("last_activity < {$expire}");

Correct this should be:

$this->CI->db->where("last_activity <", $expire); 

As for:

$this->CI->db->select('DATE_FORMAT(news.date_start, "%a, %d %b %Y %T") as news_date', FALSE);

That is not a quick workaround, that second parameter is there designed entirely to disable backticks, nothing else.

If there is a problem with aliases (highly possible, as Rick notes there is probably a more elegant way) then putting this into the develop branch and unit testing the hell out of it would certainly be useful. If you can show it breaking, then show it working that pull request will go in easily.

@croxton

This comment has been minimized.

Copy link
Owner Author

croxton commented Jun 13, 2012

Hi Phil

thanks for commenting. The 'quick workaround' refers to the code I posted below, not the addition of the FALSE parameter, which doesn't actually disable backticks for aliases as you might expect. That is one possible bug.

The second problem relates to how a fieldname and alias are split in two. Arguably there should be some further parsing of these values before they are enclosed by backticks, so as to ensure that adjacent operators and hardcoded SQL functions are not also mistakenly enclosed by the backticks.

@stefkes

This comment has been minimized.

Copy link

stefkes commented Jun 13, 2012

I encountered the same problem in the session library's garbage collection function. The problem also occurs if you run

$this->db->where('language !=', 'NL'); $res = $this->db->get('translations')->result_array();

This produces

SELECT * FROM (product_translations_old) WHERE language != 'NL'

Obviously there should be no backticks around the != part. I'd say active record is not really working properly at all in 2.1.0.

@croxton

This comment has been minimized.

Copy link
Owner Author

croxton commented Jun 13, 2012

Hi stefkes, please let me know if the fix I posted above above works for that query.

@fabdrol

This comment has been minimized.

Copy link

fabdrol commented Jun 14, 2012

@croxton did your/@philsturgeon's updates fix the problem on your end? I'm having the same issues, but they're not going away.. pretty annoying, since it never happened before in older CI versions..

@philsturgeon

This comment has been minimized.

Copy link

philsturgeon commented Jun 14, 2012

2.1.0 has been around for a long time. If it was completely broken for everyone Im sure somebody would have spoken up before now. This is more likely to be a random thing, but I am of course not ruling out "epic bug" as an option.

For reference, there have been some other bugs in 2.1.1 relating to AR. One of the team snuck a change in during the re-tag process and totally screwed AR, but this has now been reverted.

Out of interest what driver are you guys using? PDO, MySQL or MySQLi?

@stefkes

This comment has been minimized.

Copy link

stefkes commented Jun 14, 2012

MySQL. Ill do some more tests tomorrow and report back.

@croxton

This comment has been minimized.

Copy link
Owner Author

croxton commented Jun 15, 2012

@fabdrol the patch I posted above fixed my problems. It should work for you if your queries were working in an older CI, but please be aware that it assumes you are using field names with alphanumeric characters and underscores only, and you are not using reserved words for field names.

@philsturgeon I'm using MySQL.

@stefkes

This comment has been minimized.

Copy link

stefkes commented Jun 15, 2012

@croxton, your code makes this query work, it failed before.

$this->db->where('id >', $id);
$res['next'] = $this->db->get('products')->row();
@philsturgeon

This comment has been minimized.

Copy link

philsturgeon commented Jun 15, 2012

If a pull request needs to be done for any specific version can one of you guys follow it up? The person with the problem is usually the best positioned to make the PR, otherwise I'm just copying stuff based on someone telling me its right. :)

@stefkes

This comment has been minimized.

Copy link

stefkes commented Jun 15, 2012

I fail to reproduce this error on a clean 2.1 install so I'll try to track down why it happens to some of us. Will follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.