[Koha-patches] [PATCH] Bug 1953 [2/3]: refactoring SQL in C4::Items::GetItemsForInventory to use placeholders

[Galen Charlton]

Hi,

On Thu, Jul 31, 2008 at 8:09 AM, paul POULAIN <paul.poulain at biblibre.com> wrote:
> Andrew Moore a écrit :
>> On Thu, Jul 31, 2008 at 7:31 AM, paul POULAIN <paul.poulain at biblibre.com> wrote:
>>
>>> /me disagree : the $dbh->quote() does exactly the same thing as the
>>> placeholder : ie escaping SQL to avoir SQL injections. So this patch
>>> solves nothing on this aspect ;-)
>>>
>> Very well. I wouldn't object to backing these patches out.
>>
> The resulting code is correct. The previous code was also correct. So I
> think both are valid. It's fair not to rollback according. I just wanted
> to point for future, that there is no injection risk with quote()

But there are other problems:

1. Not using placeholders can reduce the effectiveness of query caching.
2. If one relies on $dbh->quote(), it is too easy to miss when someone
forgets to use it.  If we settle on using placeholders and bind
variables for all queries, it's much easier to verify by visual
inspection whether a given query has an obvious vector for an
injection bug.

I am *not* in favor of backing out this patch.

Regards,

Galen
-- 
Galen Charlton
VP, Research & Development, LibLime
galen.charlton at liblime.com
p: 1-888-564-2457 x709
skype: gmcharlt