Prev: Port for Microsoft Services for Unix (SFU) or SUA
Next: [HACKERS] solution to make static changes in pg_hba.conf file?
From: Pavel Stehule on 19 Jan 2010 12:54 2010/1/19 Robert Haas <robertmhaas(a)gmail.com>: > On Tue, Jan 19, 2010 at 4:13 AM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >> 2010/1/18 Robert Haas <robertmhaas(a)gmail.com>: >>> On Mon, Jan 18, 2010 at 3:26 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>>> Robert Haas <robertmhaas(a)gmail.com> writes: >>>>> ... Â Also, I prefer an >>>>> API where the escaping function does include the quotes, so I've done >>>>> it that way in the attached patch. >>>> >>>> IMO this function should act as much like PQescapeStringConn as possible. >>> >>> Generally speaking, I agree... >>> >>>> Random differences like including or not including outer quotes don't >>>> make the user's life better. Â Random differences like a slightly >>>> different rule for the amount of space required are outright dangerous. >>> >>> I'm not sure that not including the quotes is any better. Â If someone >>> escapes foo and gets back foo, are they going to realize that escaping >>> fo"o is going to give them back fo""o rather than "fo""o"? Â One >>> difference vs. PQescapeStringConn() is that if you fail to include the >>> surrounding quotes in that case, something will almost certainly break >>> in a noisy and highly visible fashion. Â Here that might not happen, or >>> someone might call one of PQescapeStringConn() and >>> PQescapeIdentifierConn() and then use the wrong sort of outer quotes. >>> >>> IMO, it's actually pretty weird that PQescapeStringConn() and >>> quote_literal() are named differently and do incompatible things. Â I >>> think it would be a plus if this new function were a little more >>> similar to quote_ident(), but that's just MHO, of course. >>> >> >> I am afraid so we can do nothing now with this. There are two >> arguments - consistency versus robustness. If you use >> PQescapeStringConn() without outer quotes, then you have a SQL >> injection problem (there could not be error) :(. When there are no >> escape function that add outer quotes, then can be strange for >> developers working with one different. >> >> I see three solution: >> >> a) use a PQescapeIdentifConn as PQescapeStringConn, >> b) move this functionality to psql without change of API, >> c) change semantic and name - maybe PQquoteIdentifierConn() >> >> Personally I am for a) and later for b). What I know - php coders >> needs some secure function for identifier escaping - but I dislike PHP >> because every function is designed different. > > I think what you're saying is that you agree with Tom's position that > the new escaping function should not add the necessary surrounding > quotes, instead leaving that to the user. Â Is that correct? yes Pavel > > ...Robert > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 19 Jan 2010 13:32 On Tue, Jan 19, 2010 at 12:54 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >> I think what you're saying is that you agree with Tom's position that >> the new escaping function should not add the necessary surrounding >> quotes, instead leaving that to the user. Is that correct? > > yes Updated patch attached. I still think this is a bizarre API. ....Robert
From: Tom Lane on 19 Jan 2010 13:43 Robert Haas <robertmhaas(a)gmail.com> writes: > Updated patch attached. I still think this is a bizarre API. Well, if we had it to do over I'm sure we'd have done it differently, but the functions are there now and we aren't going to change them ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 19 Jan 2010 13:46 On Tue, Jan 19, 2010 at 1:43 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas(a)gmail.com> writes: >> Updated patch attached. I still think this is a bizarre API. > > Well, if we had it to do over I'm sure we'd have done it differently, > but the functions are there now and we aren't going to change them ... I agree, but I don't feel the need to repeat the mistakes we've already made by slavishly copying them into new places. I think as long as we're adding a new function, we should make it behave sanely. We could even take the opportunity to go back and add a saner version of PQescapeStringConn. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 19 Jan 2010 14:01
Robert Haas <robertmhaas(a)gmail.com> writes: > ... I think as > long as we're adding a new function, we should make it behave sanely. > We could even take the opportunity to go back and add a saner version > of PQescapeStringConn. Well, it's a bit late in the devel cycle to be inventing from scratch, but if we did want to do something "saner" what would it look like? I can see the following possibilities: * include boundary quotes (and E too in the literal case). This would imply telling people they should leave whitespace around the value in the constructed query ... or should we insert leading/trailing spaces to prevent that mistake too? * malloc our own result value instead of expecting caller to provide space. This adds another failure case (out of memory) but it's not really much different from OOM on the caller side. * anything else you want to change? I suggest we could use PQescapeLiteral + PQescapeIdentifier as names if we provide a new pair of functions defined with a different API. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |