Hi Kris,
I was lacking time to work on this as I was quite busy at work as well
as with my family, but I am going to finish this in the next days.
Kris Jurka wrote:
> Looks good so far. So two typos in comments. Incremental diff attached.
Thanks, included in my local patch.
> ...
> Doesn't really matter how you pass this around. The patch's usage of an
> additional boolean parameter looks fine to me.
Ok.
>> Question 2:
>> -----------
>> The problem with the V2 protocol code is, that tracking the state of
>> standard_conforming_strings is not really possible, AFAICS. Currently
>> I just read standard_conforming_strings at startup, and that's it. So
>> as soon as someone changes the variable, quoting will be incorrect,
>> which is a security issue. Therefore my new suggestion is to used the
>> E'' string syntax in the V2 protocol path if the server version is >=
>> 8.2. The version of the server cannot change during the connection, so
>> we are save here.
>
> I'm not super worried about this. If an administrator changes the value
> in postgresql.conf and HUPs the postmaster they really can't expect
> every client to pick this up immediately. Even when we are monitoring
> the state (V3) we won't know it has changed until after we execute the
> next query (with the wrong escaping setting). If an interface allows
> the user to enter arbitrary SQL such that they can change this setting
> then it's not really a security issue because they don't need to break
> escaping to sneak a query in because they can already execute any query
> they want.
I mostly thought about cases where applications assume they can set
standard_conforming_strings, or when administrators change the settings
while an application server (with pooled connections) is running. We
should at least document this issue.
>> For question 1, I would then suggest to add an argument to the V2Query
>> constructor that tells whether E'' should be used instead of '', or not.
>
> This is a good idea.
It think it's the safest approach for V2Query. I guess we must really
prefix the string literal with " E". Otherwise cases like "BETWEEN?AND?"
will break for string parameters.
>> For parsing inside the JDBC driver, we would still rely on the
>> initially reported value of standard_conforming_strings. We could also
>> add an URL parameter to specify what should be set initially.
>>
>
> I'd like to see a generic way of setting any GUC parameter at startup,
> instead of just this one. The existing methods of setting this per user
> or per database should be OK.
Ok, I will leave that alone for now.
Best Regards
Michael Paesold
