_protect_identifiers not reset after database query

[eluser]Unknown[/eluser]
Hi,

First of all, I must say thank you for making this wonderful framework. I enjoy every minute developing applications using CodeIgniter.

I just found out a "glitch", which might be a bug. If it is not a bug, please move this topic to the appropriate section of the forum.

In my application, I perform multiple database queries per each request. For some queries, I have to turn off the identifiers protection feature (i.e. adding backticks to table names etc.) . For example:
$this->db->select('UNIX_TIMESTAMP(`timestamp`) AS `date`', FALSE);
$this->db->get();

The problem is, all the subsequent queries using get() or get_where() will automatically have the backtick feature turned off. This causes inconsistencies, and in some cases database query errors in my application. Further digging into DB_active_rec.php file in the database folder reveals the source of the problem:

Code:

/**
     * Select
     *
     * Generates the SELECT portion of the query
     *
     * @access    public
     * @param    string
     * @return    object
     */
    function select($select = '*', $escape = NULL)
    {
        // Set the global value if this was sepecified    
        if (is_bool($escape))
        {
            $this->_protect_identifiers = $escape;
        }
        
        if (is_string($select))
        {
            $select = explode(',', $select);
        }

        foreach ($select as $val)
        {
            $val = trim($val);

            if ($val != '')
            {
                $this->ar_select[] = $val;

                if ($this->ar_caching === TRUE)
                {
                    $this->ar_cache_select[] = $val;
                    $this->ar_cache_exists[] = 'select';
                }
            }
        }
        return $this;
    }

Seems like the private property _protect_identifiers is not reset after each SELECT query. Is it a bug? Or is it a deliberate behavior of Active DB?

Thank you in advance.

[eluser]xwero[/eluser]
The bug also can cause other problems, for example inside the query building.

Code:

$this->db->select(‘UNIX_TIMESTAMP(`timestamp`) AS `date`’, FALSE);
$this->db->select('YEAR(timestamp) year');

The fields in the second select will not be protected because of the change in the global setting.

As i browsed through the code i have seen no reason why the fields should go through the _protect_identifiers method, which is a confusing name as the db library has a class value by the same name, in the _compile_select method.

Code:

if (count($this->ar_select) == 0)
            {
                $sql .= '*';        
            }
            else
            {                
                // Cycle through the "select" portion of the query and prep each column name.
                // The reason we protect identifiers here rather then in the select() function
                // is because until the user calls the from() function we don't know if there are aliases
                foreach ($this->ar_select as $key => $val)
                {
                    $this->ar_select[$key] = $this->_protect_identifiers($val);
                }
                
                $sql .= implode(', ', $this->ar_select);
            }

select fields from the select_max, select_min, select_avg and select_sum method who's value has gone through the _protect_identifiers method already have to go through it again.

A thorough fix would be to change the _compile_select code to

Code:

$sql = (count($this->ar_select) == 0) ? '*' : implode(', ', $this->ar_select) ;

And because _protect_identifiers class variable is set to true by default the second parameter the select method is likely to only be set to false so the method code can change to

Code:

function select($select = '*', $escape = TRUE)
    {
        if (is_string($select))
        {
            $select = explode(',', $select);
        }

        foreach ($select as $val)
        {
            $val = trim($val);

            if ($val != '')
            {
                if($escape != FALSE)
                {
                   $val = $this->_protect_identifiers($val);
                }                

                $this->ar_select[] = $val;

                if ($this->ar_caching === TRUE)
                {
                    $this->ar_cache_select[] = $val;
                    $this->ar_cache_exists[] = 'select';
                }
            }
        }
        return $this;
    }

It's an whole other ballgame if a method like set_identifier_protection(BOOLEAN) is added to the library.

[eluser]sujimohan[/eluser]
Solution to this problem is very simple.

Just use it like

Code:

$this->db->select(<query string>,false);

[eluser]mddd[/eluser]
@garyc40: I think you are right, this is a bug and should be corrected. I suggest you submit it on the bitbucket CodeIgniter site so it can be fixed for the 2.0 release. Maybe it's already been adressed, this behaviour has been known for some time but that shouldn't stop us from fixing it.

In general: I think the ultimate solution should be to have an escape routine that knows what to escape and what not to escape. Which will probably be a big job to do correctly. But that would be the best. Then the need for setting escaping on/off would totally disappear!