Re: PQescapeString handling of \0

Hi.

Here's a patch which makes PQescapeString stop escaping when \0 is seen.
Currently. if somebody passes a string with embedded \0 (for example, perl
allows \0s in it's scalars and DBD::Pg does no checks), PQescapeString will
return an invalid (bigger) length for \0-terminated string.

--
Best regards,
Igor Shevchenko

Igor Shevchenko <igor(at)carcass(dot)ath(dot)cx> writes:
> Here's a patch which makes PQescapeString stop escaping when \0 is seen.

Hm. Is that really the right behavior? Given that the function is
defined to take a counted string rather than a null-terminated string,
I'd sort of expect it to turn \0 into "\000" or some such. Of course,
given that this could overflow the specified size of the return buffer,
one might simply conclude that the defined API is broken ...

regards, tom lane

If the purpose of PQescapeString is to make a safe variant of a given string
(NULL-terminated or counted), where safity means ability to simply add it to
the query (which is char* and is passed directly to PQexec), then either
length should return what strlen would return for the given string or \0
should be escaped somehow.

On Monday 29 September 2003 21:22, Tom Lane wrote:
> Igor Shevchenko <igor(at)carcass(dot)ath(dot)cx> writes:
> > Here's a patch which makes PQescapeString stop escaping when \0 is seen.
>
> Hm. Is that really the right behavior? Given that the function is
> defined to take a counted string rather than a null-terminated string,
> I'd sort of expect it to turn \0 into "\000" or some such. Of course,
> given that this could overflow the specified size of the return buffer,
> one might simply conclude that the defined API is broken ...
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html

--
Best regards,
Igor Shevchenko

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Tom Lane wrote:
> Hm. Is that really the right behavior? Given that the function is
> defined to take a counted string rather than a null-terminated string,
> I'd sort of expect it to turn \0 into "\000" or some such. Of course,
> given that this could overflow the specified size of the return buffer,
> one might simply conclude that the defined API is broken ...

Igor Shevchenko replied:
> If the purpose of PQescapeString is to make a safe variant of a given string
> (NULL-terminated or counted), where safity means ability to simply add it to
> the query (which is char* and is passed directly to PQexec), then either
> length should return what strlen would return for the given string or \0
> should be escaped somehow.

Has we reached a consensus on this? I'd like to have DBD::Pg and PQescapeString
consistent. Right now, we (DBD::Pg) are leaning toward the "bail if we hit \0"
theory, but it would be nice to have something definitive in the API.

- --
Greg Sabino Mullane greg(at)turnstep(dot)com
PGP Key: 0x14964AC8 200310030936

-----BEGIN PGP SIGNATURE-----
Comment: http://www.turnstep.com/pgp.html

iD8DBQE/fXvlvJuQZxSWSsgRAsc9AKDbn7QdDCk0lGDVP17249I0VbN9ygCg+BH4
UJxIs5PakXA0Mkth0td0jHw=
=KMEd
-----END PGP SIGNATURE-----

greg(at)turnstep(dot)com writes:
> Tom Lane wrote:
>>> Hm. Is that really the right behavior?

> Has we reached a consensus on this? I'd like to have DBD::Pg and
> PQescapeString consistent. Right now, we (DBD::Pg) are leaning toward
> the "bail if we hit \0" theory, but it would be nice to have something
> definitive in the API.

Ugly as it is, I think that we have little choice but to go with the
behavior Igor proposes (stop at \0). If we do the other then we will
have to raise the required size of the output buffer, which will
silently break existing apps (possibly creating security holes, even).
So essentially PQescapeString is going to become like strncpy --- stop
at either \0 or the given count.

This patch hasn't been applied as of 7.4b4 but I'll try to get it in
shortly.

regards, tom lane

On Fri, Oct 03, 2003 at 11:31:02AM -0400, Tom Lane wrote:
>
> So essentially PQescapeString is going to become like strncpy --- stop
> at either \0 or the given count.

I've no idea whether this makes sense, but as a matter of style, shouldn't
strings containing '\0's be escaped as binary strings anyway? And if we're
going to see multibyte encodings growing in popularity (lots of zeroes and
"high" byte values), perhaps PQescapeString should even be deprecated?

Jeroen

"Jeroen T. Vermeulen" <jtv(at)xs4all(dot)nl> writes:
> On Fri, Oct 03, 2003 at 11:31:02AM -0400, Tom Lane wrote:
>> So essentially PQescapeString is going to become like strncpy --- stop
>> at either \0 or the given count.

> I've no idea whether this makes sense, but as a matter of style, shouldn't
> strings containing '\0's be escaped as binary strings anyway? And if we're
> going to see multibyte encodings growing in popularity (lots of zeroes and
> "high" byte values), perhaps PQescapeString should even be deprecated?

I think you might be confusing PQescapeString (intended for text) with
PQescapeBytea (intended for binary data). We do not support any text
encodings that require embedded zeroes, and are unlikely to do so
anytime soon. (Had anyone suggested this as a design goal when we were
doing the recent FE/BE protocol redesign, perhaps the protocol wouldn't
still rely on null-terminated strings. But no one did and it's too late
now.)

regards, tom lane

On Fri, Oct 03, 2003 at 12:02:03PM -0400, Tom Lane wrote:
>
> I think you might be confusing PQescapeString (intended for text) with
> PQescapeBytea (intended for binary data). We do not support any text

I'm certain that I'm confusing those two--what I don't know is whether
that's necessarily a bad thing. What exactly constitutes a binary
string? Do strings of text with some weird bytes in them fit the
definition?

Jeroen

Igor Shevchenko <igor(at)carcass(dot)ath(dot)cx> writes:
> Here's a patch which makes PQescapeString stop escaping when \0 is seen.

I've applied this patch plus some further code and documentation
cleanup.

regards, tom lane