Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 2 You must be signed in to fork a gist
  • Save croxton/2923657 to your computer and use it in GitHub Desktop.
Save croxton/2923657 to your computer and use it in GitHub Desktop.
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
Copy link

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
Copy link
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
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
Copy link
Author

croxton commented Jun 13, 2012

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

@fabdrol
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
Copy link

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
Copy link

stefkes commented Jun 14, 2012

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

@croxton
Copy link
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
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
Copy link

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
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