On 10/16/2016 12:09 PM, Fabien COELHO wrote:
Patch applies cleanly, make check ok... however AFAICS it only means that it compiles but it is not tested in anyway... This is is annoying. Well I'm not sure whether other options are tested either, but they should.
Thanks for taking the time to review my patch! I haven't found much regarding the testing of connection options. However since there isn't anything fancy going on in PasswordFromFile() I'm not too worried about that.

The documentation needs to be updated.
I've written a couple lines now.
I aligned the definition of the connection option and the environment variable with that of other (conn.opt&env.var.) pairs and added mention of the different options to the doc of the "Password File".

The reported password file is wrong. It is even more funny if ~/.pgpass contains the right password: the pgpassfile option *is* taken into account, ./foo is read and it fails, but the error message is not updated and points to the wrong file. The error message stuff should be updated to look for the pgpassfile connection string option...
That was indeed an Error on my side, I hadn't updated the errormessages to inform you which file has been used.

So attached is an updated version of the patch.

I'd like to ask for some input on how to handle invalid files - right now no message is shown, the user just gets a password prompt as a result, however I think a message when the custom pgpassfile hasn't been found would be helpful.

Kind regards,
Julian

--

Julian Markwort
Westphalian Wilhelms-University in M√ľnster
julian(dot)markwort(at)uni-muenster(dot)de

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4e34f00..1bd5597 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -930,7 +930,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         Note that authentication is likely to fail if <literal>host</>
         is not the name of the server at network address <literal>hostaddr</>.
         Also, note that <literal>host</> rather than <literal>hostaddr</>
-        is used to identify the connection in <filename>~/.pgpass</> (see
+        is used to identify the connection in a password file (see
         <xref linkend="libpq-pgpass">).
        </para>
 
@@ -986,6 +986,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
+     <varlistentry id="libpq-connect-pgpassfile" xreflabel="pgpassfile">
+      <term><literal>pgpassfile</literal></term>
+      <listitem>
+      <para>
+        Specifies the name of the file used to lookup passwords.
+        Defaults to the password file (see <xref linkend="libpq-pgpass">).
+      </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-connect-timeout" 
xreflabel="connect_timeout">
       <term><literal>connect_timeout</literal></term>
       <listitem>
@@ -6862,9 +6872,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void 
*passThrough)
       <indexterm>
        <primary><envar>PGPASSFILE</envar></primary>
       </indexterm>
-      <envar>PGPASSFILE</envar> specifies the name of the password file to
-      use for lookups.  If not set, it defaults to <filename>~/.pgpass</>
-      (see <xref linkend="libpq-pgpass">).
+      <envar>PGPASSFILE</envar> behaves the same as the <xref
+      linkend="libpq-connect-pgpassfile"> connection parameter.
      </para>
     </listitem>
 
@@ -7136,13 +7145,15 @@ myEventProc(PGEventId evtId, void *evtInfo, void 
*passThrough)
   </indexterm>
 
   <para>
-   The file <filename>.pgpass</filename> in a user's home directory or the
-   file referenced by <envar>PGPASSFILE</envar> can contain passwords to
+   The file <filename>.pgpass</filename> in a user's home directory can 
contain passwords to
    be used if the connection requires a password (and no password has been
    specified  otherwise). On Microsoft Windows the file is named
    <filename>%APPDATA%\postgresql\pgpass.conf</> (where
    <filename>%APPDATA%</> refers to the Application Data subdirectory in
    the user's profile).
+   Alternatively, a password file can be specified
+   using the connection parameter <xref linkend="libpq-connect-pgpassfile">
+   or the environment variable <envar>PGPASSFILE</envar>.
   </para>
 
   <para>
diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index f3a9e5a..db1de26 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -184,6 +184,10 @@ static const internalPQconninfoOption PQconninfoOptions[] 
= {
                "Database-Password", "*", 20,
        offsetof(struct pg_conn, pgpass)},
 
+       {"pgpassfile", "PGPASSFILE", NULL, NULL,
+               "Database-Password-File", "", 64,
+       offsetof(struct pg_conn, pgpassfile)},
+
        {"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
                "Connect-timeout", "", 10,              /* strlen(INT32_MAX) == 
10 */
        offsetof(struct pg_conn, connect_timeout)},
@@ -375,7 +379,7 @@ static int parseServiceFile(const char *serviceFile,
                                 bool *group_found);
 static char *pwdfMatchesString(char *buf, char *token);
 static char *PasswordFromFile(char *hostname, char *port, char *dbname,
-                                char *username);
+                                char *username, char *pgpassfile);
 static bool getPgPassFilename(char *pgpassfile);
 static void dot_pg_pass_warning(PGconn *conn);
 static void default_threadlock(int acquire);
