-
-
Save croxton/2923657 to your computer and use it in GitHub Desktop.
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; | |
} |
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.
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.
Hi stefkes, please let me know if the fix I posted above above works for that query.
@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..
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?
MySQL. Ill do some more tests tomorrow and report back.
@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.
@croxton, your code makes this query work, it failed before.
$this->db->where('id >', $id);
$res['next'] = $this->db->get('products')->row();
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. :)
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.
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.
Correct this should be:
As for:
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.