Обсуждение: pgsql: Fix parsing of integer values for connection parameters in libpq

Fix parsing of integer values for connection parameters in libpq

Commit e7a2217 has introduced stricter checks for integer values in
connection parameters for libpq.  However this failed to correctly check
after trailing whitespaces, while leading whitespaces were discarded per
the use of strtol(3).  This fixes and refactors the parsing logic to
handle both cases consistently.  Note that trying to restrict the use of
trailing whitespaces can easily break connection strings like in ECPG
regression tests (these have allowed me to catch the parsing bug with
connect_timeout).

Author: Michael Paquier
Reviewed-by: Lars Kanis
Discussion: https://postgr.es/m/a9b4cbd7-4ecb-06b2-ebd7-1739bbff3217@greiz-reinsdorf.de
Backpatch-through: 12

Branch
------
REL_12_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/2b0f959b5119cb2bb1d135ac04a8c5272bbcab03

Modified Files
--------------
src/interfaces/libpq/fe-connect.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)

On 2019-10-21 04:18, Michael Paquier wrote:
> Fix parsing of integer values for connection parameters in libpq

Something in this code doesn't make sense:

+   /*
+    * Skip any trailing whitespace; if anything but whitespace remains
before
+    * the terminating character, fail
+    */
+   while (*end && *end != '\0' && isspace((unsigned char) *end))
+       end++;
+
+   if (*end && *end != '\0')
+       goto error;

You probably want something like

    if (end && *end != ...)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Oct 22, 2019 at 10:05:45AM +0200, Peter Eisentraut wrote:
> You probably want something like
>
>     if (end && *end != ...)

Yes, it looks like a brain fade here.  Conversion failures are tracked
before that, so strtol() would not return NULL for endptr.  The first
part could just be removed as per the attached.
--
Michael

On Tue, Oct 22, 2019 at 07:05:53PM +0900, Michael Paquier wrote:
> Yes, it looks like a brain fade here.  Conversion failures are tracked
> before that, so strtol() would not return NULL for endptr.  The first
> part could just be removed as per the attached.

I looked at that with a fresher mind, and fixed it, with an extra
assertion making sure that we never call it with a NULL input.  All
callers of parse_int_param are careful to not call the routine with a
NULL input, but future callers may not be that careful...
--
Michael