Tweak libpq's PQhost, PQhostaddr, and psql's \connect

Fixes some problems introduced by 6e5f8d489acc:

* When reusing conninfo data from the previous connection in \connect,
  the host address should only be reused if it was specified as
  hostaddr; if it wasn't, then 'host' is resolved afresh.  We were
  reusing the same IP address, which ignores a possible DNS change
  as well as any other addresses that the name resolves to than the
  one that was used in the original connection.

* PQhost, PQhostaddr: Don't present user-specified hostaddr when we have
  an inet_net_ntop-produced equivalent address.  The latter has been
  put in canonical format, which is cleaner (so it produces "127.0.0.1"
  when given "host=2130706433", for example).

* Document the hostaddr-reusing aspect of \connect.

* Fix some code comments

Author: Fabien Coelho
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20190527203713.GA58392@gust.leadboat.com

2 files changed, 40 insertions, 21 deletions

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 695d6ba9f14..6263f8a7798 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2870,6 +2870,26 @@ param_is_newly_set(const char *old_val, const char *new_val)
 	return false;
 }
 
+/* return whether the connection has 'hostaddr' in its conninfo */
+static bool
+has_hostaddr(PGconn *conn)
+{
+	bool		used = false;
+	PQconninfoOption *ciopt = PQconninfo(conn);
+
+	for (PQconninfoOption *p = ciopt; p->keyword != NULL; p++)
+	{
+		if (strcmp(p->keyword, "hostaddr") == 0 && p->val != NULL)
+		{
+			used = true;
+			break;
+		}
+	}
+
+	PQconninfoFree(ciopt);
+	return used;
+}
+
 /*
  * do_connect -- handler for \connect
  *
@@ -2929,24 +2949,24 @@ do_connect(enum trivalue reuse_previous_specification,
 		port = NULL;
 	}
 
-	/* grab missing values from the old connection */
+	/*
+	 * Grab missing values from the old connection.  If we grab host (or host
+	 * is the same as before) and hostaddr was set, grab that too.
+	 */
 	if (reuse_previous)
 	{
 		if (!user)
 			user = PQuser(o_conn);
-		if (host && strcmp(host, PQhost(o_conn)) == 0)
+		if (host && strcmp(host, PQhost(o_conn)) == 0 &&
+			has_hostaddr(o_conn))
 		{
-			/*
-			 * if we are targeting the same host, reuse its hostaddr for
-			 * consistency
-			 */
 			hostaddr = PQhostaddr(o_conn);
 		}
 		if (!host)
 		{
 			host = PQhost(o_conn);
-			/* also set hostaddr for consistency */
-			hostaddr = PQhostaddr(o_conn);
+			if (has_hostaddr(o_conn))
+				hostaddr = PQhostaddr(o_conn);
 		}
 		if (!port)
 			port = PQport(o_conn);
@@ -3129,7 +3149,10 @@ do_connect(enum trivalue reuse_previous_specification,
 			char	   *host = PQhost(pset.db);
 			char	   *hostaddr = PQhostaddr(pset.db);
 
-			/* If the host is an absolute path, the connection is via socket */
+			/*
+			 * If the host is an absolute path, the connection is via socket
+			 * unless overridden by hostaddr
+			 */
 			if (is_absolute_path(host))
 			{
 				if (hostaddr && *hostaddr)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e58fa6742a4..c800d7921e3 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1536,9 +1536,7 @@ getHostaddr(PGconn *conn, char *host_addr, int host_addr_len)
 {
 	struct sockaddr_storage *addr = &conn->raddr.addr;
 
-	if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
-		strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, host_addr_len);
-	else if (addr->ss_family == AF_INET)
+	if (addr->ss_family == AF_INET)
 	{
 		if (inet_net_ntop(AF_INET,
 						  &((struct sockaddr_in *) addr)->sin_addr.s_addr,
@@ -6463,6 +6461,10 @@ PQhost(const PGconn *conn)
 
 	if (conn->connhost != NULL)
 	{
+		/*
+		 * Return the verbatim host value provided by user, or hostaddr in its
+		 * lack.
+		 */
 		if (conn->connhost[conn->whichhost].host != NULL &&
 			conn->connhost[conn->whichhost].host[0] != '\0')
 			return conn->connhost[conn->whichhost].host;
@@ -6480,15 +6482,9 @@ PQhostaddr(const PGconn *conn)
 	if (!conn)
 		return NULL;
 
-	if (conn->connhost != NULL)
-	{
-		if (conn->connhost[conn->whichhost].hostaddr != NULL &&
-			conn->connhost[conn->whichhost].hostaddr[0] != '\0')
-			return conn->connhost[conn->whichhost].hostaddr;
-
-		if (conn->connip != NULL)
-			return conn->connip;
-	}
+	/* Return the parsed IP address */
+	if (conn->connhost != NULL && conn->connip != NULL)
+		return conn->connip;
 
 	return "";
 }