Re: 7.2 fe-exec.c patch to PQescapeString() - Mailing list pgsql-patches

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
>>
>>
>