quoting psql varible as identifier
in [PgSql]
|
Prev: Port for Microsoft Services for Unix (SFU) or SUA
Next: [HACKERS] solution to make static changes in pg_hba.conf file?
From: Robert Haas on 18 Jan 2010 15:19 On Mon, Jan 18, 2010 at 2:20 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 2010/1/18 Robert Haas <robertmhaas(a)gmail.com>: >> On Mon, Jan 18, 2010 at 1:52 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >>> 2010/1/18 Robert Haas <robertmhaas(a)gmail.com>: >>>> On Sun, Jan 17, 2010 at 2:04 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >>>>> I rewrote patch so now interface for PQescapeIdentConn is same as >>>>> PQescapeStringConn >>>>> >>>>> @3. I though so the protection under incomplete multibyte chars are >>>>> enought - missing bytes are replaced by space - like >>>>> PQescapeStringConn does. >>>> >>>> That much is fine, but the output buffer is only guaranteed to be of >>>> size 2n+1. Imagine the input is two double-quotes followed by a byte >>>> for which pg_encoding_mblen() returns 4. The input is 3 characters >>>> long so the user was responsible to provide 7 bytes of output space, >>>> but you'll try to write 9 bytes to it (including the terminating NUL). >>>> >>> I don't understand. The "length" is number of bytes, not number of >>> chars. It is maybe bad documented only. If your input string has 6 >>> bytes, then buffer have to allocated to 13 bytes. Nobody knows how >>> much is chars there. >> >> Right, but the point is we can't assume that the input is validly >> encoded. If the input ends with a garbage character that looks like >> the start of a multi-byte character, we can't assume that there's >> enough space in the output buffer to store the required number of >> padding spaces. >> >> To take an extreme example, suppose there were an encoding where any >> time the first byte of a multi-byte character has the high-bit set, >> the character is 100 bytes long. Then suppose someone call >> PQescapeStringConn(), or this new function we're adding, with a length >> argument of 1, and the first byte of the input buffer has the high-bit >> set. The caller is only required to provide a 3-byte output buffer, >> and the third byte is needed for the terminating NUL. That means that >> after we copy that first character we only have room to insert one >> padding space. The way you had it coded, since we were expecting a >> character 100 bytes long, we'd always try to insert 99 padding spaces. >> > > do you speak about previous version? Yes. > in current version is garanted new length is <= 2x original length Actually, strictly less than, but the code gets it correct. However, your latest version has some other problems. For example, you didn't update the docs to match your source-code changes. Also, I prefer an API where the escaping function does include the quotes, so I've done it that way in the attached patch. This is just the libpq changes, I figure if we can agree on this, then we can move onto the psql stuff. Comments? ....Robert
From: Tom Lane on 18 Jan 2010 15:26 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. 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. Also, why is this patch changing the documentation of PQescapeStringConn? It might be only whitespace changes, but I don't particularly wish to have to determine that. 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 18 Jan 2010 16:01 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. > Also, why is this patch changing the documentation of PQescapeStringConn? > It might be only whitespace changes, but I don't particularly wish to > have to determine that. See previous discussion upthread. http://archives.postgresql.org/pgsql-hackers/2010-01/msg01516.php ....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: Pavel Stehule on 19 Jan 2010 04:13 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. regards Pavel >> Also, why is this patch changing the documentation of PQescapeStringConn? >> It might be only whitespace changes, but I don't particularly wish to >> have to determine that. > > See previous discussion upthread. > > http://archives.postgresql.org/pgsql-hackers/2010-01/msg01516.php > > ...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 12:48
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? ....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 |