[HACKERS] PQhost may return socket dir for network connection - Mailing list pgsql-hackers

From Kyotaro HORIGUCHI
Subject [HACKERS] PQhost may return socket dir for network connection
Date
Msg-id 20170428.164334.152924301.horiguchi.kyotaro@lab.ntt.co.jp
Whole thread Raw
Responses Re: [HACKERS] PQhost may return socket dir for network connection  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Hello.

As the subject, PQhost() seems to be forgeting about the case
where only hostaddr is specified in a connection string.

PQhost() returns /var/run/postgresql or such for a connection
made from the connection string "hostaddr=127.0.0.1 port=5432
dbname=postgres". I don't believe this behavior is useful.

The first attached file is a fix for master. connhost[]->pghost
is properly set by connectOptions2() so we don't need to handle
the default case in PQhost.

The second is a fix for 9.6. connectOption2 was setting
pgunixsocket then clearing pghost reagardless of the value of
pghostaddr. This behavior inhibited PQhost() from getting the
right name to return.

The behavior for several connection strings after this fix are
the follows. The first line is the case fixed by this patch.

1. PQhost = 127.0.0.1 <= "hostaddr=127.0.0.1 port=5432 dbname=postgres"
2. PQhost = /tmp <= "port=5432 dbname=postgres"
3. PQhost = /tmp <= "host=/tmp port=5432 dbname=postgres"
4. PQhost = /tmp <= "host=/tmp hostaddr=127.0.0.1 port=5432 dbname=postgres"
5. PQhost = localhost <= "host=localhost port=5432 dbname=postgres"
6. PQhost = localhost <= "host=localhost hostaddr=127.0.0.1 port=5432 dbname=postgres"
7. PQhost = hoge <= "host=hoge hostaddr=127.0.0.1 port=5432 dbname=postgres"

The third case is somewhat confusing but it would be the user's
fault.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 5836,5854 **** PQhost(const PGconn *conn) {     if (!conn)         return NULL;
!     if (conn->connhost != NULL &&
!         conn->connhost[conn->whichhost].type != CHT_HOST_ADDRESS)
!         return conn->connhost[conn->whichhost].host;
!     else if (conn->pghost != NULL && conn->pghost[0] != '\0')         return conn->pghost;
!     else
!     {
! #ifdef HAVE_UNIX_SOCKETS
!         return DEFAULT_PGSOCKET_DIR;
! #else
!         return DefaultHost;
! #endif
!     } }  char *
--- 5836,5857 ---- {     if (!conn)         return NULL;
! 
!     /*
!      * We should try to avoid returning ip address. connhost[]->host stores IP
!      * address in the case of CHT_HOST_ADDRESS so try to use conn->pghost
!      * instead.
!      */
!     if (conn->connhost == NULL ||
!         (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS &&
!          conn->pghost != NULL && conn->pghost[0] != '\0'))         return conn->pghost;
! 
!     /*
!      * Otherwise we use this as host name even though it might be an IP
!      * address.
!      */
!     return conn->connhost[conn->whichhost].host; }  char *
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 819,827 **** connectOptions2(PGconn *conn)     }      /*
!      * Allow unix socket specification in the host name      */
!     if (conn->pghost && is_absolute_path(conn->pghost))     {         if (conn->pgunixsocket)
free(conn->pgunixsocket);
--- 819,828 ----     }      /*
!      * Allow unix socket specification in the host name if hostaddr is not set      */
!     if ((conn->pghostaddr == NULL || conn->pghostaddr[0] == '\0') &&
!         conn->pghost && is_absolute_path(conn->pghost))     {         if (conn->pgunixsocket)
free(conn->pgunixsocket);
***************
*** 5354,5370 **** PQhost(const PGconn *conn)         return NULL;     if (conn->pghost != NULL && conn->pghost[0] !=
'\0')        return conn->pghost;
 
!     else
!     { #ifdef HAVE_UNIX_SOCKETS
!         if (conn->pgunixsocket != NULL && conn->pgunixsocket[0] != '\0')
!             return conn->pgunixsocket;
!         else
!             return DEFAULT_PGSOCKET_DIR; #else
!         return DefaultHost; #endif
-     } }  char *
--- 5355,5369 ----         return NULL;     if (conn->pghost != NULL && conn->pghost[0] != '\0')         return
conn->pghost;
!     if (conn->pghostaddr != NULL && conn->pghostaddr[0] != '\0')
!         return conn->pghostaddr; #ifdef HAVE_UNIX_SOCKETS
!     if (conn->pgunixsocket != NULL && conn->pgunixsocket[0] != '\0')
!         return conn->pgunixsocket;
!     return DEFAULT_PGSOCKET_DIR; #else
!     return DefaultHost; #endif }  char *

pgsql-hackers by date:

Previous
From: Antonin Houska
Date:
Subject: Re: [HACKERS] Partition-wise aggregation/grouping
Next
From: Kyotaro HORIGUCHI
Date:
Subject: [HACKERS] .pgpass's behavior has changed