@@ -806,8 +810,9 @@ connectOptions2(PGconn *conn)
        {
                if (conn->pgpass)
                        free(conn->pgpass);
+               /* We'll pass conn->pgpassfile regardless of it's contents - 
checks happen in PasswordFromFile() */
                conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
-                                                                               
conn->dbName, conn->pguser);
+                                                                               
conn->dbName, conn->pguser, conn->pgpassfile);
                if (conn->pgpass == NULL)
                {
                        conn->pgpass = strdup(DefaultPassword);
@@ -5703,12 +5708,12 @@ pwdfMatchesString(char *buf, char *token)
        return NULL;
 }
 
-/* Get a password from the password file. Return value is malloc'd. */
+/* Get a password from the password file or the user-specified pgpassfile. 
Return value is malloc'd. */
 static char *
-PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
+PasswordFromFile(char *hostname, char *port, char *dbname, char *username, 
char *pgpassfile)
 {
        FILE       *fp;
-       char            pgpassfile[MAXPGPATH];
+       char            temp_path[MAXPGPATH];
        struct stat stat_buf;
 
 #define LINELEN NAMEDATALEN*5
@@ -5735,8 +5740,13 @@ PasswordFromFile(char *hostname, char *port, char 
*dbname, char *username)
        if (port == NULL)
                port = DEF_PGPORT_STR;
 
-       if (!getPgPassFilename(pgpassfile))
-               return NULL;
+       if(!pgpassfile || pgpassfile[0]=='\0')
+       {
+               /* If no pgpassfile was supplied by the user or it is empty, we 
try to get a global pgpassfile */
+               if (!getPgPassFilename(temp_path))
+                       return NULL;
+               pgpassfile = temp_path;
+       }
 
        /* If password file cannot be opened, ignore it. */
        if (stat(pgpassfile, &stat_buf) != 0)
@@ -5858,11 +5868,14 @@ dot_pg_pass_warning(PGconn *conn)
        {
                char            pgpassfile[MAXPGPATH];
 
-               if (!getPgPassFilename(pgpassfile))
-                       return;
+               /* If no pgpassfile was specified with the connection, we try 
to get the default one */
+               if (!conn->pgpassfile){
+                       if(!getPgPassFilename(pgpassfile))
+                               return;
+               }
                appendPQExpBuffer(&conn->errorMessage,
                                          libpq_gettext("password retrieved 
from file \"%s\"\n"),
-                                                 pgpassfile);
+                                                 
(conn->pgpassfile)?(conn->pgpassfile):pgpassfile);
        }
 }
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 7007692..9f5c2f9 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -317,6 +317,7 @@ struct pg_conn
        char       *replication;        /* connect as the replication standby? 
*/
        char       *pguser;                     /* Postgres username and 
password, if any */
        char       *pgpass;
+       char       *pgpassfile;         /* path to a file containing the 
password */
        char       *keepalives;         /* use TCP keepalives? */
        char       *keepalives_idle;    /* time between TCP keepalives */
        char       *keepalives_interval;        /* time between TCP keepalive
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to