Received: from malur.postgresql.org ([2a02:16a8:dc51::56])
	by arkaria.postgresql.org with esmtps
 (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1fkc44-0007UH-BP
	for pgsql-hackers@arkaria.postgresql.org; Tue, 31 Jul 2018 21:20:20 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1fkc42-0007MF-Sj
	for pgsql-hackers@arkaria.postgresql.org; Tue, 31 Jul 2018 21:20:18 +0000
Received: from makus.postgresql.org ([2001:4800:1501:1::229])
	by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1fkc42-0007Lw-Cn
	for pgsql-hackers@lists.postgresql.org; Tue, 31 Jul 2018 21:20:18 +0000
Received: from sss.pgh.pa.us ([66.207.139.130])
	by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1fkc3z-00015Z-9i
	for pgsql-hackers@lists.postgresql.org; Tue, 31 Jul 2018 21:20:17 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id w6VLKAqu010440;
	Tue, 31 Jul 2018 17:20:10 -0400
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Bizarre behavior in libpq's searching of ~/.pgpass
In-reply-to: <11455.1533047501@sss.pgh.pa.us>
References: <30805.1532749137@sss.pgh.pa.us>
 <CA+TgmoYwNjrapiDo6BEpMCEVc0GxNSUKd8hObsUZJLEsVnpUqA@mail.gmail.com>
 <11455.1533047501@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 31 Jul 2018 10:31:41 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <10405.1533071971.0@sss.pgh.pa.us>
Date: Tue, 31 Jul 2018 17:20:10 -0400
Message-ID: <10439.1533072010@sss.pgh.pa.us>
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <10405.1533071971.1@sss.pgh.pa.us>

I wrote:
> But now that I look at it, it seems like the code in connectOptions2
> has also Gotten It Wrong.  Shouldn't the replacement of "unspecified"
> cases by DEFAULT_PGSOCKET_DIR/DefaultHost also happen on an entry-by-
> entry basis, so that "host=foo," would behave as though the empty
> entry were "localhost"?

Here's an updated patch that fixes that aspect too.  Although this
might seem independent, it's not really: this version of the patch
eliminates the corner case where we have neither a host or hostaddr
to search for.  The check for empty hostname in passwordFromFile is
thereby dead code, though it seemed better style to leave it in.

(Basically, the point here is to guarantee that passwordFromFile has
the same idea of what host/port we are going to connect to as the
actual connection code does.  That was not true before, either for
the host or the port :-()

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="search-pgpass-with-host-or-hostaddr-2.patch";
	charset="us-ascii"
Content-ID: <10405.1533071971.2@sss.pgh.pa.us>
Content-Description: search-pgpass-with-host-or-hostaddr-2.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d67212b..0b1ffb7 100644
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 938,945 ****
       <para>
         If a password file is used, you can have different passwords for
         different hosts. All the other connection options are the same fo=
r every
!        host, it is not possible to e.g. specify a different username for
!        different hosts.
       </para>
     </sect3>
    </sect2>
--- 938,945 ----
       <para>
         If a password file is used, you can have different passwords for
         different hosts. All the other connection options are the same fo=
r every
!        host in the list; it is not possible to e.g. specify different
!        usernames for different hosts.
       </para>
     </sect3>
    </sect2>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 961,967 ****
          name of the directory in which the socket file is stored.  If
          multiple host names are specified, each will be tried in turn in
          the order given.  The default behavior when <literal>host</liter=
al> is
!         not specified is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexter=
m> in
          <filename>/tmp</filename> (or whatever socket directory was spec=
ified
          when <productname>PostgreSQL</productname> was built). On machin=
es without
--- 961,967 ----
          name of the directory in which the socket file is stored.  If
          multiple host names are specified, each will be tried in turn in
          the order given.  The default behavior when <literal>host</liter=
al> is
!         not specified, or is empty, is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexter=
m> in
          <filename>/tmp</filename> (or whatever socket directory was spec=
ified
          when <productname>PostgreSQL</productname> was built). On machin=
es without
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 969,975 ****
         </para>
         <para>
          A comma-separated list of host names is also accepted, in which =
case
!         each host name in the list is tried in order. See
          <xref linkend=3D"libpq-multiple-hosts"/> for details.
         </para>
        </listitem>
--- 969,976 ----
         </para>
         <para>
          A comma-separated list of host names is also accepted, in which =
case
!         each host name in the list is tried in order; an empty item in t=
he
!         list selects the default behavior as explained above. See
          <xref linkend=3D"libpq-multiple-hosts"/> for details.
         </para>
        </listitem>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1020,1033 ****
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</lit=
eral>
          is not the name of the server at network address <literal>hostad=
dr</literal>.
!         Also, note that <literal>host</literal> rather than <literal>hos=
taddr</literal>
          is used to identify the connection in a password file (see
          <xref linkend=3D"libpq-pgpass"/>).
         </para>
  =

         <para>
          A comma-separated list of <literal>hostaddr</literal> values is =
also
!         accepted, in which case each host in the list is tried in order.=
 See
          <xref linkend=3D"libpq-multiple-hosts"/> for details.
         </para>
         <para>
--- 1021,1037 ----
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</lit=
eral>
          is not the name of the server at network address <literal>hostad=
dr</literal>.
!         Also, when both <literal>host</literal> and <literal>hostaddr</l=
iteral>
!         are specified, <literal>host</literal>
          is used to identify the connection in a password file (see
          <xref linkend=3D"libpq-pgpass"/>).
         </para>
  =

         <para>
          A comma-separated list of <literal>hostaddr</literal> values is =
also
!         accepted, in which case each host in the list is tried in order.
!         An empty item in the list causes the corresponding host name to =
be
!         used, or the default host name if that is empty as well. See
          <xref linkend=3D"libpq-multiple-hosts"/> for details.
         </para>
         <para>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1047,1055 ****
          name extension for Unix-domain
          connections.<indexterm><primary>port</primary></indexterm>
          If multiple hosts were given in the <literal>host</literal> or
!         <literal>hostaddr</literal> parameters, this parameter may speci=
fy a list
!         of ports of equal length, or it may specify a single port number=
 to
!         be used for all hosts.
         </para>
        </listitem>
       </varlistentry>
--- 1051,1062 ----
          name extension for Unix-domain
          connections.<indexterm><primary>port</primary></indexterm>
          If multiple hosts were given in the <literal>host</literal> or
!         <literal>hostaddr</literal> parameters, this parameter may speci=
fy a
!         comma-separated list of ports of the same length as the host lis=
t, or
!         it may specify a single port number to be used for all hosts.
!         An empty string, or an empty item in a comma-separated list,
!         specifies the default port number established
!         when <productname>PostgreSQL</productname> was built.
         </para>
        </listitem>
       </varlistentry>
*************** char *PQuser(const PGconn *conn);
*** 1683,1688 ****
--- 1690,1706 ----
  char *PQpass(const PGconn *conn);
  </synopsis>
        </para>
+ =

+       <para>
+        <function>PQpass</function> will return either the password speci=
fied
+        in the connection parameters, or if there was none and the passwo=
rd
+        was obtained from the <link linkend=3D"libpq-pgpass">password
+        file</link>, it will return that.  In the latter case,
+        if multiple hosts were specified in the connection parameters, it=
 is
+        not possible to rely on the result of <function>PQpass</function>=
 until
+        the connection is established.  The status of the connection can =
be
+        checked using the function <function>PQstatus</function>.
+       </para>
       </listitem>
      </varlistentry>
  =

*************** myEventProc(PGEventId evtId, void *evtIn
*** 7521,7533 ****
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>=
.
!    A host name of <literal>localhost</literal> matches both TCP (host na=
me
!    <literal>localhost</literal>) and Unix domain socket (<literal>pghost=
</literal> empty
!    or the default socket directory) connections coming from the local
!    machine. In a standby server, a database name of <literal>replication=
</literal>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness becaus=
e
!    users have the same password for all databases in the same cluster.
    </para>
  =

    <para>
--- 7539,7556 ----
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>=
.
!    The host name field is matched to the <literal>host</literal> connect=
ion
!    parameter if that is specified, otherwise to
!    the <literal>hostaddr</literal> parameter if that is specified; if ne=
ither
!    are given then the host name <literal>localhost</literal> is searched=
 for.
!    The host name <literal>localhost</literal> is also searched for when
!    the connection is a Unix-domain socket connection in which
!    the <literal>host</literal> parameter equals the installation's defau=
lt
!    socket directory path.
!    In a standby server, a database name of <literal>replication</literal=
>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness otherw=
ise,
!    because users have the same password for all databases in the same cl=
uster.
    </para>
  =

    <para>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-c=
onnect.c
index bd7dac1..4b35994 100644
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** parse_comma_separated_list(char **startp
*** 901,906 ****
--- 901,908 ----
  static bool
  connectOptions2(PGconn *conn)
  {
+ 	int			i;
+ =

  	/*
  	 * Allocate memory for details about each host to which we might possib=
ly
  	 * try to connect.  For that, count the number of elements in the hosta=
ddr
*************** connectOptions2(PGconn *conn)
*** 920,930 ****
  =

  	/*
  	 * We now have one pg_conn_host structure per possible host.  Fill in t=
he
! 	 * host details for each one.
  	 */
  	if (conn->pghostaddr !=3D NULL && conn->pghostaddr[0] !=3D '\0')
  	{
- 		int			i;
  		char	   *s =3D conn->pghostaddr;
  		bool		more =3D true;
  =

--- 922,931 ----
  =

  	/*
  	 * We now have one pg_conn_host structure per possible host.  Fill in t=
he
! 	 * host and hostaddr fields for each, by splitting the parameter string=
s.
  	 */
  	if (conn->pghostaddr !=3D NULL && conn->pghostaddr[0] !=3D '\0')
  	{
  		char	   *s =3D conn->pghostaddr;
  		bool		more =3D true;
  =

*************** connectOptions2(PGconn *conn)
*** 933,940 ****
  			conn->connhost[i].hostaddr =3D parse_comma_separated_list(&s, &more);
  			if (conn->connhost[i].hostaddr =3D=3D NULL)
  				goto oom_error;
- =

- 			conn->connhost[i].type =3D CHT_HOST_ADDRESS;
  		}
  =

  		/*
--- 934,939 ----
*************** connectOptions2(PGconn *conn)
*** 948,954 ****
  =

  	if (conn->pghost !=3D NULL && conn->pghost[0] !=3D '\0')
  	{
- 		int			i;
  		char	   *s =3D conn->pghost;
  		bool		more =3D true;
  =

--- 947,952 ----
*************** connectOptions2(PGconn *conn)
*** 957,973 ****
  			conn->connhost[i].host =3D parse_comma_separated_list(&s, &more);
  			if (conn->connhost[i].host =3D=3D NULL)
  				goto oom_error;
- =

- 			/* Identify the type of host. */
- 			if (conn->pghostaddr =3D=3D NULL || conn->pghostaddr[0] =3D=3D '\0')
- 			{
- 				conn->connhost[i].type =3D CHT_HOST_NAME;
- #ifdef HAVE_UNIX_SOCKETS
- 				if (is_absolute_path(conn->connhost[i].host))
- 					conn->connhost[i].type =3D CHT_UNIX_SOCKET;
- #endif
- 			}
  		}
  		if (more || i !=3D conn->nconnhost)
  		{
  			conn->status =3D CONNECTION_BAD;
--- 955,963 ----
  			conn->connhost[i].host =3D parse_comma_separated_list(&s, &more);
  			if (conn->connhost[i].host =3D=3D NULL)
  				goto oom_error;
  		}
+ =

+ 		/* Check for wrong number of host items. */
  		if (more || i !=3D conn->nconnhost)
  		{
  			conn->status =3D CONNECTION_BAD;
*************** connectOptions2(PGconn *conn)
*** 979,1007 ****
  	}
  =

  	/*
! 	 * If neither host or hostaddr options was given, connect to default ho=
st.
  	 */
! 	if ((conn->pghostaddr =3D=3D NULL || conn->pghostaddr[0] =3D=3D '\0') &=
&
! 		(conn->pghost =3D=3D NULL || conn->pghost[0] =3D=3D '\0'))
  	{
! 		Assert(conn->nconnhost =3D=3D 1);
  #ifdef HAVE_UNIX_SOCKETS
! 		conn->connhost[0].host =3D strdup(DEFAULT_PGSOCKET_DIR);
! 		conn->connhost[0].type =3D CHT_UNIX_SOCKET;
  #else
! 		conn->connhost[0].host =3D strdup(DefaultHost);
! 		conn->connhost[0].type =3D CHT_HOST_NAME;
  #endif
! 		if (conn->connhost[0].host =3D=3D NULL)
! 			goto oom_error;
  	}
  =

  	/*
  	 * Next, work out the port number corresponding to each host name.
  	 */
  	if (conn->pgport !=3D NULL && conn->pgport[0] !=3D '\0')
  	{
- 		int			i;
  		char	   *s =3D conn->pgport;
  		bool		more =3D true;
  =

--- 969,1016 ----
  	}
  =

  	/*
! 	 * Now, for each host slot, identify the type of address spec, and fill=
 in
! 	 * the default address if nothing was given.
  	 */
! 	for (i =3D 0; i < conn->nconnhost; i++)
  	{
! 		pg_conn_host *ch =3D &conn->connhost[i];
! =

! 		if (ch->hostaddr !=3D NULL && ch->hostaddr[0] !=3D '\0')
! 			ch->type =3D CHT_HOST_ADDRESS;
! 		else if (ch->host !=3D NULL && ch->host[0] !=3D '\0')
! 		{
! 			ch->type =3D CHT_HOST_NAME;
  #ifdef HAVE_UNIX_SOCKETS
! 			if (is_absolute_path(ch->host))
! 				ch->type =3D CHT_UNIX_SOCKET;
! #endif
! 		}
! 		else
! 		{
! 			if (ch->host)
! 				free(ch->host);
! #ifdef HAVE_UNIX_SOCKETS
! 			ch->host =3D strdup(DEFAULT_PGSOCKET_DIR);
! 			ch->type =3D CHT_UNIX_SOCKET;
  #else
! 			ch->host =3D strdup(DefaultHost);
! 			ch->type =3D CHT_HOST_NAME;
  #endif
! 			if (ch->host =3D=3D NULL)
! 				goto oom_error;
! 		}
  	}
  =

  	/*
  	 * Next, work out the port number corresponding to each host name.
+ 	 *
+ 	 * Note: unlike the above for host names, this could leave the port fie=
lds
+ 	 * as null or empty strings.  We will substitute DEF_PGPORT whenever we
+ 	 * read such a port field.
  	 */
  	if (conn->pgport !=3D NULL && conn->pgport[0] !=3D '\0')
  	{
  		char	   *s =3D conn->pgport;
  		bool		more =3D true;
  =

*************** connectOptions2(PGconn *conn)
*** 1065,1072 ****
  	}
  =

  	/*
! 	 * Supply default password if none given.  Note that the password might=
 be
! 	 * different for each host/port pair.
  	 */
  	if (conn->pgpass =3D=3D NULL || conn->pgpass[0] =3D=3D '\0')
  	{
--- 1074,1081 ----
  	}
  =

  	/*
! 	 * If password was not given, try to look it up in password file.  Note
! 	 * that the result might be different for each host/port pair.
  	 */
  	if (conn->pgpass =3D=3D NULL || conn->pgpass[0] =3D=3D '\0')
  	{
*************** connectOptions2(PGconn *conn)
*** 1089,1108 ****
  =

  		if (conn->pgpassfile !=3D NULL && conn->pgpassfile[0] !=3D '\0')
  		{
- 			int			i;
- =

  			for (i =3D 0; i < conn->nconnhost; i++)
  			{
  				/*
! 				 * Try to get a password for this host from pgpassfile. We use
! 				 * host name rather than host address in the same manner as
! 				 * PQhost().
  				 */
! 				char	   *pwhost =3D conn->connhost[i].host;
  =

! 				if (conn->connhost[i].type =3D=3D CHT_HOST_ADDRESS &&
! 					conn->connhost[i].host !=3D NULL &&
! 					conn->connhost[i].host[0] !=3D '\0')
  					pwhost =3D conn->connhost[i].hostaddr;
  =

  				conn->connhost[i].password =3D
--- 1098,1113 ----
  =

  		if (conn->pgpassfile !=3D NULL && conn->pgpassfile[0] !=3D '\0')
  		{
  			for (i =3D 0; i < conn->nconnhost; i++)
  			{
  				/*
! 				 * Try to get a password for this host from file.  We use host
! 				 * for the hostname search key if given, else hostaddr (at
! 				 * least one of them is guaranteed nonempty by now).
  				 */
! 				const char *pwhost =3D conn->connhost[i].host;
  =

! 				if (pwhost =3D=3D NULL || pwhost[0] =3D=3D '\0')
  					pwhost =3D conn->connhost[i].hostaddr;
  =

  				conn->connhost[i].password =3D
*************** passwordFromFile(const char *hostname, c
*** 6385,6398 ****
  #define LINELEN NAMEDATALEN*5
  	char		buf[LINELEN];
  =

! 	if (dbname =3D=3D NULL || strlen(dbname) =3D=3D 0)
  		return NULL;
  =

! 	if (username =3D=3D NULL || strlen(username) =3D=3D 0)
  		return NULL;
  =

  	/* 'localhost' matches pghost of '' or the default socket directory */
! 	if (hostname =3D=3D NULL)
  		hostname =3D DefaultHost;
  	else if (is_absolute_path(hostname))
  =

--- 6390,6403 ----
  #define LINELEN NAMEDATALEN*5
  	char		buf[LINELEN];
  =

! 	if (dbname =3D=3D NULL || dbname[0] =3D=3D '\0')
  		return NULL;
  =

! 	if (username =3D=3D NULL || username[0] =3D=3D '\0')
  		return NULL;
  =

  	/* 'localhost' matches pghost of '' or the default socket directory */
! 	if (hostname =3D=3D NULL || hostname[0] =3D=3D '\0')
  		hostname =3D DefaultHost;
  	else if (is_absolute_path(hostname))
  =

*************** passwordFromFile(const char *hostname, c
*** 6403,6409 ****
  		if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) =3D=3D 0)
  			hostname =3D DefaultHost;
  =

! 	if (port =3D=3D NULL)
  		port =3D DEF_PGPORT_STR;
  =

  	/* If password file cannot be opened, ignore it. */
--- 6408,6414 ----
  		if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) =3D=3D 0)
  			hostname =3D DefaultHost;
  =

! 	if (port =3D=3D NULL || port[0] =3D=3D '\0')
  		port =3D DEF_PGPORT_STR;
  =

  	/* If password file cannot be opened, ignore it. */

------- =_aaaaaaaaaa0--