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

From Bruce Momjian
Subject Re: 7.2 fe-exec.c patch to PQescapeString()
Date
Msg-id 200204060215.g362F0008892@candle.pha.pa.us
Whole thread Raw
In response to Re: 7.2 fe-exec.c patch to PQescapeString()  (Ed Loehr <pgpatches@bluepolka.net>)
List pgsql-patches
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?

---------------------------------------------------------------------------

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
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

pgsql-patches by date:

Previous
From: Ed Loehr
Date:
Subject: Re: 7.2 fe-exec.c patch to PQescapeString()
Next
From: Bruce Momjian
Date:
Subject: Re: new food for the contrib/ directory