Re: 7.2 fe-exec.c patch to PQescapeString()

Bruce Momjian wrote:

> I am not sure about this patch. If they pass NULL as string pointers,
> but a positive length, I think we should crash rather than assuming
> everything is OK. The code already works OK for length = 0. In fact,
> the patch makes length=0 do nothing, rather than having it execute this
> line:
>
> /* Write the terminating NUL character. */
> *target = '\0';

Good catch on the length = 0 case. If length is 0 we should still assume
'to' satisfies (2 * length + 1) and allow the target write.

I still think it ought not to crash on null 'from' string with a positive
length if there is sufficient definition of the function to expect the caller
to recover. Absence of a crash does not imply everything is OK, just as the
absence of a backend crash on a malformed query does not imply the query
succeeded. It is the caller's responsibility (and privilege) to check the
return value. If they don't do that, they get what they deserve. But if you
crash, you take that ability away from the caller and nuke possibilities for
error handling and recovery. In the case of length > 0 and 'from' == NULL,
returning 0 rather than crashing gives the caller a chance to check the input
length against the return. So I would change the patch to:

if ( length > 0 && ! from ) { return 0; }

Ed

>
> ---------------------------------------------------------------------------
>
> Ed Loehr wrote:
>
>>This patch makes PQescapeString() guard against null input
>>
>>ptrs and/or length == 0 input. If any of these occur, the
>>
>>function returns 0.
>>
>>
>
>>*** fe-exec.c.orig Thu Apr 4 16:06:38 2002
>>--- fe-exec.c Thu Apr 4 16:07:30 2002
>>***************
>>*** 75,76 ****
>>--- 75,80 ----
>>
>>+ if ( ! to || ! from || ! length ) {
>>+ return 0;
>>+ }
>>+
>> while (remaining > 0)
>>
>
>>---------------------------(end of broadcast)---------------------------
>>TIP 5: Have you checked our extensive FAQ?
>>
>>http://www.postgresql.org/users-lounge/docs/faq.html
>>
>