Re: contrib: hack for psycopg2 performance

On Wed, May 12, 2010 at 12:40 PM, Karsten Hilbert
<[email protected]> wrote:

> However, if I deliberately go
>
>        import psycopg2
>        psycopg2.use_scary_Params_call_and_let_me_shoot_my_foot = True
>
> it would switch to using *Params and let me live with it.
>
> Note that I am not asking for psycopg2 to check and
> deliberately except on me should I still try to chain SQL
> with ";". I would have to live with whatever consequences
> ensue.

I feel the existence of this option a kind of creepy sign that "by
default psycopg is less safe": try to write its documentation: I guess
it would be awkward. Something as "if you choose *params, nothing
changes. But you can't send more statements split by ";". But it is
somewhat more secure". I really feel it not justifiable, or at least
not in a way I can explain.


>> _Assuming_ that psycopg will work fine in both cases, *Params and not *Params (in the
>> set of queries accepted by the more restrictive), why should the user
>> be loaded with the burden of choosing which one to use?
>
> Because *Params offers a quantitative advantage (speed) and
> a qualitative advantage (*ridding* oneself of SQL injection).

Do we have a measure of the speed advantage? The one reported by the
OP is about a edge case of a query with 1500 integer parameters,
whereas string parameters (i.e. all the ones for which we decide not
to mess with their server binary representation) would not be affected
by the speedup - let alone the fact that regular queries hardly have
more than a dozen params. In order to gain this speedup, you have to
drop any roundtrip optimization and still mangle the query in order to
replace the psycopg placeholders (%s or %(name)s) with the libpq
placeholders (whose are in $1, $2... style). So, there is not really a
clean-cut winner.

About ridding of the SQL injection, I think psycopg is already doing
its best by trusting the libpq. It there was a problem - a bug, a
weakness - we obviously should look for a solution, but it doesn't
seem we are in this situation.


>> > I know I would use that in GNUmed.
>>
>> Would you choose the *Param way because you don't trust psycopg in
>> performing the right escape or for some other reason?
>
> Because when dealing with your Hep C/HIV lab results or
> pregnancy data or transgender operational procedures I'd
> rather choose the most secure path possible even if that
> should incur a (reasonable) penalty. It is not a matter of
> trust (which is quantitative) but rather of
> can-or-cannot-happen (as in binary).

I think we are talking about two different level of security here.
Having sensible data sniffed or altered by a man in the middle is not
the kind of attack that parameters escaping deals with, and AFAIK data
on the wire can be read/altered both with the params already merged to
the query and with the params in a separate structure. This kind of
protection is up to the ssl layer.


> I do appreciate the argument of not desiring to support two
> code paths.

Unfortunately we had to, if we decided to support *Params for any
reason. Notice anyway that I consider this the weakest of the points:
as already emerged in other discussions, I think avoiding a feature
just to avoid its maintenance is a weak argument. If there are
scenarios in which *Params is the best choice (as .executemany() may
be, but then we may use PQexecPrepared instead) it would be only
laziness not going for it.


-- Daniele