Welcome Guest, Not a member yet? Register   Sign In
prevent database update when no where statement found
#1

[eluser]a&w[/eluser]
What is an effective way to prevent updating your entire table with an update when you carelessly omit for whatever reason to include a "where" statement?

Is there anything already provide by the database class to prevent such a blunder?


Edit: I do remember some discussion about this in the past, unfortunately my forum search came up fruitless.
#2

[eluser]Michael Wales[/eluser]
Don't carelessly omit the where statement.

Code:
function save($id = NULL, $new_data = array()) {
  if ((is_int($id)) && (count($new_data) > 0)) {
    $query = $this->db->update('table', $new_data, array('id' => $id));
    if ($this->db->affected_rows() === 1) {
      return TRUE;
    }
  }
  return FALSE;
}
#3

[eluser]Michael Wales[/eluser]
To expound on this a bit more - the update() method is doing exactly what you tell it to. Just as with MySQL, if you omit the WHERE portion of the statement it will update the entire table (since that is what you are telling it to do). CodeIgniter's the same way - you are telling it to update the entire table - it's going to do what you tell it to do.
#4

[eluser]Colin Williams[/eluser]
You should also have your database backed up every so often.
#5

[eluser]a&w[/eluser]
Thanks for the reply, but with all due respect, you missed my point. I realize it is my mistake. I was asking to see if someone came up with some kind of error check or class modification to prevent that.

Basically before doing any update statement, kill the update if no "where" portion of the query has been provided. This would be a collosal blunder on a production site should someone make some errant mistake like this.

I added:
Code:
if ($this->restrict_no_where)
{
    return $sql = '';
}

to the appropriate db driver (mysqli_driver, etc) to illustrate my point. I could add that property to the config for that database or overwrite where needed perhaps.

Code:
/**
     * Update statement
     *
     * Generates a platform-specific update string from the supplied data
     *
     * @access    public
     * @param    string    the table name
     * @param    array    the update data
     * @param    array    the where clause
     * @param    array    the orderby clause
     * @param    array    the limit clause
     * @return    string
     */
    function _update($table, $values, $where, $orderby = array(), $limit = FALSE)
    {
        foreach($values as $key => $val)
        {
            $valstr[] = $key." = ".$val;
        }
        
        $limit = ( ! $limit) ? '' : ' LIMIT '.$limit;
        
        $orderby = (count($orderby) >= 1)?' ORDER BY '.implode(", ", $orderby):'';
    
        $sql = "UPDATE ".$table." SET ".implode(', ', $valstr);
/*CHANGE*/
        if (!$where && $this->restrict_no_where)
        {
            return $sql = '';
        }
/*CHANGE*/
        else
        {
            $sql .= ($where != '' AND count($where) >=1) ? " WHERE ".implode(" ", $where) : '';
        
        }
        
        $sql .= $orderby.$limit;
        
        return $sql;
    }
#6

[eluser]Michael Wales[/eluser]
Oh - for like a development environment, to make sure you don't jack up your DB?

I would just override the Active Record class modifying that one function, add in a simple check for it that calls show_error() if count($values) < 1

Then when it comes time to go production, you just have to delete that one file from your application's libraries directory.

Make a hack of it - no need for there to be a config entry and what-not, you don't want to spend more time on your development environment than what you are developing. Wink
#7

[eluser]Colin Williams[/eluser]
Quote:This would be a collosal blunder on a production site should someone make some errant mistake like this.

I can guarantee that one day, such a thing will happen. Which is why I recommend regularly backing up the database, on all environments, development, staging and production. Back it up and repo it
#8

[eluser]a&w[/eluser]
What is your perception on why this would be bad to leave in place?

Using the config seemed plausible because the config already has an option to disable debugging.

So to that I'd just add:
Code:
$db['default']['hostname'] = $hostname;
$db['default']['username'] = $username;
$db['default']['password'] = $password;
$db['default']['database'] = $database;
$db['default']['dbdriver'] = "mysqli";
$db['default']['dbprefix'] = "";
$db['default']['pconnect'] = TRUE;
$db['default']['db_debug'] = $db_debug;
/* */
$db['default']['restrict_no_where'] = TRUE;
/* */
$db['default']['cache_on'] = FALSE;
$db['default']['cachedir'] = "";
$db['default']['char_set'] = "utf8";
$db['default']['dbcollat'] = "utf8_general_ci";

I currently have a production switch so it disables error checking, debugging, etc. when the files are not on the development site.

Code:
switch(IS_PRODUCTION) {
    case false:
        // Local (development) server
        $hostname  = 'localhost';
        $username  = 'root';
        $password  = '####';
        $db_debug  = 'msg';//true; //do report database errors (echos errors to page, can't catch via ajax)
        break;
    default:
        // Remote (production) server
        $hostname  = 'localhost';
        $username  = 'root';
        $password  = '####';
        $database = 'cc_issues';
        $db_debug  = false; //don't report database errors
        break;
}
#9

[eluser]Michael Wales[/eluser]
Yeah, that would be a good place for it. It's all a matter of personal preference - to me, this is a hack to keep me from screwing up my development database. Personally, I would never use it because if I tell CI to update the whole database, that's what I want it to do.

So, I guess what I am trying to say is - it's your environment, do what you want to make it work for you. Smile
#10

[eluser]a&w[/eluser]
Ok, thanks. I appreciate your insight.

For me, I guess I'd like to error on the conservative side. I can still update without the where just by setting that property accordingly. Kind of like how you can switch protect identifiers on/off.

As it turns out just returning the empty string for sql works to kill that query, but doesn't work with transactions. An empty sql '' statement throws a debug message only, it won't set the transaction status to false, so the transaction still goes through.

That is unexpected or maybe a bug? I mean if it threw an error message with an invalid query I would think that would merit not committing the transaction, no?

circa line 244 of DB_driver.php
Code:
if ($sql == '')
        {
            if ($this->db_debug)
            {
                log_message('error', 'Invalid query: '.$sql);
                return $this->display_error('db_invalid_query');
            }
            return FALSE;
        }

....
        // Run the Query
        if (FALSE === ($this->result_id = $this->simple_query($sql)))
        {
            if ($this->save_queries == TRUE)
            {
                $this->query_times[] = 0;
            }
        
            // This will trigger a rollback if transactions are being used
            $this->_trans_status = FALSE;

I think that first if block should set the transaction status to false.




Theme © iAndrew 2016 - Forum software by © MyBB