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

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b86764003d3d1ca595500df24d84eb85f3cac49a..c6c20de24347237d7e043ced0579509c7bd1e3ee 100644 (file)
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -911,6 +911,9 @@ testdb=>
         <replaceable class="parameter">host</replaceable> or
         <replaceable class="parameter">port</replaceable>
         as <literal>-</literal> is equivalent to omitting that parameter.
+        If <literal>hostaddr</literal> was specified in the original
+        connection's <structname>conninfo</structname>, that address is reused
+        for the new connection (disregarding any other host specification).
         </para>
 
         <para>

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 695d6ba9f14891f5f2690b6651d7f4b1176c71e9..6263f8a77987a65f32b35d7551e8ae0dc8691c00 100644 (file)
--- 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 e58fa6742a4dcdd88f6635872ecc74ed51594345..c800d7921e318b1fb82d4237a02f802cad052c8c 100644 (file)
--- 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 "";
 }