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

Bruce Momjian wrote:

> Our general libpq behavior is not to mask programming errors in the
> library, i.e. masking a clearly invalid NULL pointer. What if the
> caller doesn't check the returned length?

They get occasional unexpected results? Your point is well-taken

regarding masking programming errors, but guarding against a null

ptr crash is not 'masking the error' when the function interface

provides a means to communicate the error to the caller as in this

case. Even if it didn't, I still have no affection for low-level

functions that decide to abort the entire system rather than giving

their callers a chance for error-handling.

Ed

> Ed Loehr wrote:
>
>>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
>>>>
>>>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 6: Have you searched our list archives?
>>
>>http://archives.postgresql.org
>>
>>
>