libpq stricter integer parsing

[Fabien COELHO]

Follow up on a patch and discussion with Tom, currently integer parsing on 
keywords in libpq is quite loose, resulting in trailing garbage being 
ignored and allowing to hide bugs, eg:

  sh> psql "connect_timeout=2,port=5433"

The timeout is set to 2, and the port directive is silently ignored.
However, URL parsing is stricter, eg on "port".

The attached patch checks integer syntax errors and overflows, and report 
errors.

The pros is that it helps detect bugs. The cons is that some people may 
not want to know about these if it works in the end.

--
Fabien.diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 49de975e7f..94d114562a 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1579,6 +1579,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
     </varlistentry>
     </variablelist>
    </para>
+
+   <para>
+    Integer values expected for keywords <literal>port</literal>,
+    <literal>connect_timeout</literal>,
+    <literal>keepalives_idle</literal>,
+    <literal>keepalives_interval</literal> and
+    <literal>keepalives_timeout</literal> are parsed strictly.
+    Versions of <application>libpq</application> before
+    <product>PostgreSQL 12</product> accepted trailing garbage or overflows.
+   </para>
   </sect2>
  </sect1>
 
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 9315a27561..15280dc208 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1586,6 +1586,35 @@ useKeepalives(PGconn *conn)
 	return val != 0 ? 1 : 0;
 }
 
+/*
+ * Parse integer, report any syntax error, and tell if all is well.
+ */
+static bool
+strict_atoi(const char *str, int *val, PGconn *conn, const char *context)
+{
+	char 	*endptr;
+
+	errno = 0;
+	*val = strtol(str, &endptr, 10);
+
+	if (errno != 0)
+	{
+		appendPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("invalid integer \"%s\" for keyword \"%s\": %s\n"),
+						  str, context, strerror(errno));
+		return false;
+	}
+	else if (*endptr != '\0')
+	{
+		appendPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("invalid integer \"%s\" for keyword \"%s\"\n"),
+						  str, context);
+		return false;
+	}
+
+	return true;
+}
+
 #ifndef WIN32
 /*
  * Set the keepalive idle timer.
@@ -1598,7 +1627,9 @@ setKeepalivesIdle(PGconn *conn)
 	if (conn->keepalives_idle == NULL)
 		return 1;
 
-	idle = atoi(conn->keepalives_idle);
+	if (!strict_atoi(conn->keepalives_idle, &idle, conn, "keepalives_idle"))
+		return 0;
+
 	if (idle < 0)
 		idle = 0;
 
@@ -1630,7 +1661,9 @@ setKeepalivesInterval(PGconn *conn)
 	if (conn->keepalives_interval == NULL)
 		return 1;
 
-	interval = atoi(conn->keepalives_interval);
+	if (!strict_atoi(conn->keepalives_interval, &interval, conn, "keepalives_interval"))
+		return 0;
+
 	if (interval < 0)
 		interval = 0;
 
@@ -1663,7 +1696,9 @@ setKeepalivesCount(PGconn *conn)
 	if (conn->keepalives_count == NULL)
 		return 1;
 
-	count = atoi(conn->keepalives_count);
+	if (!strict_atoi(conn->keepalives_count, &count, conn, "keepalives_count"))
+		return 0;
+
 	if (count < 0)
 		count = 0;
 
@@ -1697,13 +1732,17 @@ setKeepalivesWin32(PGconn *conn)
 	int			idle = 0;
 	int			interval = 0;
 
-	if (conn->keepalives_idle)
-		idle = atoi(conn->keepalives_idle);
+	if (conn->keepalives_idle &&
+		!strict_atoi(conn->keepalives_idle, &idle, conn, "keepalives_idle"))
+		return 0;
+
+	if (conn->keepalives_interval &&
+		!strict_atoi(conn->keepalives_interval, &interval, conn, "keepalives_interval"))
+		return 0;
+
 	if (idle <= 0)
 		idle = 2 * 60 * 60;		/* 2 hours = default */
 
-	if (conn->keepalives_interval)
-		interval = atoi(conn->keepalives_interval);
 	if (interval <= 0)
 		interval = 1;			/* 1 second = default */
 
@@ -1784,7 +1823,9 @@ connectDBStart(PGconn *conn)
 			thisport = DEF_PGPORT;
 		else
 		{
-			thisport = atoi(ch->port);
+			if (!strict_atoi(ch->port, &thisport, conn, "port"))
+				goto connect_errReturn;
+
 			if (thisport < 1 || thisport > 65535)
 			{
 				appendPQExpBuffer(&conn->errorMessage,
@@ -1916,7 +1957,9 @@ connectDBComplete(PGconn *conn)
 	 */
 	if (conn->connect_timeout != NULL)
 	{
-		timeout = atoi(conn->connect_timeout);
+		if (!strict_atoi(conn->connect_timeout, &timeout, conn, "connect_timeout"))
+			return 0;
+
 		if (timeout > 0)
 		{
 			/*
@@ -1927,6 +1970,8 @@ connectDBComplete(PGconn *conn)
 			if (timeout < 2)
 				timeout = 2;
 		}
+		else /* negative means 0 */
+			timeout = 0;
 	}
 
 	for (;;)