[bug#30386] [PATCH v2 cuirass] database: Prevent SQL injection.

[Danny Milosavljevic <dannym@scratchpost.org>]

Hi Ludo,

no worries!

> optimization looks good, provided the extra conditions don’t make sqlite
> slower.  

Compared to parsing the SQL text again and again (which is dead slow), I think
an extra NULL check *on the same field* is not going to matter at all.

Even compared to using lots of main memory and thus not being able to use
the processor's cache (if we had lots of prepared statements), I think an
extra NULL check is still better :)

Of course once we have a lot of data in the tables, the actual lookup costs
will dwarf any setup costs.  Then still, it's checking the same field that's
used anyway, so the extra cost should be neglible.

>Do you think we can salvage this bit from your patch?  

Sure.

> It might allow us to use ‘sqlite-exec’ directly, and thus
> benefit from the binding support in there, as in:
> 
>   (sqlite-exec db "… WHERE " id " is NULL or …")

I added sqlite-bind-arguments with keyword arguments specifically so sqlite-exec
doesn't suck.

So it would be like (sqlite-exec db "SELECT … :a … :b … :a"
                                    #:a 42
                                    #:b 2)

Before, it was:

(sqlite-exec db "SELECT … ? … ? … ?"
                42
                2
                42)

which repeated stuff - and was very fragile when changing things (one can easily
get the order wrong and it would not have errored out).