My partner on a PHP project objects my practice of always sanitizing integer values in dynamic SQL. We do use parameterized queries when possible. But for UPDATE and DELETE conditions Zend_Db_Adapter requires a non-parameterized SQL string. That's why I, even without thinking, always write something like:

$db->delete('table_foo', 'id = ' . intval($obj->get_id()));

Which is equivalent, but is a shorter version of (I've checked the ZF source code):

$db->delete('table_foo', $db->qouteInto('id = ?', $obj->get_id(), 'INTEGER'));

My partner strongly objects this intval(), saying that if $obj ID is null (the object is not yet saved to DB), I开发者_运维技巧 will not notice an error, and the DB operation will just silently execute. That's what has actually happened to him.

He says that if we sanitize all the HTML forms input, there's no way an integer ID can possibly get into '; DROP TABLE ...', or ' OR 1 = 1', or another nasty value, and get inserted into our SQL queries. Thus, I'm just paranoid, and am making our lives unnecessarily more complicated. "Stop trusting the $_SESSION values then" he says.

However, for string value conditions he does agree with:

$db->update->(
    'table_foo',
    $columns,
    'string_column_bar = ' . $db->qoute($string_value))
);

I failed to prove him wrong, and he failed to prove me wrong. Can you do either?

Frankly, your partner is off his rocker: sanitizing is cheap, there's no good reason not to do it. Even if you are sanitizing what is in the HTML forms, if those checks somehow break on production, you'll be happy that you have a backup in other places. Also, it promotes a good practice.

You should sanitize—always

Which do you consider more trouble:

• Having to track down a bug that didn't cause a failed SQL query.
• Having to restore data after you make a mistake in sanitizing the form input and someone exploits that.

Whichever you pick, there's your answer. Personally, I tend to lean towards the paranoid side of things as well.

If anything, you could do both: create your own function that first checks for null and then calls intval(), and use that instead. Then you get the best of both worlds.

I think your partner is wrong - he is not considering separation of concerns between data sanatisation in the model (where your DB code lives) and data validation of your forms.

Normally your form validation logic will be in a separate area of the application to your model. I.e. when adding validators to form elements, and so this is often done in the form class itself. The purpose of this layer of validation code is to validate the input of the form and return the appropriate messages if there is anything wrong.

So I think data sanitisation in the model should be considered separately to this, as the Model is really a standalone class - and thus should be responsible for it's own data sanitisation. Since in theory you should be able to re-use this model elsewhere in your application, the model should not then assume that the sanitisation has been done elsewhere, i.e. as part of the form validation layer.

Your partners main point about not noticing failed SQL queries is not really an issue in practice - it is better to code defensively.

You should of course always sanitize and not rely on HTML forms. What if you change your code and reuse that part with some other data, that came not from HTML form but from webservice or email or any other source that you decide to add a year later? Using intval() here seems to be fine.

form input should idd always be sanitized, but not every variable that goes into a query should be sanitized imo...
The origin of the variable does play a significant role here.

You just have to know if the data used can be trusted...

If you look deeper inside the Zend framework code, you'll see that $db->quoteInto() turns into $db->quote which returns (string) intval($value) with INTEGER as type.

If type is undefined, $db->_quote() is called. Its code is following:

protected function _quote($value)
{
    if (is_int($value) || is_float($value)) {
        return $value;
    }
    return "'" . addcslashes($value, "\000\n\r\\'\"\032") . "'";
}

Whatever the calling method used (with or without specifying type), $db->delete is totally safe.

You should first check that the ID is not null. If it is, you know not to do a useless query and continue from there. It does not make sense to track down issues by reading failed SQL queries or so.

All data retrieved from a form should be sanitized. No exceptions. All data retrieved from your system should have already been sanitized before it got into your system, and therefore shouldn't be sanitized when retrieved from the system again.

So, the question is - where is this integer coming from?